test: restore skipped connectToDb tests#69
Merged
Conversation
The connectToDb suite was skipped because its logger mock used the wrong relative path (../src instead of ../../src from tests/unit), so the mock never applied and the assertions failed. Fix the path and unskip; both connect-success and connect-failure cases now pass, and db.ts (previously 0% coverage) is now exercised. Leave the dev-mode JWKS test skipped with a documented reason: it depends on ./keys/dev/public.pem (absent in CI) and on fs/jose mocks that do not reliably reach the app built in beforeAll. Re-enabling it needs harness rework, tracked in #14. Addresses #14.
Bccorb
force-pushed
the
test/restore-skipped-db-jwks
branch
from
842147a to
8f7efcd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Bccorb
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #14
Summary
tests/unit/db.spec.ts(connectToDb) was genuinely broken: the logger mock used../src/utils/logger, but fromtests/unit/that resolves totests/src(nonexistent), so the mock never applied and the log assertions failed. Corrected to../../src/utils/loggerand unskipped. Both the connect-success and connect-failure cases pass, anddb.ts(previously 0% coverage) is now exercised.Dev JWKS test stays skipped (with a reason)
I initially unskipped
tests/integration/jwks/jwks.spec.ts(returns dev jwks) too, and it passed locally — but it fails in CI (404instead of200). Root cause: the dev branch ofjwksHandlerreads./keys/dev/public.pemfrom disk, which exists in a local dev checkout but not in CI (keys/is gitignored), and thefs/josemocks don't reliably reach the handler because the app is built once inbeforeAll, before the per-testresetModules/env stubbing. Re-enabling it needs harness rework (build the app per test, or exercise the dev branch without real key material). It's re-skipped with an inline comment explaining this.Testing
db.spec.ts+jwks.spec.tspass (dev JWKS test skipped). Typecheck + lint clean.Note on CI
The
verify / verify(cross-repo conformance) check fails on this PR, but it fails on every open PR including docs-only ones, so it is pre-existing and unrelated to this change (react harness "invalid URL" + adapter rate-limit 429s).