From b967d4441a1bc9498493a939a143e0c4045462e1 Mon Sep 17 00:00:00 2001 From: Yazan-O Date: Mon, 26 Oct 2026 11:18:02 -0500 Subject: [PATCH 1/3] fix(test): show the failed step on the single-run result card and annotation `test run --wait` and `test wait` printed failureKind and error but never failedStepIndex, which `test result` already shows. The single-run GitHub annotation dropped it too. The card now prints `failedStepIndex ` after failureKind for a failed or blocked run, and summarizeSingleRun carries the index so the annotation ends with ` (step )`. The index is printed as the server sends it, like `test result`. JSON output and batch rows are unchanged. Closes #672 --- src/commands/test.run.spec.ts | 58 ++++++++++++++++++++++++++++++++++- src/commands/test.ts | 2 ++ src/lib/gh-output.test.ts | 40 ++++++++++++++++++++++++ src/lib/gh-output.ts | 6 +++- 4 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/commands/test.run.spec.ts b/src/commands/test.run.spec.ts index fef12ac6..ac427903 100644 --- a/src/commands/test.run.spec.ts +++ b/src/commands/test.run.spec.ts @@ -16,7 +16,7 @@ import { DRY_RUN_BANNER, resetDryRunBannerForTesting } from '../lib/client-facto import type { FetchImpl } from '../lib/http.js'; import type { RunResponse, TriggerRunResponse, BatchRunFreshResponse } from '../lib/runs.types.js'; import { takeTelemetryExtras } from '../lib/telemetry.js'; -import { runTestRun, runTestRunAll } from './test.js'; +import { renderRunResponseText, runTestRun, runTestRunAll } from './test.js'; // --------------------------------------------------------------------------- // Helpers @@ -7203,6 +7203,29 @@ describe('runTestRun — --target-url reachability preflight wiring', () => { }); }); +describe('run card: failed step (#672)', () => { + it('prints failedStepIndex under failureKind for a failed run, numbered as `test result` prints it', () => { + const lines = renderRunResponseText(makeFailedRun()).split('\n'); + const kind = lines.indexOf('failureKind assertion'); + expect(kind).toBeGreaterThanOrEqual(0); + expect(lines[kind + 1]).toBe('failedStepIndex 2'); + }); + + it('prints it for a blocked run too', () => { + const text = renderRunResponseText({ ...makeFailedRun(), status: 'blocked' }); + expect(text.split('\n')).toContain('failedStepIndex 2'); + }); + + it('omits the line when the server reported no failed step, or the run passed', () => { + expect(renderRunResponseText({ ...makeFailedRun(), failedStepIndex: null })).not.toContain( + 'failedStepIndex', + ); + expect(renderRunResponseText({ ...makePassedRun(), failedStepIndex: 2 })).not.toContain( + 'failedStepIndex', + ); + }); +}); + describe('gh-output integration on single-test run --wait (Gap A)', () => { function singleRunHarness(run: RunResponse) { const { credentialsPath } = makeCreds(); @@ -7258,6 +7281,39 @@ describe('gh-output integration on single-test run --wait (Gap A)', () => { ); }); + it('the text card and the annotation both name the failed step', async () => { + const { credentialsPath, fetchImpl } = singleRunHarness({ + ...makeFailedRun(), + error: 'Expected heading to be visible', + }); + const stdoutLines: string[] = []; + const err = await runTestRun( + { + profile: 'default', + output: 'text', + debug: false, + dryRun: false, + testId: 'test_xyz', + wait: true, + timeoutSeconds: 60, + ghOutput: true, + }, + { + credentialsPath, + fetchImpl, + stdout: line => stdoutLines.push(line), + stderr: () => undefined, + env: {} as NodeJS.ProcessEnv, + sleep: instantSleep, + }, + ).catch(e => e); + expect(err).toMatchObject({ exitCode: 1 }); + expect(stdoutLines.join('\n').split('\n')).toContain('failedStepIndex 2'); + const annotations = stdoutLines.filter(line => line.startsWith('::error')); + expect(annotations).toHaveLength(1); + expect(annotations[0]).toContain('::status=failed Expected heading to be visible (step 2)'); + }); + it('under Actions with --output json: stdout stays parseable JSON, ::error:: goes to stderr', async () => { const { credentialsPath, fetchImpl } = singleRunHarness(makeFailedRun()); const stdoutLines: string[] = []; diff --git a/src/commands/test.ts b/src/commands/test.ts index 8bf5ce5b..f22bb877 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -7563,6 +7563,8 @@ export function renderRunResponseText( if (run.startedAt) lines.push(`startedAt ${run.startedAt}`); if (run.finishedAt) lines.push(`finishedAt ${run.finishedAt}`); if (run.failureKind) lines.push(`failureKind ${run.failureKind}`); + if ((run.status === 'failed' || run.status === 'blocked') && run.failedStepIndex !== null) + lines.push(`failedStepIndex ${run.failedStepIndex}`); // D5-UX: show the human error string when status is failed/blocked and // the backend provided it. Truncate long multi-line errors to the first // line (≤200 chars) so the text output stays readable. JSON mode is diff --git a/src/lib/gh-output.test.ts b/src/lib/gh-output.test.ts index 40519bee..88982483 100644 --- a/src/lib/gh-output.test.ts +++ b/src/lib/gh-output.test.ts @@ -188,6 +188,46 @@ describe('summarizeSingleRun', () => { }); }); +describe('single-run annotation: failed step (#672)', () => { + function annotationsFor(run: Parameters[0]): string[] { + const lines: string[] = []; + emitGithubOutputs( + summarizeSingleRun(run), + {}, + { stdout: line => lines.push(line), stderr: () => {}, appendFile: () => {} }, + { force: true }, + ); + return lines; + } + + it('carries failedStepIndex onto the row and names the step after the error', () => { + const run = { + testId: 'test_b', + runId: 'run_b', + status: 'failed', + error: 'Expected heading to be visible', + executionUrl: 'https://portal.example.com/exec/b', + failedStepIndex: 4, + }; + expect(summarizeSingleRun(run).runs[0]!.failedStepIndex).toBe(4); + expect(annotationsFor(run)).toEqual([ + '::error title=TestSprite test_b::status=failed Expected heading to be visible (step 4) https://portal.example.com/exec/b', + ]); + }); + + it('names the step when the run carries no error text', () => { + expect(annotationsFor({ testId: 'test_b', status: 'failed', failedStepIndex: 1 })).toEqual([ + '::error title=TestSprite test_b::status=failed (step 1)', + ]); + }); + + it('leaves the row and the annotation unchanged when there is no failed step', () => { + const run = { testId: 'test_b', status: 'failed', error: 'boom', failedStepIndex: null }; + expect(summarizeSingleRun(run).runs[0]).not.toHaveProperty('failedStepIndex'); + expect(annotationsFor(run)).toEqual(['::error title=TestSprite test_b::status=failed boom']); + }); +}); + describe('renderJobSummaryMarkdown', () => { it('renders the counts headline and one table row per run', () => { const md = renderJobSummaryMarkdown(summarizeAcceptedPayload(PAYLOAD)); diff --git a/src/lib/gh-output.ts b/src/lib/gh-output.ts index 4f9b5bdf..e00c723b 100644 --- a/src/lib/gh-output.ts +++ b/src/lib/gh-output.ts @@ -34,6 +34,7 @@ export interface CiRunRow { /** This run's execution-result page link — anchors the Run column. */ executionUrl?: string; error?: string; + failedStepIndex?: number; } export interface CiSummary { @@ -241,6 +242,7 @@ export function summarizeSingleRun(run: { dashboardUrl?: string | null; executionUrl?: string | null; error?: string | null; + failedStepIndex?: number | null; }): CiSummary { const row: CiRunRow = { testId: String(run.testId ?? ''), @@ -250,6 +252,7 @@ export function summarizeSingleRun(run: { ...(typeof run.dashboardUrl === 'string' ? { dashboardUrl: run.dashboardUrl } : {}), ...(typeof run.executionUrl === 'string' ? { executionUrl: run.executionUrl } : {}), ...(typeof run.error === 'string' && run.error.length > 0 ? { error: run.error } : {}), + ...(typeof run.failedStepIndex === 'number' ? { failedStepIndex: run.failedStepIndex } : {}), }; const passed = row.status === 'passed' ? 1 : 0; const timedOut = row.status === 'timeout' ? 1 : 0; @@ -408,6 +411,7 @@ export function emitGithubOutputs( for (const row of summary.runs) { if (row.status === 'passed') continue; const detail = row.error !== undefined ? ` ${row.error}` : ''; + const step = row.failedStepIndex !== undefined ? ` (step ${row.failedStepIndex})` : ''; // Prefer this run's result page over the test-case page — on a failure the // result is what the reviewer wants to open. Falls back to the test-case // link (V2 runs, which have no execution page), then to nothing. @@ -416,7 +420,7 @@ export function emitGithubOutputs( // Escape both halves: a raw multiline run error (or a testId) must not be // able to smuggle a second workflow command into the Actions stream. const title = `TestSprite ${escapeCommandProperty(row.title?.trim() || row.testId)}`; - const message = escapeCommandData(`status=${row.status}${detail}${link}`); + const message = escapeCommandData(`status=${row.status}${detail}${step}${link}`); // Severity mirrors the summary counters: never-dispatched rows are // warnings (they may not even gate the exit), real failures are errors. const severity = NON_DISPATCHED_STATUSES.has(row.status) ? 'warning' : 'error'; From 88fb36df27f9834621c26e587f063aef8f0c11d2 Mon Sep 17 00:00:00 2001 From: Yazan-O Date: Mon, 26 Oct 2026 11:20:15 -0500 Subject: [PATCH 2/3] fix(test): keep the step off annotations for runs that did not fail The single-run annotation now names the failed step only for failed or blocked runs, matching the result card. --- src/lib/gh-output.test.ts | 6 ++++++ src/lib/gh-output.ts | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/lib/gh-output.test.ts b/src/lib/gh-output.test.ts index 88982483..3efb615c 100644 --- a/src/lib/gh-output.test.ts +++ b/src/lib/gh-output.test.ts @@ -226,6 +226,12 @@ describe('single-run annotation: failed step (#672)', () => { expect(summarizeSingleRun(run).runs[0]).not.toHaveProperty('failedStepIndex'); expect(annotationsFor(run)).toEqual(['::error title=TestSprite test_b::status=failed boom']); }); + + it('drops the step unless the run failed or was blocked', () => { + const run = { testId: 'test_b', status: 'cancelled', failedStepIndex: 3 }; + expect(summarizeSingleRun(run).runs[0]).not.toHaveProperty('failedStepIndex'); + expect(annotationsFor(run).join('\n')).not.toContain('(step'); + }); }); describe('renderJobSummaryMarkdown', () => { diff --git a/src/lib/gh-output.ts b/src/lib/gh-output.ts index e00c723b..7873b356 100644 --- a/src/lib/gh-output.ts +++ b/src/lib/gh-output.ts @@ -252,7 +252,10 @@ export function summarizeSingleRun(run: { ...(typeof run.dashboardUrl === 'string' ? { dashboardUrl: run.dashboardUrl } : {}), ...(typeof run.executionUrl === 'string' ? { executionUrl: run.executionUrl } : {}), ...(typeof run.error === 'string' && run.error.length > 0 ? { error: run.error } : {}), - ...(typeof run.failedStepIndex === 'number' ? { failedStepIndex: run.failedStepIndex } : {}), + ...((run.status === 'failed' || run.status === 'blocked') && + typeof run.failedStepIndex === 'number' + ? { failedStepIndex: run.failedStepIndex } + : {}), }; const passed = row.status === 'passed' ? 1 : 0; const timedOut = row.status === 'timeout' ? 1 : 0; From 8469a5423723fba516fa55c612a71614d6fa2496 Mon Sep 17 00:00:00 2001 From: Yazan-O Date: Fri, 30 Oct 2026 18:02:26 -0500 Subject: [PATCH 3/3] fix(test): keep only the run card line, per the accepted scope of #672 Drop the annotation / summarizeSingleRun half and test the cases listed on the issue. --- src/commands/test.run.spec.ts | 44 ++++++--------------------------- src/lib/gh-output.test.ts | 46 ----------------------------------- src/lib/gh-output.ts | 9 +------ 3 files changed, 8 insertions(+), 91 deletions(-) diff --git a/src/commands/test.run.spec.ts b/src/commands/test.run.spec.ts index ac427903..7fa5a584 100644 --- a/src/commands/test.run.spec.ts +++ b/src/commands/test.run.spec.ts @@ -7204,11 +7204,11 @@ describe('runTestRun — --target-url reachability preflight wiring', () => { }); describe('run card: failed step (#672)', () => { - it('prints failedStepIndex under failureKind for a failed run, numbered as `test result` prints it', () => { - const lines = renderRunResponseText(makeFailedRun()).split('\n'); + it('prints failedStepIndex right after failureKind for a failed run', () => { + const lines = renderRunResponseText({ ...makeFailedRun(), failedStepIndex: 4 }).split('\n'); const kind = lines.indexOf('failureKind assertion'); expect(kind).toBeGreaterThanOrEqual(0); - expect(lines[kind + 1]).toBe('failedStepIndex 2'); + expect(lines[kind + 1]).toBe('failedStepIndex 4'); }); it('prints it for a blocked run too', () => { @@ -7216,10 +7216,13 @@ describe('run card: failed step (#672)', () => { expect(text.split('\n')).toContain('failedStepIndex 2'); }); - it('omits the line when the server reported no failed step, or the run passed', () => { + it('omits the line when the server reported no failed step', () => { expect(renderRunResponseText({ ...makeFailedRun(), failedStepIndex: null })).not.toContain( 'failedStepIndex', ); + }); + + it('omits the line for a passed run', () => { expect(renderRunResponseText({ ...makePassedRun(), failedStepIndex: 2 })).not.toContain( 'failedStepIndex', ); @@ -7281,39 +7284,6 @@ describe('gh-output integration on single-test run --wait (Gap A)', () => { ); }); - it('the text card and the annotation both name the failed step', async () => { - const { credentialsPath, fetchImpl } = singleRunHarness({ - ...makeFailedRun(), - error: 'Expected heading to be visible', - }); - const stdoutLines: string[] = []; - const err = await runTestRun( - { - profile: 'default', - output: 'text', - debug: false, - dryRun: false, - testId: 'test_xyz', - wait: true, - timeoutSeconds: 60, - ghOutput: true, - }, - { - credentialsPath, - fetchImpl, - stdout: line => stdoutLines.push(line), - stderr: () => undefined, - env: {} as NodeJS.ProcessEnv, - sleep: instantSleep, - }, - ).catch(e => e); - expect(err).toMatchObject({ exitCode: 1 }); - expect(stdoutLines.join('\n').split('\n')).toContain('failedStepIndex 2'); - const annotations = stdoutLines.filter(line => line.startsWith('::error')); - expect(annotations).toHaveLength(1); - expect(annotations[0]).toContain('::status=failed Expected heading to be visible (step 2)'); - }); - it('under Actions with --output json: stdout stays parseable JSON, ::error:: goes to stderr', async () => { const { credentialsPath, fetchImpl } = singleRunHarness(makeFailedRun()); const stdoutLines: string[] = []; diff --git a/src/lib/gh-output.test.ts b/src/lib/gh-output.test.ts index 3efb615c..40519bee 100644 --- a/src/lib/gh-output.test.ts +++ b/src/lib/gh-output.test.ts @@ -188,52 +188,6 @@ describe('summarizeSingleRun', () => { }); }); -describe('single-run annotation: failed step (#672)', () => { - function annotationsFor(run: Parameters[0]): string[] { - const lines: string[] = []; - emitGithubOutputs( - summarizeSingleRun(run), - {}, - { stdout: line => lines.push(line), stderr: () => {}, appendFile: () => {} }, - { force: true }, - ); - return lines; - } - - it('carries failedStepIndex onto the row and names the step after the error', () => { - const run = { - testId: 'test_b', - runId: 'run_b', - status: 'failed', - error: 'Expected heading to be visible', - executionUrl: 'https://portal.example.com/exec/b', - failedStepIndex: 4, - }; - expect(summarizeSingleRun(run).runs[0]!.failedStepIndex).toBe(4); - expect(annotationsFor(run)).toEqual([ - '::error title=TestSprite test_b::status=failed Expected heading to be visible (step 4) https://portal.example.com/exec/b', - ]); - }); - - it('names the step when the run carries no error text', () => { - expect(annotationsFor({ testId: 'test_b', status: 'failed', failedStepIndex: 1 })).toEqual([ - '::error title=TestSprite test_b::status=failed (step 1)', - ]); - }); - - it('leaves the row and the annotation unchanged when there is no failed step', () => { - const run = { testId: 'test_b', status: 'failed', error: 'boom', failedStepIndex: null }; - expect(summarizeSingleRun(run).runs[0]).not.toHaveProperty('failedStepIndex'); - expect(annotationsFor(run)).toEqual(['::error title=TestSprite test_b::status=failed boom']); - }); - - it('drops the step unless the run failed or was blocked', () => { - const run = { testId: 'test_b', status: 'cancelled', failedStepIndex: 3 }; - expect(summarizeSingleRun(run).runs[0]).not.toHaveProperty('failedStepIndex'); - expect(annotationsFor(run).join('\n')).not.toContain('(step'); - }); -}); - describe('renderJobSummaryMarkdown', () => { it('renders the counts headline and one table row per run', () => { const md = renderJobSummaryMarkdown(summarizeAcceptedPayload(PAYLOAD)); diff --git a/src/lib/gh-output.ts b/src/lib/gh-output.ts index 7873b356..4f9b5bdf 100644 --- a/src/lib/gh-output.ts +++ b/src/lib/gh-output.ts @@ -34,7 +34,6 @@ export interface CiRunRow { /** This run's execution-result page link — anchors the Run column. */ executionUrl?: string; error?: string; - failedStepIndex?: number; } export interface CiSummary { @@ -242,7 +241,6 @@ export function summarizeSingleRun(run: { dashboardUrl?: string | null; executionUrl?: string | null; error?: string | null; - failedStepIndex?: number | null; }): CiSummary { const row: CiRunRow = { testId: String(run.testId ?? ''), @@ -252,10 +250,6 @@ export function summarizeSingleRun(run: { ...(typeof run.dashboardUrl === 'string' ? { dashboardUrl: run.dashboardUrl } : {}), ...(typeof run.executionUrl === 'string' ? { executionUrl: run.executionUrl } : {}), ...(typeof run.error === 'string' && run.error.length > 0 ? { error: run.error } : {}), - ...((run.status === 'failed' || run.status === 'blocked') && - typeof run.failedStepIndex === 'number' - ? { failedStepIndex: run.failedStepIndex } - : {}), }; const passed = row.status === 'passed' ? 1 : 0; const timedOut = row.status === 'timeout' ? 1 : 0; @@ -414,7 +408,6 @@ export function emitGithubOutputs( for (const row of summary.runs) { if (row.status === 'passed') continue; const detail = row.error !== undefined ? ` ${row.error}` : ''; - const step = row.failedStepIndex !== undefined ? ` (step ${row.failedStepIndex})` : ''; // Prefer this run's result page over the test-case page — on a failure the // result is what the reviewer wants to open. Falls back to the test-case // link (V2 runs, which have no execution page), then to nothing. @@ -423,7 +416,7 @@ export function emitGithubOutputs( // Escape both halves: a raw multiline run error (or a testId) must not be // able to smuggle a second workflow command into the Actions stream. const title = `TestSprite ${escapeCommandProperty(row.title?.trim() || row.testId)}`; - const message = escapeCommandData(`status=${row.status}${detail}${step}${link}`); + const message = escapeCommandData(`status=${row.status}${detail}${link}`); // Severity mirrors the summary counters: never-dispatched rows are // warnings (they may not even gate the exit), real failures are errors. const severity = NON_DISPATCHED_STATUSES.has(row.status) ? 'warning' : 'error';