Skip to content
42 changes: 26 additions & 16 deletions test/js/node/process/process.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,24 +326,34 @@ it("process.umask()", () => {
expect(process.umask()).toBe(orig);
});

it("process.versions", () => {
// Expected dependency versions — must match scripts/build/deps/*.ts commits.
// These are the ACTUAL commits built into bun (not derived values, so
// bumping a dep requires updating this test too).
const expectedVersions = {
boringssl: "1a41b9025c2c0a37edd07ff10f6944f03e028522",
libarchive: "ded82291ab41d5e355831b96b0e1ff49e24d8939",
mimalloc: "afb41757285694f832e7a2f164d35f5717457f96",
picohttpparser: "066d2b1e9ab820703db0837a7255d92d30f0c9f5",
zlib: "12731092979c6d07f42da27da673a9f6c7b13586",
tinycc: "12882eee073cfe5c7621bcfadf679e1372d4537b",
lolhtml: "77127cd2b8545998756e8d64e36ee2313c4bb312",
ares: "3ac47ee46edd8ea40370222f91613fc16c434853",
libdeflate: "c8c56a20f8f621e6a966b716b31f1dedab6a41e3",
zstd: "f8745da6ff1ad1e7bab384bd1f9d742439278e99",
lshpack: "8905c024b6d052f083a3d11d0a169b3c2735c8a1",
it("process.versions", async () => {
// Verifies process.versions reports the same commits pinned in
// scripts/build/deps/*.ts. Reading the source files at test time keeps a
// single source of truth so dep bumps don't require touching this test.
const depsDir = resolve(import.meta.dir, "../../../../scripts/build/deps");
Comment on lines +572 to +576

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 The file now has two it("process.versions", ...) blocks with identical names — the new one at line 271 (reads dep commit hashes from scripts/build/deps/*.ts) and a pre-existing one at line 1186 (checks hardcoded node/v8/napi/modules strings). Both run when filtering with -t "^process.versions$", producing duplicate names in test reports and making it harder to tell which assertion failed. Consider renaming the new block to "process.versions (dep commits)" and the old one to "process.versions (node metadata)" to distinguish the two concerns.

Extended reasoning...

What the bug is and how it manifests

The file test/js/node/process/process.test.js has two it() blocks both named "process.versions". The first (line 271, modified by this PR) is an async test that reads each pinned commit from scripts/build/deps/.ts and asserts it matches process.versions[key]. The second (line 1186, pre-existing) is a synchronous test that asserts hardcoded string values for process.versions.node, .v8, .napi, and .modules. Both tests are structurally valid and pass, but they share the same name.

The specific code path that triggers it

The PR description explicitly calls out: USE_SYSTEM_BUN=1 bun test ... -t "^process.versions$" — fails. That filter matches both blocks. Bun's test runner runs every it() whose name matches the regex, so both execute under the filter, and both appear in CI/terminal output with the identical label "process.versions".

Why existing code does not prevent it

Test frameworks (including Bun's) do not enforce unique it() names within a file or suite. The duplicate was present before this PR (confirmed in commit edde070); the PR modified the first block to switch from hardcoded hashes to dynamically derived ones, but kept the name "process.versions" unchanged.

Impact

When one of the two tests fails, the output shows a failure for "process.versions" without indicating which of the two distinct concerns failed. Anyone reading CI logs or filtering with -t would run both tests and see two results under the same label — making debugging needlessly confusing.

How to fix it

Rename one or both tests to reflect what they actually verify:

  • Line 271: "process.versions (dep commits)"
  • Line 1186: "process.versions (node metadata)"

Step-by-step proof

  1. Open test/js/node/process/process.test.js.
  2. Search for it("process.versions" — two matches appear: line 271 (the async dep-hash test added/modified by this PR) and line 1186 (the synchronous node/v8/napi/modules test).
  3. Run: bun test test/js/node/process/process.test.js -t "^process.versions$"
  4. Observe two test results both labeled "process.versions" in the output.
  5. Temporarily break the line-1186 assertion (e.g., change "24.3.0" to "0.0.0"); the failure message says "process.versions" with no indication it is the node-metadata check rather than the dep-commits check.

const deps = {
boringssl: "boringssl",
libarchive: "libarchive",
mimalloc: "mimalloc",
picohttpparser: "picohttpparser",
zlib: "zlib",
tinycc: "tinycc",
lolhtml: "lolhtml",
ares: "cares",
libdeflate: "libdeflate",
zstd: "zstd",
lshpack: "lshpack",
};

const expectedVersions = {};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
for (const [key, file] of Object.entries(deps)) {
const src = await Bun.file(join(depsDir, `${file}.ts`)).text();
Comment thread
claude[bot] marked this conversation as resolved.
// No $ anchor: some pins carry a trailing comment (zlib.ts: `"; // 2.3.3`)
const match = src.match(/^const [A-Z_]+_COMMIT = "([0-9a-f]{40})";/m);
expect(match, `failed to extract commit from ${file}.ts`).not.toBeNull();
expectedVersions[key] = match[1];
}

for (const [name, expectedHash] of Object.entries(expectedVersions)) {
expect(process.versions).toHaveProperty(name);
expect(process.versions[name]).toBe(expectedHash);
Expand Down
Loading