diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 700671b..cb14d9a 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -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 ` 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 ` 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 diff --git a/src/commands/test.create-batch-run.spec.ts b/src/commands/test.create-batch-run.spec.ts index 5719543..db2c3c1 100644 --- a/src/commands/test.create-batch-run.spec.ts +++ b/src/commands/test.create-batch-run.spec.ts @@ -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; + let errorSpy: ReturnType; + + 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 // --------------------------------------------------------------------------- diff --git a/src/commands/test.ts b/src/commands/test.ts index c6e6c31..69ce51a 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -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; @@ -2906,20 +2917,23 @@ 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( @@ -2927,8 +2941,8 @@ async function runBatchRun( 7, ); } - // 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 && @@ -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, ); }