Skip to content

Commit 7d516f2

Browse files
authored
test: migrate remaining test loops (NVIDIA#9402)
<!-- markdownlint-disable MD041 --> ## Summary Turns aggregate loop assertions into independently reported test cases only when each input benefits from its own result. The final diff keeps direct setup and ordered loops and removes unrelated syntax rewrites. ## Changes - Parameterize existing cases in 10 test files for clearer failure names. - Keep ordered setup, retry, polling, and aggregate work in direct loops. - Restore the registry target loop and all syntactic-only changes. - Remove the repeated loop-helper abstractions from the earlier revision. - Limit the final change to 221 additions and 206 deletions. - Record a passing documentation-writer review for `521d9f10a`; no public documentation changes are needed. ## 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: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: Not applicable ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [ ] 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 — GitHub CI is the maintainer-authorized validation path for this repair. - [ ] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — GitHub CI is running for the latest PR commit. - [ ] 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> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved test coverage for Windows-host Docker probing and strict tool-calling modes. * Expanded validation of onboarding progress metadata and reserved dashboard ports. * Added independent coverage for channel presets, CLI compatibility, and command dispatch behavior. * Separated latency configuration tests for defaults, valid values, and invalid inputs. * Strengthened stored-auth transition and migration concern coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
1 parent 2ddf676 commit 7d516f2

10 files changed

Lines changed: 221 additions & 206 deletions

src/lib/inference/onboard-host-docker-internal.test.ts

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -46,42 +46,46 @@ describe("host.docker.internal onboarding inference policy", () => {
4646
expect(result.message).toMatch(/host\.openshell\.internal:11435/);
4747
});
4848

49-
it("recognizes Windows-host Ollama tool calls returned through Docker stdout (#9116)", async () => {
50-
const seenCommands: Array<{ command: string; args: readonly string[] }> = [];
51-
const containerProbeSpawnSyncImpl = (
52-
command: string,
53-
args: readonly string[],
54-
): SpawnSyncReturns<string> => {
55-
seenCommands.push({ command, args });
56-
const body = JSON.stringify({
57-
choices: [
58-
{
59-
message: {
60-
tool_calls: [
61-
{
62-
id: "call_1",
63-
type: "function",
64-
function: { name: "sessions_send", arguments: '{"message":"hello"}' },
65-
},
66-
],
49+
it.each([
50+
["required", true],
51+
["optional", false],
52+
] as const)(
53+
"recognizes Windows-host Ollama tool calls when strict tool calling is %s (#9116)",
54+
async (_label, requireChatCompletionsToolCalling) => {
55+
const seenCommands: Array<{ command: string; args: readonly string[] }> = [];
56+
const containerProbeSpawnSyncImpl = (
57+
command: string,
58+
args: readonly string[],
59+
): SpawnSyncReturns<string> => {
60+
seenCommands.push({ command, args });
61+
const body = JSON.stringify({
62+
choices: [
63+
{
64+
message: {
65+
tool_calls: [
66+
{
67+
id: "call_1",
68+
type: "function",
69+
function: { name: "sessions_send", arguments: '{"message":"hello"}' },
70+
},
71+
],
72+
},
6773
},
68-
},
69-
],
70-
});
71-
const writeOutIndex = args.indexOf("-w");
72-
const writeOut = args[writeOutIndex + 1];
73-
const stdout = `${body}${writeOut.replace("%{http_code}", "200")}`;
74-
return {
75-
pid: 123,
76-
output: [stdout, ""],
77-
stdout,
78-
stderr: "",
79-
status: 0,
80-
signal: null,
74+
],
75+
});
76+
const writeOutIndex = args.indexOf("-w");
77+
const writeOut = args[writeOutIndex + 1];
78+
const stdout = `${body}${writeOut.replace("%{http_code}", "200")}`;
79+
return {
80+
pid: 123,
81+
output: [stdout, ""],
82+
stdout,
83+
stderr: "",
84+
status: 0,
85+
signal: null,
86+
};
8187
};
82-
};
8388

84-
for (const requireChatCompletionsToolCalling of [true, false]) {
8589
const result = await probeOpenAiLikeEndpointOptimized(
8690
"http://host.docker.internal:11434/v1",
8791
"openai/nemotron-mini",
@@ -99,15 +103,15 @@ describe("host.docker.internal onboarding inference policy", () => {
99103
api: "openai-completions",
100104
label: "Chat Completions API",
101105
});
102-
}
103-
expect(seenCommands).toHaveLength(2);
104-
seenCommands.forEach(({ command, args }) => {
105-
expect(command).toBe("docker");
106-
expect(args).toContain("curlimages/curl:8.10.1");
107-
expect(args).toContain("http://host.docker.internal:11434/v1/chat/completions");
108-
expect(args).not.toContain("--volume");
109-
});
110-
});
106+
expect(seenCommands).toHaveLength(1);
107+
seenCommands.forEach(({ command, args }) => {
108+
expect(command).toBe("docker");
109+
expect(args).toContain("curlimages/curl:8.10.1");
110+
expect(args).toContain("http://host.docker.internal:11434/v1/chat/completions");
111+
expect(args).not.toContain("--volume");
112+
});
113+
},
114+
);
111115

112116
it.each([
113117
{

src/lib/onboard/machine/definition.test.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ const expectedStateOrder = [
2828
"failed",
2929
];
3030

31+
const progressStateDefinitions = ONBOARD_MACHINE_STATE_DEFINITIONS.flatMap((definition) =>
32+
"progress" in definition ? [definition] : [],
33+
);
34+
3135
describe("onboard machine definition", () => {
3236
it("is the canonical ordered state catalog", () => {
3337
expect(ONBOARD_MACHINE_STATE_IDS).toEqual(expectedStateOrder);
@@ -76,16 +80,16 @@ describe("onboard machine definition", () => {
7680
expect(ONBOARD_SESSION_STEP_TO_MACHINE_STATE).toEqual(mappingFromDefinitions);
7781
});
7882

79-
it("keeps progress metadata attached only to state-backed steps", () => {
80-
for (const definition of ONBOARD_MACHINE_STATE_DEFINITIONS) {
81-
if (!("progress" in definition)) continue;
83+
it.each(progressStateDefinitions)(
84+
"keeps progress metadata attached to state-backed step $state",
85+
(definition) => {
8286
expect("stepName" in definition).toBe(true);
8387
expect(definition.progress.total).toBe(8);
8488
expect(definition.progress.number).toBeGreaterThanOrEqual(1);
8589
expect(definition.progress.number).toBeLessThanOrEqual(definition.progress.total);
8690
expect(definition.progress.title).not.toHaveLength(0);
87-
}
88-
});
91+
},
92+
);
8993

9094
it("looks up definitions by state", () => {
9195
expect(getOnboardMachineStateDefinition("gateway")).toMatchObject({

src/lib/onboard/machine/progress.test.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,17 @@ import { getOnboardProgressStep, ONBOARD_PROGRESS_STEPS } from "./progress";
1010

1111
vi.mock("../prompt-helpers", () => ({ step: vi.fn() }));
1212

13+
const progressStateDefinitions = ONBOARD_MACHINE_STATE_DEFINITIONS.flatMap((definition) =>
14+
"progress" in definition ? [definition] : [],
15+
);
16+
1317
describe("onboard progress metadata", () => {
14-
it("derives state-backed progress labels from machine definitions", () => {
15-
for (const definition of ONBOARD_MACHINE_STATE_DEFINITIONS) {
16-
if (!("progress" in definition)) continue;
18+
it.each(progressStateDefinitions)(
19+
"derives the $state progress label from its machine definition",
20+
(definition) => {
1721
expect(ONBOARD_PROGRESS_STEPS[definition.stepName]).toEqual(definition.progress);
18-
}
19-
});
22+
},
23+
);
2024

2125
it("preserves the existing eight-step onboarding labels", () => {
2226
expect(ONBOARD_PROGRESS_STEPS).toEqual({

src/lib/onboard/managed-startup-profile.test.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import {
2828
} from "./managed-startup/profile";
2929

3030
const CA_SHA256 = "a".repeat(64);
31+
const HERMES_RESERVED_API_PORTS = [
32+
8_642, 8_643, 8_644, 8_645, 8_646, 8_647, 8_648, 8_649, 8_650, 8_651, 8_652, 18_642,
33+
];
3134

3235
const MESSAGING_PLAN = {
3336
schemaVersion: 1,
@@ -1138,20 +1141,18 @@ describe("managed startup profile", () => {
11381141
).toThrow(/reserved API ports 8642-8652 or 18642/);
11391142
});
11401143

1141-
it.each(
1142-
Array.from([HERMES_API_PORT_RANGE_START - 1, HERMES_API_PORT_RANGE_END + 1], (value) => [
1143-
value,
1144-
]),
1145-
)("rejects port %s outside the reserved Hermes API port range", (port) => {
1146-
for (let port = HERMES_API_PORT_RANGE_START; port <= HERMES_API_PORT_RANGE_END; port += 1) {
1144+
it.each(HERMES_RESERVED_API_PORTS)("rejects reserved Hermes API port %s", (port) => {
11471145
expect(() =>
11481146
validateManagedStartupProfile({
11491147
...HERMES_PROFILE,
11501148
dashboard: { ...HERMES_PROFILE.dashboard, publicPort: port },
11511149
}),
11521150
).toThrow(/reserved API ports/);
1153-
}
1151+
});
11541152

1153+
it.each([HERMES_API_PORT_RANGE_START - 1, HERMES_API_PORT_RANGE_END + 1])(
1154+
"accepts dashboard port %s outside the reserved Hermes API port range",
1155+
(port) => {
11551156
expect(() =>
11561157
validateManagedStartupProfile({
11571158
...HERMES_PROFILE,
@@ -1162,7 +1163,8 @@ describe("managed startup profile", () => {
11621163
},
11631164
}),
11641165
).not.toThrow();
1165-
});
1166+
},
1167+
);
11661168

11671169
it.each([
11681170
"-----BEGIN CERTIFICATE-----\nMIIB\n-----END CERTIFICATE-----",

test/channels-add-preset.test.ts

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -861,19 +861,15 @@ describe("channels add verifies bridge startup after rebuild (#4314, #4390)", ()
861861
});
862862

863863
describe("channel preset source-of-truth", () => {
864-
it("every channel registered in KNOWN_CHANNELS ships a preset YAML that parsePresetPolicyKeys() accepts", () => {
865-
const failures: string[] = [];
866-
for (const name of knownChannelNames()) {
864+
it.each(knownChannelNames())(
865+
"channel $name ships a preset that parsePresetPolicyKeys accepts",
866+
(name) => {
867867
const content = policies.loadPreset(name);
868-
if (content === null) {
869-
failures.push(`${name}: preset YAML not found on disk`);
870-
continue;
871-
}
872-
if (policies.parsePresetPolicyKeys(content).length === 0) {
873-
failures.push(`${name}: parsePresetPolicyKeys returned no entries`);
874-
}
875-
}
876-
877-
expect(failures).toEqual([]);
878-
});
868+
expect(content, `${name}: preset YAML not found on disk`).not.toBeNull();
869+
expect(
870+
policies.parsePresetPolicyKeys(content!).length,
871+
`${name}: parsePresetPolicyKeys returned no entries`,
872+
).toBeGreaterThan(0);
873+
},
874+
);
879875
});

test/cli-oclif-compatibility.test.ts

Lines changed: 57 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -481,30 +481,24 @@ describe("oclif compatibility dispatch", () => {
481481
}
482482
});
483483

484-
it("corrects a single sandbox-like global status argument without a CLI subprocess", async () => {
485-
await withDirectPublicDispatch(
486-
async ({ dispatchCli, exitSpy, runOclifArgv, runOclifCommandById, stderr }) => {
487-
const cases = [
488-
{ argv: ["status", "alpha"], command: "nemoclaw alpha status" },
489-
{ argv: ["status", "--json", "alpha"], command: "nemoclaw alpha status --json" },
490-
{ argv: ["status", "alpha", "--json"], command: "nemoclaw alpha status --json" },
491-
{ argv: ["status", "alpha", "--help"], command: "nemoclaw alpha status --help" },
492-
{
493-
argv: ["status", "alpha", "--json", "--help"],
494-
command: "nemoclaw alpha status --help",
495-
},
496-
{
497-
argv: ["status", "alpha", "--help", "--json"],
498-
command: "nemoclaw alpha status --help",
499-
},
500-
];
501-
502-
for (const { argv, command } of cases) {
503-
stderr.length = 0;
504-
exitSpy.mockClear();
505-
runOclifArgv.mockClear();
506-
runOclifCommandById.mockClear();
507-
484+
it.each([
485+
{ argv: ["status", "alpha"], command: "nemoclaw alpha status" },
486+
{ argv: ["status", "--json", "alpha"], command: "nemoclaw alpha status --json" },
487+
{ argv: ["status", "alpha", "--json"], command: "nemoclaw alpha status --json" },
488+
{ argv: ["status", "alpha", "--help"], command: "nemoclaw alpha status --help" },
489+
{
490+
argv: ["status", "alpha", "--json", "--help"],
491+
command: "nemoclaw alpha status --help",
492+
},
493+
{
494+
argv: ["status", "alpha", "--help", "--json"],
495+
command: "nemoclaw alpha status --help",
496+
},
497+
])(
498+
"corrects a single sandbox-like global status argument to $command",
499+
async ({ argv, command }) => {
500+
await withDirectPublicDispatch(
501+
async ({ dispatchCli, exitSpy, runOclifArgv, runOclifCommandById, stderr }) => {
508502
await expect(dispatchCli(argv)).rejects.toThrow("process.exit:2");
509503

510504
const output = stderr.join("\n");
@@ -514,32 +508,26 @@ describe("oclif compatibility dispatch", () => {
514508
expect(exitSpy).toHaveBeenCalledWith(2);
515509
expect(runOclifArgv).not.toHaveBeenCalled();
516510
expect(runOclifCommandById).not.toHaveBeenCalled();
517-
}
518-
},
519-
);
520-
});
521-
522-
it("leaves ambiguous or unsafe global status arguments to the strict parser", async () => {
523-
await withDirectPublicDispatch(
524-
async ({ dispatchCli, exitSpy, runOclifArgv, runOclifCommandById, stderr }) => {
525-
const cases = [
526-
["status", "--bogus"],
527-
["status", "--bogus", "alpha"],
528-
["status", "alpha", "--bogus"],
529-
["status", "alpha", "beta"],
530-
["status", "status"],
531-
["status", "help"],
532-
["status", "sandbox"],
533-
["status", "internal"],
534-
["status", "alpha;echo pwned"],
535-
];
536-
537-
for (const argv of cases) {
538-
stderr.length = 0;
539-
exitSpy.mockClear();
540-
runOclifArgv.mockClear();
541-
runOclifCommandById.mockClear();
511+
},
512+
);
513+
},
514+
);
542515

516+
it.each([
517+
["status", "--bogus"],
518+
["status", "--bogus", "alpha"],
519+
["status", "alpha", "--bogus"],
520+
["status", "alpha", "beta"],
521+
["status", "status"],
522+
["status", "help"],
523+
["status", "sandbox"],
524+
["status", "internal"],
525+
["status", "alpha;echo pwned"],
526+
])(
527+
"leaves ambiguous or unsafe global status arguments to the strict parser [%j]",
528+
async (...argv) => {
529+
await withDirectPublicDispatch(
530+
async ({ dispatchCli, exitSpy, runOclifArgv, runOclifCommandById, stderr }) => {
543531
await dispatchCli(argv);
544532

545533
expect(runOclifCommandById).toHaveBeenCalledWith(
@@ -551,30 +539,35 @@ describe("oclif compatibility dispatch", () => {
551539
expect(exitSpy).not.toHaveBeenCalled();
552540
expect(stderr.join("\n")).not.toContain("does not take a sandbox name");
553541
expect(stderr.join("\n")).not.toContain("Run:");
554-
}
555-
},
556-
);
557-
});
558-
559-
it.each(["status", "help", "sandbox", "internal", "alpha;echo pwned"])(
560-
"keeps strict status parser errors in process [%s]",
561-
async (token) => {
562-
for (const args of [["--bogus"], ["--bogus", "alpha"], ["alpha", "--bogus"]]) {
563-
await expect(StatusCommand.run(args, process.cwd())).rejects.toThrow(
564-
"Nonexistent flag: --bogus",
565-
);
566-
}
542+
},
543+
);
544+
},
545+
);
567546

568-
await expect(StatusCommand.run(["alpha", "beta"], process.cwd())).rejects.toThrow(
569-
"Unexpected arguments: alpha, beta",
547+
it.each([["--bogus"], ["--bogus", "alpha"], ["alpha", "--bogus"]])(
548+
"keeps strict status flag errors in process [%j]",
549+
async (...args) => {
550+
await expect(StatusCommand.run(args, process.cwd())).rejects.toThrow(
551+
"Nonexistent flag: --bogus",
570552
);
553+
},
554+
);
571555

556+
it.each(["status", "help", "sandbox", "internal", "alpha;echo pwned"])(
557+
"keeps strict status argument errors in process [%s]",
558+
async (token) => {
572559
await expect(StatusCommand.run([token], process.cwd())).rejects.toThrow(
573560
`Unexpected argument: ${token}`,
574561
);
575562
},
576563
);
577564

565+
it("keeps multiple strict status arguments in process", async () => {
566+
await expect(StatusCommand.run(["alpha", "beta"], process.cwd())).rejects.toThrow(
567+
"Unexpected arguments: alpha, beta",
568+
);
569+
});
570+
578571
it("routes sandbox status help directly and keeps its JSON help metadata", async () => {
579572
await withDirectPublicDispatch(async ({ dispatchCli, runOclifArgv, runOclifCommandById }) => {
580573
await dispatchCli(["alpha", "status", "--help"]);

0 commit comments

Comments
 (0)