Skip to content

test: add coverage for phantom-utils (getDeclaredPackages and installCmd) #1124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
emihesi99 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test: add coverage for phantom-utils (getDeclaredPackages and installCmd) #1124

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions tests/overrides/detectors/phantom-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import {
getDeclaredPackages,
installCmd,
} from "../../../src/overrides/detectors/phantom-utils.js";
import type { OverrideContext } from "../../../src/overrides/context.js";

describe("getDeclaredPackages", () => {
it("returns an empty Set for an empty package.json", () => {
expect(getDeclaredPackages({})).toEqual(new Set());
});

it("collects names from all four dependency sections", () => {
const pkg = {
dependencies: { react: "^18.0.0" },
devDependencies: { jest: "^30.0.0" },
peerDependencies: { typescript: "^5.0.0" },
optionalDependencies: { fsevents: "^2.0.0" },
};
const result = getDeclaredPackages(pkg);
expect(result).toEqual(
new Set(["react", "jest", "typescript", "fsevents"]),
);
});

it("ignores a dependency section that is an array", () => {
const pkg = {
dependencies: ["react", "vue"],
devDependencies: { jest: "^30.0.0" },
};
const result = getDeclaredPackages(pkg);
expect(result).toEqual(new Set(["jest"]));
});

it("ignores a dependency section that is null", () => {
const pkg = {
dependencies: null,
devDependencies: { jest: "^30.0.0" },
};
const result = getDeclaredPackages(pkg);
expect(result).toEqual(new Set(["jest"]));
});

it("ignores a dependency section that is a string", () => {
const pkg = {
dependencies: "not-an-object",
devDependencies: { jest: "^30.0.0" },
};
const result = getDeclaredPackages(pkg);
expect(result).toEqual(new Set(["jest"]));
});

@sonukapoor sonukapoor Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One case missing, and it is the only real gap the mutation run found. I added "overrides" to the section list in getDeclaredPackages and all 12 tests still passed.

It matters slightly more than it looks, because this function decides whether a package counts as declared for PD001 (high severity, and it carries an override) and PD002. A real package.json has overrides, resolutions and scripts sitting at the same level as the four sections that should count.

Something like:

it("ignores sections that are not dependency sections", () => {
  const pkg = {
    dependencies: { react: "^18.0.0" },
    overrides: { lodash: "4.17.21" },
    resolutions: { minimist: "1.2.8" },
    scripts: { build: "tsc" },
  };
  expect(getDeclaredPackages(pkg)).toEqual(new Set(["react"]));
});


it("deduplicates names that appear in multiple sections", () => {
const pkg = {
dependencies: { react: "^18.0.0" },
devDependencies: { react: "^18.0.0" },
peerDependencies: { react: "^18.0.0" },
};
const result = getDeclaredPackages(pkg);
expect(result).toEqual(new Set(["react"]));
});

it("handles scoped package names", () => {
const pkg = {
dependencies: { "@scope/pkg": "^1.0.0" },
devDependencies: { "@types/node": "^24.0.0" },
};
const result = getDeclaredPackages(pkg);
expect(result).toEqual(new Set(["@scope/pkg", "@types/node"]));
});
});

describe("installCmd", () => {
it("returns 'pnpm add' for pnpm", () => {
expect(installCmd("pnpm" as OverrideContext["packageManager"])).toBe(

@sonukapoor sonukapoor Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These five casts are not doing anything. PackageManager is already "npm" | "pnpm" | "yarn" | "bun" | "unknown" (src/overrides/context.ts:14), so every string you pass is already a member of the union. I checked by deleting all five plus the then-unused import on line 5, and tsc reports no errors.

Worth removing rather than leaving, because the cast suppresses the one error you would actually want. If someone later drops "unknown" or renames "bun", these tests keep compiling and the drift goes unnoticed. Uncast, the build breaks and tells you, which for a file whose whole job is to pin this function's contract is the behaviour you want.

It also collapses each assertion back to one line:

expect(installCmd("pnpm")).toBe("pnpm add");

"pnpm add",
);
});

it("returns 'yarn add' for yarn", () => {
expect(installCmd("yarn" as OverrideContext["packageManager"])).toBe(
"yarn add",
);
});

it("returns 'bun add' for bun", () => {
expect(installCmd("bun" as OverrideContext["packageManager"])).toBe(
"bun add",
);
});

it("returns 'npm install' for npm", () => {
expect(installCmd("npm" as OverrideContext["packageManager"])).toBe(
"npm install",
);
});

it("returns 'npm install' as the default for an unknown package manager", () => {
expect(installCmd("unknown" as OverrideContext["packageManager"])).toBe(
"npm install",
);
});
});