Skip to content

test: pin semver build-metadata handling and exercise the real versionMatchesRange path #1087

Description

@sonukapoor

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions