test: add unit coverage for src/utils/advisory.ts

[suhanemathur]

What changed and why

Adds unit tests for the isPrivateRegistrySource, isGitSource, and
hasCommitShaPinning functions in src/utils/advisory.ts, which had no
dedicated test coverage.

Closes #1024

Before test coverage

Test Suites: 13 failed, 101 passed, 114 total
Tests: 76 failed, 1370 passed, 1446 total
Snapshots: 0 total
Time: 28.814 s, estimated 29 s
Ran all test suites.

After test coverage

Test Suites: 13 failed, 102 passed, 115 total
Tests: 76 failed, 1381 passed, 1457 total
Snapshots: 0 total
Time: 29.643 s
Ran all test suites.

Added one new file tests/utils/advisory.test.ts with 11 new unit tests,
all passing, covering positive, negative, and undefined/empty-string
cases for all three functions.

[sonukapoor]

Good coverage overall - the makePkg factory is a clean pattern, and the undefined/empty-string edge cases are exactly the right things to pin down.

Two small things before we merge:

1. The file is missing a trailing newline on the last line.

2. isGitSource uses a prefix list that includes git+https://, gitlab.com, bitbucket.org, and others beyond GitHub. Could you add one more test for a non-GitHub prefix - e.g. git+https:// - so we have coverage that the broader list is exercised? Something like:

it("returns true when resolvedUrl uses the git+https scheme", () => {
  const pkg = makePkg({ resolvedUrl: "git+https://github.com/some-org/some-repo.git" });
  expect(isGitSource(pkg)).toBe(true);
});

Both are quick fixes - happy to merge once they're in.

Also, once this lands we will close #1037 as superseded by this PR.

[suhanemathur]

Thanks for the review! I've pushed a fix for both:

• Added the trailing newline
• Added the git+https scheme test case for isGitSource

Ready for another look.

[sonukapoor]

Nice work on the structure here, @suhanemathur - makePkg is clean and used consistently, and the test descriptions are clear. A couple of things before I can merge this:

Rebase needed

Your PR description shows "Before: 13 suites failed, 76 tests failed" as the baseline. That's not the current state of main - all tests are passing there. Your branch was cut from an older point. Can you do a git rebase origin/main and re-run the suite? The "After" numbers should show a clean 0-failing baseline with your 11 tests added on top.

Missing git+ssh:// case in isGitSource

The source has "git+ssh://" in GIT_SOURCE_PREFIXES - a distinct protocol from git+https:// and https://github.com/ that none of your five tests cover. Worth adding:

it("returns true when resolvedUrl uses the git+ssh scheme", () => {
  const pkg = makePkg({
    resolvedUrl: "git+ssh://git@github.com/some-org/some-repo.git",
  });
  expect(isGitSource(pkg)).toBe(true);
});

Optional (not blocking): a short-SHA case for hasCommitShaPinning to document that 7-char abbreviated hashes don't count:

it("returns false when resolvedUrl has a short (abbreviated) commit SHA", () => {
  const pkg = makePkg({
    resolvedUrl: "git+https://github.com/some-org/some-repo.git#a1b2c3d",
  });
  expect(hasCommitShaPinning(pkg)).toBe(false);
});

Fix the rebase and the git+ssh test and this is good to go.

[suhanemathur]

Thanks for the review! I have made the following changes:

• Rebased onto the latest upstream/main
• Added the git+ssh:// test case for isGitSource
• Added the short (abbreviated) SHA test case for hasCommitShaPinning

Branch is rebased onto the latest upstream/main. The required CI / build check passes on Linux. Local Windows runs show some pre-existing failing suites unrelated to this change, with identical failure counts before and after this PR's tests were added.

[sonukapoor]

Looks great - the coverage is solid and both follow-up items from the review (git+ssh:// test and the short-SHA alias test) are in. Merging.

[sonukapoor]

Merged - thank you @suhanemathur!