Skip to content

Commit 2d03922

Browse files
authored
test: convert loop-generated cases to tables (#9265)
<!-- markdownlint-disable MD041 --> ## Summary Convert 10 loop-generated test groups to Vitest `it.each` tables. This keeps the existing cases and assertions while making each candidate explicit in the test runner and reducing the test-loop growth baseline from 1,377 to 1,367. ## Changes - Convert host redaction, endpoint label, header parity, credential resolution, channel preset, DGX platform, host alias, sandbox drift, and Slack readiness cases to table tests. - Preserve the existing inputs, assertions, setup, cleanup, and behavior-oriented test names. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This is a test-only registration refactor with no user-facing behavior or supported product change. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The change only replaces suite-level test generation with `it.each`; sensitive inputs, assertions, state isolation, and production controls are unchanged. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Reviewed all nine changed test files and all 10 conversions from suite-level generated tests to `it.each`. The conversions preserve behavior-oriented test titles and do not change user-facing NemoClaw behavior or documentation. - Agent: Codex Desktop <!-- docs-review-head-sha: 15b5d30 --> <!-- docs-review-agents-blob-sha: e30afb2 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [ ] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run` for the nine changed files passed 198 tests; `npm run test:titles:check` passed; the scanner reports 1,367 loops, 10 fewer than the baseline. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
1 parent 146643b commit 2d03922

9 files changed

Lines changed: 80 additions & 85 deletions

File tree

nemoclaw/src/onboard/config.test.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -105,24 +105,25 @@ describe("onboard/config", () => {
105105
expect(describeOnboardProvider(config)).toBe("My Custom Provider");
106106
});
107107

108-
const endpointCases: [EndpointType, string][] = [
109-
["build", "NVIDIA Endpoints"],
110-
["openai", "OpenAI"],
111-
["anthropic", "Anthropic"],
112-
["gemini", "Google Gemini"],
113-
["ollama", "Local Ollama"],
114-
["vllm", "Local vLLM"],
115-
["nim-local", "Local NVIDIA NIM"],
116-
["ncp", "NVIDIA Cloud Partner"],
117-
["custom", "Other OpenAI-compatible endpoint"],
108+
const endpointCases: Array<{ endpointType: EndpointType; expected: string }> = [
109+
{ endpointType: "build", expected: "NVIDIA Endpoints" },
110+
{ endpointType: "openai", expected: "OpenAI" },
111+
{ endpointType: "anthropic", expected: "Anthropic" },
112+
{ endpointType: "gemini", expected: "Google Gemini" },
113+
{ endpointType: "ollama", expected: "Local Ollama" },
114+
{ endpointType: "vllm", expected: "Local vLLM" },
115+
{ endpointType: "nim-local", expected: "Local NVIDIA NIM" },
116+
{ endpointType: "ncp", expected: "NVIDIA Cloud Partner" },
117+
{ endpointType: "custom", expected: "Other OpenAI-compatible endpoint" },
118118
];
119119

120-
for (const [endpointType, expected] of endpointCases) {
121-
it(`returns "${expected}" for endpoint type "${endpointType}"`, () => {
120+
it.each(endpointCases)(
121+
'returns "$expected" for endpoint type "$endpointType"',
122+
({ endpointType, expected }) => {
122123
const config = makeConfig({ endpointType, providerLabel: undefined });
123124
expect(describeOnboardProvider(config)).toBe(expected);
124-
});
125-
}
125+
},
126+
);
126127

127128
it("returns Unknown for unsupported endpoint types", () => {
128129
const config = makeConfig({

src/lib/actions/sandbox/host-aliases.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ function expectHostAliasError(action: () => void): HostAliasesCommandError {
4040
}
4141

4242
describe("host alias legacy gateway support checks", () => {
43-
for (const driver of ["docker", "vm"] as const) {
44-
it(`rejects ${driver} driver sandboxes before probing Docker`, () => {
43+
it.each(["docker", "vm"] as const)(
44+
"rejects %s driver sandboxes before probing Docker",
45+
(driver) => {
4546
const probeLegacyGatewayContainer = vi.fn(() => ({ state: "present" as const }));
4647

4748
const error = expectHostAliasError(() =>
@@ -56,8 +57,8 @@ describe("host alias legacy gateway support checks", () => {
5657
);
5758
expect(error.message).toContain(`which the ${driver} driver does not run`);
5859
expect(probeLegacyGatewayContainer).not.toHaveBeenCalled();
59-
});
60-
}
60+
},
61+
);
6162

6263
it("reports a missing legacy gateway distinctly from Docker probe failures", () => {
6364
const error = expectHostAliasError(() =>

src/lib/messaging/channels/slack/hooks/status-health.test.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,9 @@ describe("slack.statusHealth hook", () => {
116116
expect(execute).not.toHaveBeenCalled();
117117
});
118118

119-
for (const [name, account, inputs, expected, signal] of [
119+
it.each([
120120
[
121-
"classifies unavailable Slack credentials as terminal (#7383)",
121+
"unavailable Slack credentials",
122122
{
123123
running: false,
124124
connected: false,
@@ -130,14 +130,15 @@ describe("slack.statusHealth hook", () => {
130130
{ label: "Account probe", severity: "fail" },
131131
],
132132
[
133-
"classifies a Slack plugin probe failure as terminal (#7383)",
133+
"a Slack plugin probe failure",
134134
{ probe: { ok: false, error: "plugin failed to load" } },
135135
{},
136136
["terminal", "plugin", "plugin_probe_failed", false],
137137
{ label: "Account probe", severity: "warn" },
138138
],
139-
] as const) {
140-
it(name, () => {
139+
] as const)(
140+
"classifies %s as terminal (#7383)",
141+
(_condition, account, inputs, expected, signal) => {
141142
const [state, category, reason, retryable] = expected;
142143
const report = runProbe(account, inputs).report;
143144
expect(report.readiness).toMatchObject({
@@ -147,8 +148,8 @@ describe("slack.statusHealth hook", () => {
147148
retryable,
148149
} satisfies Partial<ChannelReadiness>);
149150
expect(report.signals).toContainEqual(expect.objectContaining(signal));
150-
});
151-
}
151+
},
152+
);
152153

153154
it("returns a null transition time for an out-of-range Slack timestamp (#7383)", () => {
154155
expect(

src/lib/onboard/vllm-menu.test.ts

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,24 @@ describe("buildVllmMenuEntries", () => {
3636
assert.match(entries[0].label, /running/);
3737
});
3838

39-
for (const [platform, hostLabel] of [
40-
["spark", "Spark"],
41-
["station", "Station"],
42-
] as const) {
43-
it(`does not mark the running entry experimental on DGX ${hostLabel}`, () => {
44-
const entries = buildVllmMenuEntries({
45-
vllmRunning: true,
46-
vllmProfile: null,
47-
experimental: false,
48-
platform,
49-
hasVllmImage: false,
50-
log: () => {},
51-
env: {},
52-
});
53-
assert.equal(entries.length, 1);
54-
assert.equal(entries[0].key, "vllm");
55-
assert.doesNotMatch(entries[0].label, /experimental/);
56-
assert.match(entries[0].label, /running/);
39+
it.each([
40+
{ platform: "spark", hostLabel: "Spark" },
41+
{ platform: "station", hostLabel: "Station" },
42+
] as const)("does not mark the running entry experimental on DGX $hostLabel", ({ platform }) => {
43+
const entries = buildVllmMenuEntries({
44+
vllmRunning: true,
45+
vllmProfile: null,
46+
experimental: false,
47+
platform,
48+
hasVllmImage: false,
49+
log: () => {},
50+
env: {},
5751
});
58-
}
52+
assert.equal(entries.length, 1);
53+
assert.equal(entries[0].key, "vllm");
54+
assert.doesNotMatch(entries[0].label, /experimental/);
55+
assert.match(entries[0].label, /running/);
56+
});
5957

6058
it("returns the install entry when a profile matches and EXPERIMENTAL is set", () => {
6159
const entries = buildVllmMenuEntries({

src/lib/openshell-sandbox-list.test.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -76,22 +76,20 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => {
7676
vi.restoreAllMocks();
7777
});
7878

79-
for (const [name, issue] of [
80-
["gateway image drift", imageDriftIssue],
81-
["host-process gateway drift", hostProcessDriftIssue],
82-
] as const) {
83-
it(`exits before querying sandbox state for ${name}`, async () => {
84-
mocks.detectPreflightIssue.mockReturnValueOnce(issue);
85-
86-
await expect(captureSandboxListWithGatewayPreflightOrExit(context)).rejects.toThrow(
87-
"process.exit(1)",
88-
);
89-
90-
expect(mocks.printIssue).toHaveBeenCalledWith(issue, context);
91-
expect(mocks.captureOpenshell).not.toHaveBeenCalled();
92-
expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled();
93-
});
94-
}
79+
it.each([
80+
{ name: "gateway image drift", issue: imageDriftIssue },
81+
{ name: "host-process gateway drift", issue: hostProcessDriftIssue },
82+
])("exits before querying sandbox state for $name", async ({ issue }) => {
83+
mocks.detectPreflightIssue.mockReturnValueOnce(issue);
84+
85+
await expect(captureSandboxListWithGatewayPreflightOrExit(context)).rejects.toThrow(
86+
"process.exit(1)",
87+
);
88+
89+
expect(mocks.printIssue).toHaveBeenCalledWith(issue, context);
90+
expect(mocks.captureOpenshell).not.toHaveBeenCalled();
91+
expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled();
92+
});
9593

9694
it("returns the successful sandbox list without gateway recovery", async () => {
9795
const result = await captureSandboxListWithGatewayPreflightOrExit(context);

src/lib/policy/host-redaction.test.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,9 @@ describe("isInternalHost", () => {
8888
"w.localdomain",
8989
];
9090

91-
for (const host of internal) {
92-
it(`treats ${host} as internal`, () => {
93-
expect(isInternalHost(host)).toBe(true);
94-
});
95-
}
91+
it.each(internal)("treats %s as internal", (host) => {
92+
expect(isInternalHost(host)).toBe(true);
93+
});
9694

9795
const external = [
9896
"api.slack.com",
@@ -107,11 +105,9 @@ describe("isInternalHost", () => {
107105
"fec0::1",
108106
"2001:db8::1",
109107
];
110-
for (const host of external) {
111-
it(`treats ${host} as external`, () => {
112-
expect(isInternalHost(host)).toBe(false);
113-
});
114-
}
108+
it.each(external)("treats %s as external", (host) => {
109+
expect(isInternalHost(host)).toBe(false);
110+
});
115111
});
116112

117113
describe("hostStemsFromEndpoints", () => {

src/lib/sandbox/hermes-upstream-header.parity.test.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,11 @@ const FIXTURES: Array<{ name: string; config: Record<string, unknown> }> = [
5050
];
5151

5252
describe("buildHermesUpstreamHeader parity", () => {
53-
for (const fixture of FIXTURES) {
54-
it(`agent and host helpers produce identical output for: ${fixture.name}`, () => {
55-
const agent = buildAgentHeader(fixture.config);
56-
const host = buildHostHeader(fixture.config);
57-
expect(host).toBe(agent);
58-
});
59-
}
53+
it.each(FIXTURES)("agent and host helpers produce identical output for: $name", ({ config }) => {
54+
const agent = buildAgentHeader(config);
55+
const host = buildHostHeader(config);
56+
expect(host).toBe(agent);
57+
});
6058

6159
it("strips newlines and control characters so the comment cannot escape into YAML", () => {
6260
const malicious = {

test/canonical-credential-resolution.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ describe("resolveProviderCredential — canonical credential resolution (#2306)"
9999
},
100100
];
101101

102-
for (const { name, credentialEnv, value } of providers) {
103-
it(`resolves ${credentialEnv} (${name}) from credentials.json when not in env`, async () => {
102+
it.each(providers)(
103+
"resolves $credentialEnv ($name) from credentials.json when not in env",
104+
async ({ credentialEnv, value }) => {
104105
const tmpDir = createFixtureHome(credentialEnv, value);
105106
// Ensure env does NOT have the key
106107
vi.stubEnv(credentialEnv, "");
@@ -111,8 +112,8 @@ describe("resolveProviderCredential — canonical credential resolution (#2306)"
111112

112113
expect(result).toBe(value);
113114
expect(process.env[credentialEnv]).toBe(value);
114-
});
115-
}
115+
},
116+
);
116117

117118
it("returns env value when only in process.env", async () => {
118119
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-2306-envonly-"));

test/channels-add-preset.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -402,8 +402,9 @@ describe("channels add applies a matching policy preset (#3437)", () => {
402402
);
403403
});
404404

405-
for (const channel of ["telegram", "slack", "discord"]) {
406-
it(`applies the '${channel}' preset before triggering rebuild`, async () => {
405+
it.each(["telegram", "slack", "discord"])(
406+
"applies the '%s' preset before triggering rebuild",
407+
async (channel) => {
407408
await addSandboxChannel("test-sb", { channel });
408409

409410
expect(applyPresetSpy).toHaveBeenCalledOnce();
@@ -414,8 +415,8 @@ describe("channels add applies a matching policy preset (#3437)", () => {
414415
expect(callOrder.indexOf(`applyPreset:${channel}`)).toBeLessThan(
415416
callOrder.indexOf("promptAndRebuild"),
416417
);
417-
});
418-
}
418+
},
419+
);
419420

420421
it("applies the tokenless WhatsApp preset for Hermes before triggering rebuild", async () => {
421422
sandboxAgent = "hermes";

0 commit comments

Comments
 (0)