Two test gaps found while reviewing #1084. That PR is merged and the behaviour is correct; what is missing is coverage pinning it, so either could regress silently.
Both are small and self-contained. Good pick-up if you want a first contribution here.
1. Build metadata is untested
Semver §10 says build metadata is ignored when comparing versions, so 1.2.3+build and 1.2.3 are equal in precedence. compareVersions gets this right, but nothing tests it. Verified by mutation: deleting the (?:\+.*)? group from VERSION_SHAPE in src/utils/version.ts still leaves all 1,675 tests passing. That mutation quietly routes every version carrying build metadata to the legacy fallback, where compareVersions("1.2.3+build", "1.2.3") returns -1 instead of 0.
Add to tests/helpers.test.ts, near the other semver precedence tests:
expect(compareVersions("1.2.3+build", "1.2.3")).toBe(0);
expect(compareVersions("1.2.3-beta+a", "1.2.3-beta+b")).toBe(0);
To check your work: make the assertions pass, then temporarily delete (?:\+.*)? from VERSION_SHAPE and confirm your new tests fail. Put it back afterwards.
2. A test that copies the code it is meant to test
tests/helpers.test.ts has a test named "reports a pre-release install as vulnerable when the fix landed in the release (issue #1077)". It hand-copies the body of versionMatchesRange into the test rather than calling the real function in src/advisory/local-db.ts. So it proves compareVersions behaves correctly, but proves nothing about the scanner path it is named after. Verified: mutating the real versionMatchesRange (changing >= 0 to > 0 at src/advisory/local-db.ts:224) leaves that test green.
The copy is not even faithful, since it drops the real function's introduced && guard.
The fix is to exercise the real path. tests/local-advisory-source.test.ts already seeds lodash with fixed: 4.17.21 around lines 63-67. Adding a package at a pre-release below that fix, for example lodash@4.17.21-beta.1, should report the vulnerability. That pins the scanner path end to end.
Consider whether the hand-copied test still earns its place once the real one exists, or whether it should be reworked to test compareVersions directly under an honest name.
Notes
Please do not change any behaviour in src/utils/version.ts or src/advisory/local-db.ts for this. Both are correct today; this is purely about adding coverage. If you find a case where they are genuinely wrong, that is worth its own issue.
Run npm test before opening a PR.
Two test gaps found while reviewing #1084. That PR is merged and the behaviour is correct; what is missing is coverage pinning it, so either could regress silently.
Both are small and self-contained. Good pick-up if you want a first contribution here.
1. Build metadata is untested
Semver §10 says build metadata is ignored when comparing versions, so
1.2.3+buildand1.2.3are equal in precedence.compareVersionsgets this right, but nothing tests it. Verified by mutation: deleting the(?:\+.*)?group fromVERSION_SHAPEinsrc/utils/version.tsstill leaves all 1,675 tests passing. That mutation quietly routes every version carrying build metadata to the legacy fallback, wherecompareVersions("1.2.3+build", "1.2.3")returns-1instead of0.Add to
tests/helpers.test.ts, near the other semver precedence tests:To check your work: make the assertions pass, then temporarily delete
(?:\+.*)?fromVERSION_SHAPEand confirm your new tests fail. Put it back afterwards.2. A test that copies the code it is meant to test
tests/helpers.test.tshas a test named "reports a pre-release install as vulnerable when the fix landed in the release (issue #1077)". It hand-copies the body ofversionMatchesRangeinto the test rather than calling the real function insrc/advisory/local-db.ts. So it provescompareVersionsbehaves correctly, but proves nothing about the scanner path it is named after. Verified: mutating the realversionMatchesRange(changing>= 0to> 0atsrc/advisory/local-db.ts:224) leaves that test green.The copy is not even faithful, since it drops the real function's
introduced &&guard.The fix is to exercise the real path.
tests/local-advisory-source.test.tsalready seedslodashwithfixed: 4.17.21around lines 63-67. Adding a package at a pre-release below that fix, for examplelodash@4.17.21-beta.1, should report the vulnerability. That pins the scanner path end to end.Consider whether the hand-copied test still earns its place once the real one exists, or whether it should be reworked to test
compareVersionsdirectly under an honest name.Notes
Please do not change any behaviour in
src/utils/version.tsorsrc/advisory/local-db.tsfor this. Both are correct today; this is purely about adding coverage. If you find a case where they are genuinely wrong, that is worth its own issue.Run
npm testbefore opening a PR.