Skip to content

Commit 243ac5a

Browse files
authored
test: convert next 200 test loops to tables (#9349)
<!-- markdownlint-disable MD041 --> ## Summary Convert another 200 existing test loops into independently reported table tests. The test-loop scanner moves from 943 findings on the merged baseline to 743 without adding named-function callback exemptions. ## Changes - Replace input, policy, environment, workflow, and security-case loops with inline `it.each` or `describe.each` registrations across 150 test files. - Keep runtime-output assertions as single behavioral tests when their rows do not exist until the test runs. - Lower the legacy size budgets for two test files that became shorter. ## 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: - [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: Test-only registration refactor; negative, redaction, and source-shape contract assertions remain intact, credential-shaped fixtures stay out of titles, and the independent documentation-writer review passed on commit `ae3af882a`. - [ ] 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 - [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 <all 150 changed test files>`: 4,993 passed, 5 skipped; annotation-fix replay: 44 passed; title-quality replay: 1,260 passed, 1 skipped - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — the full Vitest run was attempted but did not pass on this host: unchanged Docker, external gateway, DGX, and GPU host-qualification tests failed; the run then reached the known long-running integration tail and was terminated. The explicit changed-file run passed. - [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) Additional validation: - `npm run typecheck:cli` - `npm --prefix nemoclaw run typecheck` - `npm run checks:repository` - `npm run source-shape:check` - `npm run test:titles:check` - `npm run test-size:check` - `npm run test-loops:scan -- --top 3`: 2,428 files, 743 loops in 372 files - Normal `pre-commit` and `commit-msg` hooks passed - Documentation writer review: PASS, no docs needed; reviewer `/root/documentation_writer_review_round_seven`, commit `ae3af882a` --- 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 security, validation, redaction, path safety, credential handling, runtime behavior, and workflow boundaries. * Converted numerous repetitive checks into independently reported parameterized cases, making failures easier to identify. * Preserved existing behavior and expectations while improving test isolation and cleanup. * **Chores** * Updated test file-size budgets to reflect the revised test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
1 parent e746431 commit 243ac5a

151 files changed

Lines changed: 6256 additions & 5949 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

ci/test-file-size-budget.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
"src/lib/onboard/preflight.test.ts": 1875,
88
"test/generate-openclaw-config.test.ts": 1907,
99
"test/install-preflight.test.ts": 3025,
10-
"test/nemoclaw-start.test.ts": 4789,
10+
"test/nemoclaw-start.test.ts": 4785,
1111
"test/onboard-messaging.test.ts": 2033,
12-
"test/onboard-selection.test.ts": 4178
12+
"test/onboard-selection.test.ts": 4177
1313
}
1414
}

nemoclaw/src/blueprint/runner.test.ts

Lines changed: 40 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,15 @@ describe("runner", () => {
848848
expect(plan.timestamp).toBeDefined();
849849
});
850850

851-
it("persists only the explicit safe plan schema", async () => {
851+
it.each([
852+
"credential_env",
853+
"credential_default",
854+
"SECRET_KEY",
855+
"default-secret-value",
856+
"real-secret",
857+
"future-token-value",
858+
"future-authorization",
859+
])("persists only the explicit safe plan schema [%s]", async (leaked) => {
852860
const bp = {
853861
components: {
854862
inference: {
@@ -902,17 +910,8 @@ describe("runner", () => {
902910
endpoint: "https://api.example.com",
903911
model: "gpt-4",
904912
});
905-
for (const leaked of [
906-
"credential_env",
907-
"credential_default",
908-
"SECRET_KEY",
909-
"default-secret-value",
910-
"real-secret",
911-
"future-token-value",
912-
"future-authorization",
913-
]) {
914-
expect(entry.content).not.toContain(leaked);
915-
}
913+
914+
expect(entry.content).not.toContain(leaked);
916915
});
917916

918917
it("emits all progress milestones", async () => {
@@ -1184,7 +1183,19 @@ describe("runner", () => {
11841183
expect(stdoutText()).toContain('"nc-run-1"');
11851184
});
11861185

1187-
it("re-renders only safe allowlisted fields from plan.json", () => {
1186+
it.each([
1187+
"credential_env",
1188+
"credential_default",
1189+
"SECRET_KEY",
1190+
"default-secret-value",
1191+
"future-token-value",
1192+
"future-authorization",
1193+
"sandbox-token-value",
1194+
"router-authorization",
1195+
"top-level-token-value",
1196+
"top-level-authorization",
1197+
"future-api-key",
1198+
])("re-renders only safe allowlisted fields from plan.json [%s]", (leaked) => {
11881199
const rid = "nc-run-sensitive";
11891200
addDir(`${RUNS_DIR}/${rid}`);
11901201
addFile(
@@ -1253,21 +1264,8 @@ describe("runner", () => {
12531264
dry_run: false,
12541265
});
12551266
const out = stdoutText();
1256-
for (const leaked of [
1257-
"credential_env",
1258-
"credential_default",
1259-
"SECRET_KEY",
1260-
"default-secret-value",
1261-
"future-token-value",
1262-
"future-authorization",
1263-
"sandbox-token-value",
1264-
"router-authorization",
1265-
"top-level-token-value",
1266-
"top-level-authorization",
1267-
"future-api-key",
1268-
]) {
1269-
expect(out).not.toContain(leaked);
1270-
}
1267+
1268+
expect(out).not.toContain(leaked);
12711269
});
12721270

12731271
it("prints unknown status when plan.json is missing", () => {
@@ -1288,17 +1286,14 @@ describe("runner", () => {
12881286

12891287
// ── Path traversal rejection ──────────────────────────────────
12901288

1291-
it.each([
1292-
"../../etc",
1293-
"../tmp",
1294-
"valid.with.dots",
1295-
"foo\x00bar",
1296-
"/absolute/path",
1297-
])("rejects malicious run ID: %j", (rid) => {
1298-
expect(() => {
1299-
actionStatus(rid);
1300-
}).toThrow(/Invalid run ID/);
1301-
});
1289+
it.each(["../../etc", "../tmp", "valid.with.dots", "foo\x00bar", "/absolute/path"])(
1290+
"rejects malicious run ID: %j",
1291+
(rid) => {
1292+
expect(() => {
1293+
actionStatus(rid);
1294+
}).toThrow(/Invalid run ID/);
1295+
},
1296+
);
13021297

13031298
it("accepts a legitimate hyphenated run ID", () => {
13041299
const rid = "nc-20260406-abc12345";
@@ -1354,16 +1349,12 @@ describe("runner", () => {
13541349

13551350
// ── Path traversal rejection ──────────────────────────────────
13561351

1357-
it.each([
1358-
"../../etc",
1359-
"../tmp",
1360-
"valid.with.dots",
1361-
"foo\x00bar",
1362-
"/absolute/path",
1363-
"",
1364-
])("rejects malicious run ID: %j", async (rid) => {
1365-
await expect(actionRollback(rid)).rejects.toThrow(/Invalid run ID/);
1366-
});
1352+
it.each(["../../etc", "../tmp", "valid.with.dots", "foo\x00bar", "/absolute/path", ""])(
1353+
"rejects malicious run ID: %j",
1354+
async (rid) => {
1355+
await expect(actionRollback(rid)).rejects.toThrow(/Invalid run ID/);
1356+
},
1357+
);
13671358

13681359
it("throws when rollback plan has no sandbox_name", async () => {
13691360
const runDir = `${RUNS_DIR}/nc-run-1`;

nemoclaw/src/security/snapshot-sanitizer-failure.test.ts

Lines changed: 75 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,49 @@ describe("migration snapshot sanitizer fallbacks", () => {
8484
mtimeNs: "3",
8585
ctimeNs: "4",
8686
};
87+
const malformedDescriptorOutputs = [
88+
{ label: "non-JSON output", output: "not-json" },
89+
{
90+
label: "array directories",
91+
output: JSON.stringify({ root: identity, directories: [], files: [] }),
92+
},
93+
{
94+
label: "escaping directory path",
95+
output: JSON.stringify({
96+
root: identity,
97+
directories: { "nested\\escape": identity },
98+
files: [],
99+
}),
100+
},
101+
{
102+
label: "null file",
103+
output: JSON.stringify({ root: identity, directories: {}, files: [null] }),
104+
},
105+
{
106+
label: "absolute file path",
107+
output: JSON.stringify({
108+
root: identity,
109+
directories: {},
110+
files: [{ path: "/escape", metadata: identity }],
111+
}),
112+
},
113+
{
114+
label: "null file metadata",
115+
output: JSON.stringify({
116+
root: identity,
117+
directories: {},
118+
files: [{ path: "config.json", metadata: null }],
119+
}),
120+
},
121+
{
122+
label: "non-string file content",
123+
output: JSON.stringify({
124+
root: identity,
125+
directories: {},
126+
files: [{ path: "config.json", metadata: identity, content: 42 }],
127+
}),
128+
},
129+
];
87130

88131
it("fails closed when the descriptor helper is unavailable", () => {
89132
const configPath = path.join(makeRoot(), "openclaw.json");
@@ -285,35 +328,14 @@ describe("migration snapshot sanitizer fallbacks", () => {
285328
expect(readFileSync(configPath, "utf-8")).toBe(original);
286329
});
287330

288-
it("rejects every malformed descriptor scan boundary", () => {
289-
const root = { canonicalPath: makeRoot(), identity };
290-
const malformedOutputs = [
291-
"not-json",
292-
JSON.stringify({ root: identity, directories: [], files: [] }),
293-
JSON.stringify({ root: identity, directories: { "nested\\escape": identity }, files: [] }),
294-
JSON.stringify({ root: identity, directories: {}, files: [null] }),
295-
JSON.stringify({
296-
root: identity,
297-
directories: {},
298-
files: [{ path: "/escape", metadata: identity }],
299-
}),
300-
JSON.stringify({
301-
root: identity,
302-
directories: {},
303-
files: [{ path: "config.json", metadata: null }],
304-
}),
305-
JSON.stringify({
306-
root: identity,
307-
directories: {},
308-
files: [{ path: "config.json", metadata: identity, content: 42 }],
309-
}),
310-
];
311-
312-
for (const output of malformedOutputs) {
331+
it.each(malformedDescriptorOutputs)(
332+
"rejects a malformed descriptor with $label",
333+
({ output }) => {
334+
const root = { canonicalPath: makeRoot(), identity };
313335
writePythonWrapper([`printf '%s\\n' ${shellQuote(output)}`]);
314336
expect(scanDescriptorSnapshot(root, new Set())).toBeNull();
315-
}
316-
});
337+
},
338+
);
317339

318340
it("rejects unsafe roots and non-canonical helper payloads", () => {
319341
const root = makeRoot();
@@ -349,47 +371,40 @@ describe("migration snapshot sanitizer fallbacks", () => {
349371
expect(decodeDescriptorSnapshotContent(oversized)).toBeNull();
350372
});
351373

352-
it("preserves canonical base64 and UTF-8 boundary rules", () => {
353-
expect(decodeDescriptorSnapshotContent("")).toBe("");
354-
expect(decodeDescriptorSnapshotContent("Zg==")).toBe("f");
355-
expect(decodeDescriptorSnapshotContent("Zm8=")).toBe("fo");
356-
expect(decodeDescriptorSnapshotContent("Zm9v")).toBe("foo");
357-
expect(decodeDescriptorSnapshotContent("aGVsbG8=")).toBe("hello");
358-
for (const rejected of [
359-
"A",
360-
"AAAAA",
361-
"AA=A",
362-
"A===",
363-
"====",
364-
"YWJj=",
365-
"AB==",
366-
"AAB=",
367-
"/w==",
368-
"YWJj\n",
369-
]) {
374+
it.each(["A", "AAAAA", "AA=A", "A===", "====", "YWJj=", "AB==", "AAB=", "/w==", "YWJj\n"])(
375+
"preserves canonical base64 and UTF-8 boundary rules [%s]",
376+
(rejected) => {
377+
expect(decodeDescriptorSnapshotContent("")).toBe("");
378+
expect(decodeDescriptorSnapshotContent("Zg==")).toBe("f");
379+
expect(decodeDescriptorSnapshotContent("Zm8=")).toBe("fo");
380+
expect(decodeDescriptorSnapshotContent("Zm9v")).toBe("foo");
381+
expect(decodeDescriptorSnapshotContent("aGVsbG8=")).toBe("hello");
382+
370383
expect(decodeDescriptorSnapshotContent(rejected)).toBeNull();
371-
}
372-
});
384+
},
385+
);
386+
387+
it.each(["AB==", "AAB="])(
388+
"rejects non-canonical base64 at the descriptor apply boundary [%s]",
389+
(content) => {
390+
const rootPath = makeRoot();
391+
const configPath = path.join(rootPath, "config.json");
392+
writeFileSync(configPath, "original");
393+
const root = inspectDescriptorSnapshotRoot(rootPath)!;
394+
const scan = scanDescriptorSnapshot(root, new Set())!;
395+
const config = scan.files.find((file) => file.path === "config.json")!;
396+
397+
expect(scan).not.toBeNull();
398+
expect(config).toBeDefined();
373399

374-
it("rejects non-canonical base64 at the descriptor apply boundary", () => {
375-
const rootPath = makeRoot();
376-
const configPath = path.join(rootPath, "config.json");
377-
writeFileSync(configPath, "original");
378-
const root = inspectDescriptorSnapshotRoot(rootPath)!;
379-
const scan = scanDescriptorSnapshot(root, new Set())!;
380-
const config = scan.files.find((file) => file.path === "config.json")!;
381-
382-
expect(scan).not.toBeNull();
383-
expect(config).toBeDefined();
384-
for (const content of ["AB==", "AAB="]) {
385400
expect(
386401
applyDescriptorSnapshotActions(root, scan, [
387402
{ kind: "replace", path: config.path, metadata: config.metadata, content },
388403
]),
389404
).toBe(false);
390405
expect(readFileSync(configPath, "utf-8")).toBe("original");
391-
}
392-
});
406+
},
407+
);
393408

394409
it("fails closed when sanitized output cannot be installed", () => {
395410
const configPath = path.join(makeRoot(), "openclaw.json");

src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,10 +211,14 @@ describe("requestedAgentTimeoutSeconds", () => {
211211
expect(agentDispatchDeadlineSeconds(argv)).toBeUndefined();
212212
});
213213

214-
it("refuses a value that cannot be a deadline (#8723)", () => {
215-
for (const raw of ["-5", "1.5", "abc", "", "1e3"]) {
214+
it.each(["-5", "1.5", "abc", "", "1e3"])(
215+
"refuses a value that cannot be a deadline: %j (#8723)",
216+
(raw) => {
216217
expect(requestedAgentTimeoutSeconds(agent("--timeout", raw))).toBeNull();
217-
}
218+
},
219+
);
220+
221+
it("refuses a missing deadline value (#8723)", () => {
218222
expect(requestedAgentTimeoutSeconds(agent("--timeout"))).toBeNull();
219223
});
220224
});
@@ -241,7 +245,9 @@ describe("agentDispatchDeadlineSeconds", () => {
241245
);
242246
// The buffer would round past the ceiling, so the argv would carry a
243247
// deadline that differs from the one the caller asked for.
244-
expect(agentDispatchDeadlineSeconds(["openclaw", "agent", "--timeout", ceiling])).toBeUndefined();
248+
expect(
249+
agentDispatchDeadlineSeconds(["openclaw", "agent", "--timeout", ceiling]),
250+
).toBeUndefined();
245251
});
246252

247253
it("still bounds the largest deadline that survives the buffer (#8723)", () => {

src/lib/actions/sandbox/auto-pair-approval-script.test.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,16 @@ describe("buildAutoPairApprovalScript (#4263/#4616)", () => {
109109
expect(module).not.toContain("recover_failed_scope_approval");
110110
});
111111

112-
it("accepts exactly one terminal fixed receipt", () => {
112+
it.each(
113+
Array.from(
114+
[
115+
`${RECEIPT_MARKER}=approved-one\nlater output\n`,
116+
`${RECEIPT_MARKER}=approve-failed\n${RECEIPT_MARKER}=approved-one\n`,
117+
`${RECEIPT_MARKER}=raw-request-id\n`,
118+
],
119+
(value) => [value],
120+
),
121+
)("accepts exactly one terminal fixed receipt [case %#]", (output) => {
113122
for (const receipt of [
114123
"approved-one",
115124
"list-failed",
@@ -136,12 +145,7 @@ describe("buildAutoPairApprovalScript (#4263/#4616)", () => {
136145
parseAutoPairApprovalReceipt(`ignored setup output\n${RECEIPT_MARKER}=${receipt}\n`),
137146
).toBe(receipt);
138147
}
139-
for (const output of [
140-
`${RECEIPT_MARKER}=approved-one\nlater output\n`,
141-
`${RECEIPT_MARKER}=approve-failed\n${RECEIPT_MARKER}=approved-one\n`,
142-
`${RECEIPT_MARKER}=raw-request-id\n`,
143-
]) {
144-
expect(parseAutoPairApprovalReceipt(output)).toBeNull();
145-
}
148+
149+
expect(parseAutoPairApprovalReceipt(output)).toBeNull();
146150
});
147151
});

0 commit comments

Comments
 (0)