Skip to content

Commit 93c621c

Browse files
kshitij-heizenKshitijBhardwaj18claude
authored
fix(rerun): reject explicit IDs with --all and --status/--skip-terminal without --all (#163)
Two silent-footgun gaps in test rerun's flag validation: 1. Explicit test IDs combined with --all silently discarded the listed IDs — the --all branch resolves the full project test set and overwrites them — so 'rerun test_abc --all' dispatched a batch rerun of EVERY test in the project, burning rerun/auto-heal credits with no error. Both siblings already guard this exact ambiguity (test run's positional+--all guard, delete-batch's ids+--all guard). 2. --status <list> and --skip-terminal without --all were silently ignored — including INVALID --status values, which were never validated — while the same misuse of rerun's own --filter (and delete-batch's --status) exits 5. All three narrowing filters now share the same guard. Both reject with VALIDATION_ERROR (exit 5) before any network dispatch. Fixes #160 Co-authored-by: Kshitij Bhardwaj <tothemoon202154@outlook.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent f732fdb commit 93c621c

2 files changed

Lines changed: 150 additions & 0 deletions

File tree

src/commands/test.rerun.spec.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,126 @@ describe('runTestRerun — validation', () => {
280280
).rejects.toMatchObject({ code: 'VALIDATION_ERROR' });
281281
});
282282

283+
it('exit 5 (VALIDATION_ERROR) when explicit test IDs are combined with --all', async () => {
284+
// The --all branch resolves the FULL project test set and overwrites the
285+
// listed ids, so 'rerun test_abc --all' would silently rerun the ENTIRE
286+
// project instead of test_abc — burning rerun/auto-heal credits. The
287+
// guard throws BEFORE any network/dispatch. (Mirrors `test run`'s
288+
// positional+--all guard and delete-batch's ids+--all guard.)
289+
const creds = makeCreds();
290+
await expect(
291+
runTestRerun(
292+
{
293+
testIds: ['test_abc'],
294+
all: true,
295+
projectId: 'proj_1',
296+
wait: false,
297+
timeoutSeconds: 600,
298+
autoHeal: false,
299+
autoHealExplicit: false,
300+
skipDependencies: false,
301+
maxConcurrency: 10,
302+
output: 'json',
303+
profile: 'default',
304+
dryRun: false,
305+
debug: false,
306+
verbose: false,
307+
},
308+
{ ...creds, sleep: instantSleep },
309+
),
310+
).rejects.toMatchObject({
311+
code: 'VALIDATION_ERROR',
312+
details: expect.objectContaining({ field: 'test-ids' }),
313+
});
314+
});
315+
316+
it('exit 5 (VALIDATION_ERROR) when --status is passed WITHOUT --all', async () => {
317+
// --status is an --all-only narrowing filter. With explicit ids it was
318+
// silently ignored (both tests dispatched, filter dropped) — same
319+
// failure mode as the --filter guard above.
320+
const creds = makeCreds();
321+
await expect(
322+
runTestRerun(
323+
{
324+
testIds: ['test_a', 'test_b'],
325+
all: false,
326+
statusFilter: 'failed',
327+
wait: false,
328+
timeoutSeconds: 600,
329+
autoHeal: false,
330+
autoHealExplicit: false,
331+
skipDependencies: false,
332+
maxConcurrency: 10,
333+
output: 'json',
334+
profile: 'default',
335+
dryRun: false,
336+
debug: false,
337+
verbose: false,
338+
},
339+
{ ...creds, sleep: instantSleep },
340+
),
341+
).rejects.toMatchObject({
342+
code: 'VALIDATION_ERROR',
343+
details: expect.objectContaining({ field: 'status' }),
344+
});
345+
});
346+
347+
it('exit 5 (VALIDATION_ERROR) for an INVALID --status value without --all (was silently accepted)', async () => {
348+
// Before the guard, an invalid --status token without --all was never
349+
// even validated: 'rerun test_a --status notastatus' exited 0 while the
350+
// same flag on delete-batch exits 5.
351+
const creds = makeCreds();
352+
await expect(
353+
runTestRerun(
354+
{
355+
testIds: ['test_a'],
356+
all: false,
357+
statusFilter: 'notastatus',
358+
wait: false,
359+
timeoutSeconds: 600,
360+
autoHeal: false,
361+
autoHealExplicit: false,
362+
skipDependencies: false,
363+
maxConcurrency: 10,
364+
output: 'json',
365+
profile: 'default',
366+
dryRun: false,
367+
debug: false,
368+
verbose: false,
369+
},
370+
{ ...creds, sleep: instantSleep },
371+
),
372+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR' });
373+
});
374+
375+
it('exit 5 (VALIDATION_ERROR) when --skip-terminal is passed WITHOUT --all', async () => {
376+
const creds = makeCreds();
377+
await expect(
378+
runTestRerun(
379+
{
380+
testIds: ['test_a'],
381+
all: false,
382+
skipTerminal: true,
383+
wait: false,
384+
timeoutSeconds: 600,
385+
autoHeal: false,
386+
autoHealExplicit: false,
387+
skipDependencies: false,
388+
maxConcurrency: 10,
389+
output: 'json',
390+
profile: 'default',
391+
dryRun: false,
392+
debug: false,
393+
verbose: false,
394+
},
395+
{ ...creds, sleep: instantSleep },
396+
),
397+
).rejects.toMatchObject({
398+
code: 'VALIDATION_ERROR',
399+
details: expect.objectContaining({ field: 'skip-terminal' }),
400+
});
401+
});
402+
283403
it('exit 5 when --all without --project', async () => {
284404
const creds = makeCreds();
285405
try {

src/commands/test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5980,6 +5980,18 @@ export async function runTestRerun(
59805980
'provide at least one <test-id>, or use --all to rerun all tests in the project',
59815981
);
59825982
}
5983+
// Explicit ids + --all is ambiguous: the --all branch resolves the FULL
5984+
// project test set and overwrites the listed ids, so the user's narrowing
5985+
// intent would be silently replaced by a whole-project batch rerun —
5986+
// burning rerun/auto-heal credits. Reject early. (Mirrors `test run`'s
5987+
// positional+--all guard and delete-batch's ids+--all data-loss guard.)
5988+
if (opts.all && opts.testIds.length > 0) {
5989+
throw localValidationError(
5990+
'test-ids',
5991+
'pass either explicit test IDs or --all, not both — --all reruns every test in the ' +
5992+
'project and would ignore the listed IDs. Drop the IDs, or drop --all.',
5993+
);
5994+
}
59835995
if (opts.all && !opts.projectId) {
59845996
throw localValidationError(
59855997
'project',
@@ -5998,6 +6010,24 @@ export async function runTestRerun(
59986010
'Remove --filter, or add --all --project <id>.',
59996011
);
60006012
}
6013+
// --status and --skip-terminal are --all-only narrowing filters with the
6014+
// same silent-ignore failure mode as --filter above: without --all the
6015+
// explicit ids get reran unfiltered (and an invalid --status value is
6016+
// never even validated). Reject both, mirroring the --filter guard.
6017+
if (opts.statusFilter !== undefined && !opts.all) {
6018+
throw localValidationError(
6019+
'status',
6020+
'--status only applies with --all (it narrows which project tests get reran). ' +
6021+
'Remove --status, or add --all --project <id>.',
6022+
);
6023+
}
6024+
if (opts.skipTerminal && !opts.all) {
6025+
throw localValidationError(
6026+
'skip-terminal',
6027+
'--skip-terminal only applies with --all (it narrows which project tests get reran). ' +
6028+
'Remove --skip-terminal, or add --all --project <id>.',
6029+
);
6030+
}
60016031
if (
60026032
!Number.isInteger(opts.maxConcurrency) ||
60036033
opts.maxConcurrency < 1 ||

0 commit comments

Comments
 (0)