fix(tests): restore CI compat with pyjwt 2.13 and fastapi 0.137 - #206
Conversation
The dependency upgrade (#204) bumped pyjwt 2.12.1->2.13.0 and fastapi 0.136.1->0.137.0, breaking 7 tests. Both are test-only — production is unaffected (routing verified working, Deploy to Production passed on the same commit, 1647 non-affected tests stayed green). - pyjwt 2.13 now raises InvalidKeyError on empty HMAC keys. The JWT security tests sign tokens with jwt_secret, which is "" in CI (no JWT_SECRET env var and .env is gitignored). Set a non-prod JWT_SECRET in the root conftest, before any test constructs AppSettings(). - fastapi 0.137 no longer flattens include_router() routes into app.routes; included routers appear as _IncludedRouter wrappers with the prefix on .include_context. Rebuild full paths (incl. HEAD) by walking that tree in the route-registration smoke test.
Reviewer's GuideTest-only fixes to restore CI after upgrading to pyjwt 2.13.0 and fastapi 0.137.0 by ensuring JWT tests use a non-empty secret and updating route discovery logic to handle FastAPI’s new _IncludedRouter structure while remaining backward-compatible. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTwo test infrastructure fixes: ChangesTest Infrastructure Fixes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Restores test-suite compatibility with recent dependency upgrades (PyJWT 2.13+ and FastAPI 0.137+) so CI passes again without production code changes.
Changes:
- Set a default non-empty
JWT_SECRETfor tests to prevent PyJWT 2.13 from rejecting empty HMAC keys. - Update smoke-test route discovery to correctly traverse FastAPI 0.137’s included-router wrapper nodes and reconstruct full paths/methods.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/conftest.py |
Ensures tests have a JWT secret set before AppSettings() is instantiated. |
tests/smoke/test_routes_registered.py |
Reworks route collection to flatten included routers under FastAPI 0.137 while remaining compatible with older versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- conftest: set JWT_SECRET when missing OR empty (setdefault would keep an explicit JWT_SECRET="", still tripping pyjwt 2.13's empty-key guard). - smoke: store route methods as an immutable frozenset in _ResolvedRoute (a set inside a NamedTuple is mutable and makes the tuple non-hashable).
Why
The dependency upgrade in #204 turned
mainCI red — 7 tests fail in the Tests & Coverage job. This is test-only; production is unaffected (routing verified working, Deploy to Production passed on the same commit, and all 1647 non-affected tests stayed green). Thefail-fastmatrix made it look like a flaky single-version failure — really every Python version fails identically; the first to trip cancels the rest.Root causes (both from #204 bumps)
pyjwt2.12.1 → 2.13.0 now raisesInvalidKeyError: HMAC key must not be empty.The 4 JWT tests intests/integration/test_security.pysign withjwt_secret, which is""in CI (noJWT_SECRETenv var, and.envis gitignored). 2.12.1 silently tolerated the empty key.fastapi0.136.1 → 0.137.0 no longer flattensinclude_router()routes intoapp.routes; they now appear as_IncludedRouterwrapper nodes. The 3 route smoke tests intests/smoke/test_routes_registered.pyfilterapp.routesforAPIRouteand found zero.Fix (test-only, ~40 lines, 2 files)
tests/conftest.py:os.environ.setdefault("JWT_SECRET", ...)before any test buildsAppSettings(), so token signing/verification have a non-empty key everywhere.tests/smoke/test_routes_registered.py: walk the_IncludedRoutertree (accumulating.include_context.prefix) to reconstruct full paths + methods (incl. auto-HEAD), with a clean fallback to flatAPIRoutes on older FastAPI.Verification
The 7 previously-failing tests now pass; no other test changed. No production code touched.
Summary by Sourcery
Restore CI test compatibility with updated pyjwt and FastAPI dependencies by adjusting JWT test configuration and route discovery logic.
Bug Fixes:
Tests:
Summary by CodeRabbit