Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .changeset/hot-bikes-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
2 changes: 1 addition & 1 deletion .github/workflows/pr-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ jobs:
run: bun run test:theme-contrast

- name: Test suite
run: bun test ./src ./packages ./scripts ./test/cli ./test/session
run: bun run test

compiled-headless-portability:
name: Compiled headless portability (${{ matrix.os }})
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"changeset:status": "bunx @changesets/cli@2.31.0 status",
"release:version": "bunx @changesets/cli@2.31.0 version",
"prepare": "simple-git-hooks",
"test": "\"${npm_execpath:-bun}\" test ./src ./packages ./scripts ./test/cli ./test/session",
"test": "bun run ./scripts/run-test-suite.ts",
"test:theme-contrast": "bun test src/ui/themes.test.ts --test-name-pattern contrast",
"test:integration": "\"${npm_execpath:-bun}\" test ./test/pty",
"test:tty-smoke": "HUNK_RUN_TTY_SMOKE=1 \"${npm_execpath:-bun}\" test ./test/smoke",
Expand Down
79 changes: 79 additions & 0 deletions scripts/run-test-suite.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { describe, expect, test } from "bun:test";
import {
buildTestShardCommand,
DEFAULT_TEST_PATTERNS,
resolveTestShardCount,
terminateTestShardProcesses,
} from "./run-test-suite";

describe("test suite sharding", () => {
test("uses the available CPUs up to the automatic Linux cap", () => {
expect(resolveTestShardCount(1, undefined, "linux")).toBe(1);
expect(resolveTestShardCount(2, undefined, "linux")).toBe(2);
expect(resolveTestShardCount(32, undefined, "linux")).toBe(2);
});

test("accepts an explicit positive shard count on Linux", () => {
expect(resolveTestShardCount(32, "1", "linux")).toBe(1);
expect(resolveTestShardCount(2, "16", "linux")).toBe(16);
});

test("keeps non-Linux suites serial", () => {
expect(resolveTestShardCount(32, undefined, "win32")).toBe(1);
expect(resolveTestShardCount(32, "16", "darwin")).toBe(1);
});

test("rejects malformed or excessive Linux shard overrides", () => {
expect(() => resolveTestShardCount(8, "0", "linux")).toThrow(
"HUNK_TEST_SHARDS must be a positive safe integer",
);
expect(() => resolveTestShardCount(8, "2.5", "linux")).toThrow(
"HUNK_TEST_SHARDS must be a positive safe integer",
);
expect(() => resolveTestShardCount(8, "999999999999999999999999", "linux")).toThrow(
"HUNK_TEST_SHARDS must be a positive safe integer",
);
expect(() => resolveTestShardCount(8, "65", "linux")).toThrow(
"HUNK_TEST_SHARDS cannot exceed 64",
);
});

test("builds serial and sharded Bun commands", () => {
expect(buildTestShardCommand("/opt/bun", 1, 1, [], "linux")).toEqual([
"/opt/bun",
"test",
"--no-orphans",
...DEFAULT_TEST_PATTERNS,
]);
expect(buildTestShardCommand("/opt/bun", 2, 4, ["--rerun-each=2"], "linux")).toEqual([
"/opt/bun",
"test",
"--no-orphans",
"--shard=2/4",
...DEFAULT_TEST_PATTERNS,
"--rerun-each=2",
]);
expect(buildTestShardCommand("C:\\bun.exe", 1, 1, [], "win32")).toEqual([
"C:\\bun.exe",
"test",
...DEFAULT_TEST_PATTERNS,
]);
});

test("forwards termination while tolerating an already stopped shard", () => {
const signals: Array<NodeJS.Signals> = [];
terminateTestShardProcesses(
[
{ kill: (signal) => signals.push(signal as NodeJS.Signals) },
{
kill: () => {
throw new Error("already stopped");
},
},
],
"SIGTERM",
);

expect(signals).toEqual(["SIGTERM"]);
});
});
166 changes: 166 additions & 0 deletions scripts/run-test-suite.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
#!/usr/bin/env bun

/**
* Runs Hunk's default tests concurrently without Bun's isolated parallel worker mode.
*
* Bun 1.3.14's `--parallel` implies `--isolate`, which makes OpenTUI's native FFI
* renderer fail to initialize with "Cannot access 'default' before initialization."
* Independent `--shard=N/M` processes avoid that failure, but Bun runs only the one
* requested shard, so this module launches and supervises every shard. Sharding stays
* Linux-only because the complete multi-process suite is validated and benchmarked there.
*/

import { availableParallelism } from "node:os";

export const DEFAULT_TEST_PATTERNS = [
"./src",
"./packages",
"./scripts",
"./test/cli",
"./test/session",
] as const;

const MAX_AUTOMATIC_TEST_SHARDS = 2;
const MAX_EXPLICIT_TEST_SHARDS = 64;
const SHARD_TERMINATION_GRACE_MS = 1_000;

type KillableProcess = {
kill(signal?: number | NodeJS.Signals): void;
};

/** Resolve a Linux shard override or choose a bounded count from the available CPUs. */
export function resolveTestShardCount(
cpuCount: number,
override?: string,
platform: NodeJS.Platform = process.platform,
) {
if (platform !== "linux") return 1;

if (override !== undefined) {
const count = Number(override);
if (!/^\d+$/.test(override) || !Number.isSafeInteger(count) || count < 1) {
throw new Error("HUNK_TEST_SHARDS must be a positive safe integer");
}
if (count > MAX_EXPLICIT_TEST_SHARDS) {
throw new Error(`HUNK_TEST_SHARDS cannot exceed ${MAX_EXPLICIT_TEST_SHARDS}`);
}
return count;
}

return Math.min(MAX_AUTOMATIC_TEST_SHARDS, Math.max(1, Math.floor(cpuCount)));
}

/** Build one Bun test command for an independent file shard. */
export function buildTestShardCommand(
bunExecutable: string,
shard: number,
shardCount: number,
forwardedArgs: string[] = [],
platform: NodeJS.Platform = process.platform,
) {
return [
bunExecutable,
"test",
...(platform === "win32" ? [] : ["--no-orphans"]),
...(shardCount > 1 ? [`--shard=${shard}/${shardCount}`] : []),
...DEFAULT_TEST_PATTERNS,
...forwardedArgs,
];
}

/** Forward a termination signal to every live shard, tolerating shards that already exited. */
export function terminateTestShardProcesses(processes: KillableProcess[], signal: NodeJS.Signals) {
for (const proc of processes) {
try {
proc.kill(signal);
} catch {
// Another shard or the terminal process group may already have stopped it.
}
}
}

/** Run the default suite in independent Bun processes without enabling Bun's isolate mode. */
export async function main(args = Bun.argv.slice(2)) {
const shardCount = resolveTestShardCount(availableParallelism(), process.env.HUNK_TEST_SHARDS);

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.

P2 Direct environment access bypasses validation

The new supervisor reads HUNK_TEST_SHARDS directly from process.env and also spreads process.env into each child at line 87, bypassing the repository-prescribed type-safe environment interface and establishing an inconsistent configuration-access pattern.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/run-test-suite.ts
Line: 76

Comment:
**Direct environment access bypasses validation**

The new supervisor reads `HUNK_TEST_SHARDS` directly from `process.env` and also spreads `process.env` into each child at line 87, bypassing the repository-prescribed type-safe environment interface and establishing an inconsistent configuration-access pattern.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

const bunExecutable = process.execPath;

console.error(`Running the test suite in ${shardCount} shard${shardCount === 1 ? "" : "s"}...`);

const shards: Array<{ proc: ReturnType<typeof Bun.spawn>; shard: number }> = [];
try {
for (let index = 0; index < shardCount; index += 1) {
const shard = index + 1;
const proc = Bun.spawn(
buildTestShardCommand(bunExecutable, shard, shardCount, args, process.platform),
{
cwd: process.cwd(),
env: { ...process.env, npm_execpath: bunExecutable },
stdin: "inherit",
stdout: "inherit",
stderr: "inherit",
},
);
shards.push({ proc, shard });
}
} catch (error) {
const spawnedProcesses = shards.map(({ proc }) => proc);
terminateTestShardProcesses(spawnedProcesses, "SIGTERM");
const forceKillTimer = setTimeout(() => {
terminateTestShardProcesses(spawnedProcesses, "SIGKILL");
}, SHARD_TERMINATION_GRACE_MS);
forceKillTimer.unref();
try {
await Promise.allSettled(shards.map(({ proc }) => proc.exited));
} finally {
clearTimeout(forceKillTimer);
}
throw error;
}

const processes = shards.map(({ proc }) => proc);
let interruptedExitCode: number | null = null;
let forceKillTimer: ReturnType<typeof setTimeout> | null = null;
const handleSignal = (signal: NodeJS.Signals, exitCode: number) => {
if (interruptedExitCode !== null) return;
interruptedExitCode = exitCode;
terminateTestShardProcesses(processes, signal);
forceKillTimer = setTimeout(() => {
terminateTestShardProcesses(processes, "SIGKILL");
}, SHARD_TERMINATION_GRACE_MS);
forceKillTimer.unref();
};
const handleSigint = () => handleSignal("SIGINT", 130);
const handleSigterm = () => handleSignal("SIGTERM", 143);
process.once("SIGINT", handleSigint);
process.once("SIGTERM", handleSigterm);

let results: Array<{ exitCode: number; shard: number }>;
try {
results = await Promise.all(
shards.map(async ({ proc, shard }) => ({ exitCode: await proc.exited, shard })),
);
} finally {
process.off("SIGINT", handleSigint);
process.off("SIGTERM", handleSigterm);
if (forceKillTimer) clearTimeout(forceKillTimer);
}

if (interruptedExitCode !== null) return interruptedExitCode;
const failedShards = results.filter(({ exitCode }) => exitCode !== 0);

if (failedShards.length > 0) {
console.error(
`Test shard failure: ${failedShards
.map(({ exitCode, shard }) => `${shard}/${shardCount} (exit ${exitCode})`)
.join(", ")}`,
);
return 1;
}

console.error(`All ${shardCount} test shard${shardCount === 1 ? "" : "s"} passed.`);
return 0;
}

if (import.meta.main) {
process.exitCode = await main();
}
7 changes: 3 additions & 4 deletions scripts/verify-pr-release-notes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync
import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, test } from "bun:test";
import { removeTestDirectory } from "../test/helpers/filesystem";
import {
isGeneratedPrereleasePreparation,
isGeneratedReleasePath,
Expand Down Expand Up @@ -88,10 +89,8 @@ function writeGeneratedPrerelease(root: string, initialVersion = "0.17.7") {
runGit(root, ["commit", "--quiet", "-m", "prepare prerelease"]);
}

afterEach(() => {
for (const root of tempRoots.splice(0)) {
rmSync(root, { recursive: true, force: true });
}
afterEach(async () => {
await Promise.all(tempRoots.splice(0).map((root) => removeTestDirectory(root)));
});

describe("isGeneratedReleasePath", () => {
Expand Down
3 changes: 3 additions & 0 deletions src/extensions/discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ describe("manifest api version requirements", () => {
repoRoot: undefined,
globalExtensionsDir: undefined,
flagPaths: [folder],
env: {},
});

expect(candidates).toEqual([
Expand All @@ -503,6 +504,7 @@ describe("manifest api version requirements", () => {
repoRoot: undefined,
globalExtensionsDir: undefined,
flagPaths: [folder],
env: {},
});

expect(candidates).toEqual([
Expand All @@ -524,6 +526,7 @@ describe("manifest api version requirements", () => {
repoRoot: undefined,
globalExtensionsDir: undefined,
flagPaths: [folder],
env: {},
});

expect(candidates).toEqual([{ id: "bad-api-ext", path: entry, origin: "flag" }]);
Expand Down
5 changes: 3 additions & 2 deletions src/ui/components/scrollbar/VerticalScrollbar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ describe("Vertical scrollbar", () => {
contentHeight={40}
theme={theme}
height={10}
hideDelayMs={5}
hideDelayMs={120}
/>,
{ width: 2, height: 10 },
);
Expand All @@ -213,8 +213,9 @@ describe("Vertical scrollbar", () => {
await flush(setup);
expect(frameHasBackground(setup, theme.accentMuted)).toBe(true);

// Keep a wide margin beyond the deadline for Windows CI timer granularity.
await act(async () => {
await Bun.sleep(10);
await Bun.sleep(180);
});
await flush(setup);
expect(frameHasBackground(setup, theme.accentMuted)).toBe(false);
Expand Down
Loading
Loading