Skip to content
Merged
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: 1 addition & 1 deletion DOCUMENTATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ testsprite test create --plan-from ./checkout.plan.json --dry-run --output json

#### `testsprite test create-batch`

Bulk-create frontend tests from a JSONL plan-steps file (or a directory of plan files with `--plan-from-dir`). Optional `--run --max-concurrency <N>` fans out triggers.
Bulk-create frontend tests from a JSONL plan-steps file (or a directory of plan files with `--plan-from-dir`). Optional `--run --max-concurrency <N>` fans out triggers. Without `--wait`, each run is dispatched (`status: "queued"`) and the command exits 0 when every trigger is accepted — mirroring single `test run` without `--wait`; a trigger error still exits non-zero. With `--wait`, it polls every run to terminal and exits non-zero if any run does not pass.

```bash
testsprite test create-batch --plans ./plans.jsonl --run --max-concurrency 4 --output json
Expand Down
219 changes: 219 additions & 0 deletions src/commands/test.create-batch-run.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,225 @@ describe('runCreateBatch --run --wait: mixed outcomes', () => {
});
});

// ---------------------------------------------------------------------------
// No --wait: exit-code contract (issue #161)
//
// Without --wait, every trigger response is non-terminal ('queued') by design.
// A fully successful dispatch must exit 0 — success means every trigger was
// dispatched without error, mirroring single `test run` (no --wait). A trigger
// error must still exit non-zero. Existing no-wait specs only exercised error
// scenarios and never asserted the success exit code, which is how the
// "always exits 1 even when every trigger succeeds" bug slipped through.
// ---------------------------------------------------------------------------

describe('runCreateBatch --run (no --wait): exit-code contract', () => {
let logSpy: ReturnType<typeof vi.spyOn>;
let errorSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
});

afterEach(() => {
logSpy.mockRestore();
errorSpy.mockRestore();
});

it('all triggers succeed (queued) → resolves without error, exit 0; results all queued, no error (json)', async () => {
const { credentialsPath } = makeCreds();
const testIds = ['test_q1', 'test_q2', 'test_q3'];
const plansFile = writePlansJsonl([FE_SPEC, FE_SPEC, FE_SPEC]);

let pollCount = 0;
const fetchImpl = makeFetch(url => {
if (url.includes('/tests/batch')) {
return { body: makeBatchCreateResponse(testIds) };
}
const triggerMatch = /\/tests\/(test_[a-z0-9]+)\/runs$/.exec(url);
if (triggerMatch?.[1]) {
const testId = triggerMatch[1];
return { body: makeTriggerResponse(testId, `run_${testId}`) };
}
// No --wait must NOT poll — count any GET /runs as a violation.
if (/\/runs\/run_test_[a-z0-9]+/.exec(url)) {
pollCount++;
}
return {
status: 404,
body: {
error: { code: 'NOT_FOUND', message: 'not found', nextAction: '', requestId: 'r1' },
},
};
});

const stdout: string[] = [];
const stderrLines: string[] = [];

// Must NOT throw — a fully successful no-wait dispatch exits 0.
// (If runBatchRun threw a CLIError, this await would reject and fail the test.)
await runCreateBatch(
{
profile: 'default',
output: 'json',
debug: false,
dryRun: false,
plans: plansFile,
run: true,
wait: false,
timeoutSeconds: 60,
},
{
credentialsPath,
fetchImpl,
stdout: line => stdout.push(line),
stderr: line => stderrLines.push(line),
sleep: instantSleep,
},
);

expect(pollCount).toBe(0); // no polling without --wait

const printed = JSON.parse(stdout.join('')) as {
results: Array<{ testId: string; status: string; error?: unknown }>;
};
expect(printed.results).toHaveLength(3);
expect(printed.results.every(r => r.status === 'queued')).toBe(true);
expect(printed.results.every(r => r.error === undefined)).toBe(true);
});

it('all triggers succeed (queued) → text summary reports "3/3 triggered", exit 0 (no "0/N passed")', async () => {
const { credentialsPath } = makeCreds();
const testIds = ['test_t1', 'test_t2', 'test_t3'];
const plansFile = writePlansJsonl([FE_SPEC, FE_SPEC, FE_SPEC]);

const fetchImpl = makeFetch(url => {
if (url.includes('/tests/batch')) {
return { body: makeBatchCreateResponse(testIds) };
}
const triggerMatch = /\/tests\/(test_[a-z0-9]+)\/runs$/.exec(url);
if (triggerMatch?.[1]) {
const testId = triggerMatch[1];
return { body: makeTriggerResponse(testId, `run_${testId}`) };
}
return {
status: 404,
body: {
error: { code: 'NOT_FOUND', message: 'not found', nextAction: '', requestId: 'r1' },
},
};
});

const stdout: string[] = [];
const stderrLines: string[] = [];

// Must NOT throw — a fully successful no-wait dispatch exits 0.
await runCreateBatch(
{
profile: 'default',
output: 'text',
debug: false,
dryRun: false,
plans: plansFile,
run: true,
wait: false,
timeoutSeconds: 60,
},
{
credentialsPath,
fetchImpl,
stdout: line => stdout.push(line),
stderr: line => stderrLines.push(line),
sleep: instantSleep,
},
);

const summary = stderrLines.find(l => l.startsWith('batch-run summary:'));
expect(summary).toBeDefined();
expect(summary).toContain('3/3 triggered');
// The pre-fix bug printed a pass/fail summary ("0/3 passed") for a fully
// successful no-wait dispatch — assert that misleading wording is gone.
expect(summary).not.toContain('passed');
expect(stderrLines.some(l => l.includes('did not pass'))).toBe(false);
});

it('partial trigger failure (2 queued, 1 errors) → still exits non-zero; all 3 results in envelope', async () => {
const { credentialsPath } = makeCreds();
const testIds = ['test_p1', 'test_p2', 'test_p3'];
const plansFile = writePlansJsonl([FE_SPEC, FE_SPEC, FE_SPEC]);

// test_p3's trigger returns 404 NOT_FOUND — a non-retryable error (exit 4)
// that surfaces immediately as an error result. The other two dispatch fine.
const fetchImpl = makeFetch(url => {
if (url.includes('/tests/batch')) {
return { body: makeBatchCreateResponse(testIds) };
}
const triggerMatch = /\/tests\/(test_[a-z0-9]+)\/runs$/.exec(url);
if (triggerMatch?.[1]) {
const testId = triggerMatch[1];
if (testId === 'test_p3') {
return {
status: 404,
body: {
error: {
code: 'NOT_FOUND',
message: 'test not found',
nextAction: '',
requestId: 'req_p3',
},
},
};
}
return { body: makeTriggerResponse(testId, `run_${testId}`) };
}
return {
status: 404,
body: {
error: { code: 'NOT_FOUND', message: 'not found', nextAction: '', requestId: 'r1' },
},
};
});

const stdout: string[] = [];
const stderrLines: string[] = [];

const err = await runCreateBatch(
{
profile: 'default',
output: 'json',
debug: false,
dryRun: false,
plans: plansFile,
run: true,
wait: false,
timeoutSeconds: 60,
},
{
credentialsPath,
fetchImpl,
stdout: line => stdout.push(line),
stderr: line => stderrLines.push(line),
sleep: instantSleep,
},
).catch(e => e);

// A dispatch with any trigger error must exit non-zero.
expect(err).toBeInstanceOf(CLIError);
expect((err as CLIError).exitCode).not.toBe(0);

const printed = JSON.parse(stdout.join('')) as {
results: Array<{ testId: string; status: string; error?: { code: string } }>;
};
expect(printed.results).toHaveLength(3);
const queued = printed.results.filter(r => r.status === 'queued');
const errored = printed.results.filter(r => r.error !== undefined);
expect(queued).toHaveLength(2);
expect(errored).toHaveLength(1);
expect(errored[0]?.testId).toBe('test_p3');
expect(errored[0]?.error?.code).toBe('NOT_FOUND');
});
});

// ---------------------------------------------------------------------------
// --max-concurrency: verify only N in-flight at any time
// ---------------------------------------------------------------------------
Expand Down
44 changes: 30 additions & 14 deletions src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2888,6 +2888,17 @@ async function runBatchRun(
})
: batchRunResults;
out.print({ results: enrichedResults });
} else if (!opts.wait) {
// Text mode, no --wait: statuses are non-terminal by design ('queued'),
// so a pass/fail summary would misread a successful dispatch as
// "0/N passed". Report what actually happened: triggers.
const total = batchRunResults.length;
const erroredCount = batchRunResults.filter(r => r.error !== undefined).length;
const parts = [`${total - erroredCount}/${total} triggered`];
if (erroredCount > 0) {
parts.push(`${erroredCount} trigger error${erroredCount !== 1 ? 's' : ''}`);
}
stderrFn(`batch-run summary: ${parts.join(', ')}`);
} else {
// Text mode: print summary line.
const passed = batchRunResults.filter(r => r.status === 'passed').length;
Expand All @@ -2906,29 +2917,32 @@ async function runBatchRun(
stderrFn(`batch-run summary: ${parts.join(', ')}`);
}

// Determine exit code.
const allPassed = batchRunResults.every(r => r.status === 'passed');
if (allPassed) return; // exit 0
// Determine exit code. With --wait, success means every run reached the
// terminal status 'passed'. Without --wait, statuses are non-terminal by
// design ('queued' per CliBatchRunResult), so success means every trigger
// dispatched without error — mirroring single `test run` (no --wait),
// which exits 0 on a successful queued dispatch.
const failing = opts.wait
? batchRunResults.filter(r => r.status !== 'passed')
: batchRunResults.filter(r => r.error !== undefined);
if (failing.length === 0) return; // exit 0

// Check for a uniform non-pass exit code across all non-passed results.
const errorExitCodes = batchRunResults
.filter(r => r.error !== undefined)
.map(r => r.error!.exitCode);
const nonPassedStatuses = batchRunResults.filter(r => r.status !== 'passed');
// Check for a uniform non-pass exit code across all failing results.
const errorExitCodes = failing.filter(r => r.error !== undefined).map(r => r.error!.exitCode);
// Exit 7 only when EVERY run timed out — a mix of pass + timeout is "mixed
// outcomes" (exit 1), not "all timed out". `nonPassedStatuses.every(...)`
// outcomes" (exit 1), not "all timed out". `failing.every(...)` alone
// would incorrectly fire exit 7 when 1 of N passed and the rest timed out.
const allTimeout =
batchRunResults.length > 0 &&
failing.length === batchRunResults.length &&
batchRunResults.every(r => r.status === 'timeout' || r.error?.exitCode === 7);
if (allTimeout) {
throw new CLIError(
`All ${batchRunResults.length} batch run(s) timed out after ${timeoutSeconds}s.`,
7,
);
Comment on lines 2938 to 2942

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep no-wait failures in trigger terminology.

These branches can run with wait: false, but report timed-out/failed runs. No-wait only dispatches triggers; the default branch at Line 2963 already uses the correct terminology.

Proposed fix
- `All ${batchRunResults.length} batch run(s) timed out after ${timeoutSeconds}s.`
+ opts.wait
+   ? `All ${batchRunResults.length} batch run(s) timed out after ${timeoutSeconds}s.`
+   : `All ${batchRunResults.length} trigger(s) timed out after ${timeoutSeconds}s.`

- `Batch run finished: ${failing.length} run(s) failed with exit code ${uniformCode}.`
+ opts.wait
+   ? `Batch run finished: ${failing.length} run(s) failed with exit code ${uniformCode}.`
+   : `Batch run trigger finished: ${failing.length} trigger(s) failed with exit code ${uniformCode}.`

As per path instructions, text should match the underlying dispatch/trigger outcomes.

Also applies to: 2953-2957

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/test.ts` around lines 2938 - 2942, Update the no-wait error
branches around the allTimeout and corresponding failure handling to use
trigger/dispatch terminology instead of batch run terminology, including the
messages and any related counts or descriptions. Preserve the existing behavior
and leave the default wait-path wording unchanged.

Source: Path instructions

}
// If all non-passed results share the same specific exit code (6 or 11), use it.
if (errorExitCodes.length > 0 && errorExitCodes.length === nonPassedStatuses.length) {
// If all failing results share the same specific exit code (6 or 11), use it.
if (errorExitCodes.length > 0 && errorExitCodes.length === failing.length) {
const uniformCode = errorExitCodes[0];
if (
uniformCode !== undefined &&
Expand All @@ -2937,14 +2951,16 @@ async function runBatchRun(
uniformCode !== 7
) {
throw new CLIError(
`Batch run finished: ${nonPassedStatuses.length} run(s) failed with exit code ${uniformCode}.`,
`Batch run finished: ${failing.length} run(s) failed with exit code ${uniformCode}.`,
uniformCode,
);
}
}
// Default: mixed outcomes or generic failure → exit 1.
throw new CLIError(
`Batch run finished: ${batchRunResults.filter(r => r.status !== 'passed').length} of ${batchRunResults.length} run(s) did not pass.`,
opts.wait
? `Batch run finished: ${failing.length} of ${batchRunResults.length} run(s) did not pass.`
: `Batch run trigger finished: ${failing.length} of ${batchRunResults.length} trigger(s) failed.`,
1,
);
}
Expand Down
Loading