From 80bbdeda1ce9591859cd7e80e22eea8bcdadf136 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 31 Jul 2026 09:18:05 -0700 Subject: [PATCH] fix(qa): verify requested scenarios and reject silent character runs (#116912) * fix(qa): authenticate requested scenario test execution * fix(qa): reject unanswered character evaluation runs * refactor(qa): extract native vitest report validation * fix(qa): require requested native test file to exist --------- Co-authored-by: Peter Steinberger --- extensions/qa-lab/src/character-eval.test.ts | 32 ++++ extensions/qa-lab/src/character-eval.ts | 11 +- ...t-file-scenario-runner.e2e-routing.test.ts | 16 +- .../src/test-file-scenario-runner.test.ts | 174 +++++++++++++++++- .../qa-lab/src/test-file-scenario-runner.ts | 37 +--- .../src/test-file-scenario-vitest-report.ts | 121 ++++++++++++ 6 files changed, 354 insertions(+), 37 deletions(-) create mode 100644 extensions/qa-lab/src/test-file-scenario-vitest-report.ts diff --git a/extensions/qa-lab/src/character-eval.test.ts b/extensions/qa-lab/src/character-eval.test.ts index aa076471fbfe..597dfb1b27c8 100644 --- a/extensions/qa-lab/src/character-eval.test.ts +++ b/extensions/qa-lab/src/character-eval.test.ts @@ -523,6 +523,38 @@ describe("runQaCharacterEval", () => { expect(result.runs[0]?.error).toBeUndefined(); }); + it.each([ + { description: "user-only conversation", transcript: "USER Alice: hello?" }, + { description: "report-only fallback", transcript: "# Character scenario report" }, + ])("rejects a passing suite with a $description", async ({ transcript }) => { + const runSuite = vi.fn(async (params: CharacterRunSuiteParams) => + makeSuiteResult({ + outputDir: params.outputDir, + model: params.primaryModel, + transcript, + }), + ); + const runJudge = makeRunJudge([ + { model: "openai/gpt-5.6-luna", rank: 1, score: 0.5, summary: "no reply" }, + ]); + + const result = await runQaCharacterEval({ + repoRoot: tempRoot, + outputDir: path.join(tempRoot, "character"), + models: ["openai/gpt-5.6-luna"], + judgeModels: ["openai/gpt-5.6-luna"], + runSuite, + runJudge, + }); + + expectFirstRunFailure(result, { + model: "openai/gpt-5.6-luna", + error: "candidate transcript did not contain an assistant reply", + }); + expect(result.runs[0]?.stats.assistantTurns).toBe(0); + expect(result.runs[0]?.transcript).toBe(transcript); + }); + it("marks raw tool failure transcripts as failed output", async () => { const runSuite = vi.fn(async (params: CharacterRunSuiteParams) => makeSuiteResult({ diff --git a/extensions/qa-lab/src/character-eval.ts b/extensions/qa-lab/src/character-eval.ts index f60b50f46ace..54a5cf6fd811 100644 --- a/extensions/qa-lab/src/character-eval.ts +++ b/extensions/qa-lab/src/character-eval.ts @@ -543,7 +543,14 @@ export async function runQaCharacterEval(params: QaCharacterEvalParams) { scenarioIds: [scenarioId], }); const transcript = extractTranscript(result); - const transcriptFailure = detectTranscriptFailure(transcript); + const stats = collectTranscriptStats(transcript); + // Character capture tolerates missed turns, so a passing scenario alone + // cannot prove this candidate ever delivered an assistant reply. + const transcriptFailure = + detectTranscriptFailure(transcript) ?? + (stats.assistantTurns === 0 + ? "candidate transcript did not contain an assistant reply" + : undefined); const failedScenarioCount = await readQaSuiteFailedScenarioCountFromFile( result.summaryPath, ); @@ -558,7 +565,7 @@ export async function runQaCharacterEval(params: QaCharacterEvalParams) { reportPath: result.reportPath, summaryPath: result.summaryPath, transcript, - stats: collectTranscriptStats(transcript), + stats, ...(transcriptFailure ? { error: transcriptFailure } : {}), } satisfies QaCharacterEvalRun; logCharacterEvalProgress( diff --git a/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts b/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts index 8f3f91a0949f..c492422a3375 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts @@ -31,6 +31,9 @@ describe("QA native Vitest scenario routing", () => { }; try { + const requestedTestFile = path.join(repoRoot, testPath); + await fs.mkdir(path.dirname(requestedTestFile), { recursive: true }); + await fs.writeFile(requestedTestFile, "// native scenario fixture\n", "utf8"); const result = await runQaTestFileScenarios({ repoRoot, outputDir: path.join(repoRoot, ".artifacts", "qa-e2e", scenario.id), @@ -45,7 +48,18 @@ describe("QA native Vitest scenario routing", () => { } await fs.writeFile( reportArg.slice("--outputFile.json=".length), - JSON.stringify({ numFailedTests: 0, numPassedTests: 1, success: true }), + JSON.stringify({ + numFailedTests: 0, + numPassedTests: 1, + success: true, + testResults: [ + { + name: path.join(repoRoot, testPath), + status: "passed", + assertionResults: [{ fullName: "runs paired node inference", status: "passed" }], + }, + ], + }), "utf8", ); return { exitCode: 0, stdout: "1 passed\n", stderr: "" }; diff --git a/extensions/qa-lab/src/test-file-scenario-runner.test.ts b/extensions/qa-lab/src/test-file-scenario-runner.test.ts index ee4a04a8283f..eed31fe0b375 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.test.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.test.ts @@ -121,18 +121,43 @@ async function makeTempRepo(prefix: string) { async function writeNativeVitestReport( command: QaScenarioCommandExecution, - counts: { failed?: number; passed: number }, + counts: { + createRequestedTestFile?: boolean; + failed?: number; + passed: number; + testFilePath?: string; + testName?: string; + }, ) { const reportArg = command.args.find((arg) => arg.startsWith("--outputFile.json=")); if (!reportArg) { return; } + const requestedTestPath = command.args.find((arg) => arg.endsWith(".test.ts")); + if (requestedTestPath && counts.createRequestedTestFile !== false) { + const requestedTestFile = path.resolve(command.cwd, requestedTestPath); + await fs.mkdir(path.dirname(requestedTestFile), { recursive: true }); + await fs.writeFile(requestedTestFile, "// native scenario fixture\n", "utf8"); + } + const testNamePatternIndex = command.args.indexOf("--testNamePattern"); + const testName = + counts.testName ?? + (testNamePatternIndex < 0 ? undefined : command.args[testNamePatternIndex + 1]) ?? + "executes the requested scenario"; await fs.writeFile( reportArg.slice("--outputFile.json=".length), JSON.stringify({ numFailedTests: counts.failed ?? 0, numPassedTests: counts.passed, success: (counts.failed ?? 0) === 0, + testResults: [ + { + name: path.resolve(command.cwd, counts.testFilePath ?? requestedTestPath ?? "unknown"), + status: counts.passed > 0 ? "passed" : "skipped", + assertionResults: + counts.passed > 0 ? [{ fullName: testName, title: testName, status: "passed" }] : [], + }, + ], }), "utf8", ); @@ -448,6 +473,153 @@ describe("qa test file scenario runner", () => { }, ); + it.each([{ executionKind: "vitest" as const }, { executionKind: "playwright" as const }])( + "rejects a passing $executionKind report for an unrelated test file", + async ({ executionKind }) => { + const repoRoot = await makeTempRepo(`qa-${executionKind}-wrong-report-file-`); + const outputDir = path.join(repoRoot, ".artifacts", "qa-e2e", `scenario-${executionKind}`); + const result = await runQaTestFileScenarios({ + repoRoot, + outputDir, + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [ + makeTestFileScenario( + executionKind, + executionKind === "playwright" + ? "ui/src/e2e/chat-flow.e2e.test.ts" + : "extensions/qa-lab/src/coverage-report.test.ts", + ), + ], + runCommand: async (command) => { + await writeNativeVitestReport(command, { + passed: 1, + testFilePath: "extensions/qa-lab/src/unrelated.test.ts", + }); + return { exitCode: 0, stdout: "unrelated test passed\n", stderr: "" }; + }, + }); + + expect(result.results[0]).toMatchObject({ + failureMessage: expect.stringContaining("requested test file"), + status: "fail", + }); + expect(result.evidence.entries[0]?.result.status).toBe("fail"); + }, + ); + + it.each([{ executionKind: "vitest" as const }, { executionKind: "playwright" as const }])( + "rejects a passing $executionKind report when the requested test file does not exist", + async ({ executionKind }) => { + const repoRoot = await makeTempRepo(`qa-${executionKind}-missing-requested-test-`); + const scenarioPath = + executionKind === "playwright" + ? "ui/src/e2e/chat-flow.e2e.test.ts" + : "extensions/qa-lab/src/coverage-report.test.ts"; + const result = await runQaTestFileScenarios({ + repoRoot, + outputDir: path.join(repoRoot, ".artifacts", "qa-e2e", `scenario-${executionKind}`), + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [makeTestFileScenario(executionKind, scenarioPath)], + runCommand: async (command) => { + await writeNativeVitestReport(command, { + createRequestedTestFile: false, + passed: 1, + }); + return { exitCode: 0, stdout: "missing test reportedly passed\n", stderr: "" }; + }, + }); + + expect(result.results[0]).toMatchObject({ + failureMessage: expect.stringContaining("existing requested test file"), + status: "fail", + }); + expect(result.evidence.entries[0]?.result.status).toBe("fail"); + }, + ); + + it.skipIf(process.platform === "win32")( + "authenticates requested tests when the checkout root is a symlink", + async () => { + const canonicalRoot = await fs.realpath(await makeTempRepo("qa-vitest-symlinked-checkout-")); + const symlinkedRoot = path.join(canonicalRoot, "checkout-alias"); + await fs.symlink(canonicalRoot, symlinkedRoot, "dir"); + const scenarioPath = "extensions/qa-lab/src/coverage-report.test.ts"; + const result = await runQaTestFileScenarios({ + repoRoot: symlinkedRoot, + outputDir: path.join(symlinkedRoot, ".artifacts", "qa-e2e", "scenario-vitest"), + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [makeTestFileScenario("vitest", scenarioPath)], + runCommand: async (command) => { + await writeNativeVitestReport(command, { + passed: 1, + testFilePath: path.join(canonicalRoot, scenarioPath), + }); + return { exitCode: 0, stdout: "canonical test passed\n", stderr: "" }; + }, + }); + + expect(result.results[0]).toMatchObject({ status: "pass" }); + expect(result.evidence.entries[0]?.result.status).toBe("pass"); + }, + ); + + it("rejects a passing Playwright report that misses the requested test name", async () => { + const repoRoot = await makeTempRepo("qa-playwright-wrong-report-test-"); + const result = await runQaTestFileScenarios({ + repoRoot, + outputDir: path.join(repoRoot, ".artifacts", "qa-e2e", "scenario-playwright"), + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [ + makeTestFileScenario( + "playwright", + "ui/src/e2e/chat-flow.e2e.test.ts", + "required visual assertion", + ), + ], + runCommand: async (command) => { + await writeNativeVitestReport(command, { + passed: 1, + testName: "unrelated visual assertion", + }); + return { exitCode: 0, stdout: "unrelated assertion passed\n", stderr: "" }; + }, + }); + + expect(result.results[0]).toMatchObject({ + failureMessage: expect.stringContaining("requested test name"), + status: "fail", + }); + expect(result.evidence.entries[0]?.result.status).toBe("fail"); + }); + + it("records invalid Playwright test-name patterns as failed scenario evidence", async () => { + const repoRoot = await makeTempRepo("qa-playwright-invalid-report-pattern-"); + const result = await runQaTestFileScenarios({ + repoRoot, + outputDir: path.join(repoRoot, ".artifacts", "qa-e2e", "scenario-playwright"), + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [makeTestFileScenario("playwright", "ui/src/e2e/chat-flow.e2e.test.ts", "[")], + runCommand: async (command) => { + await writeNativeVitestReport(command, { + passed: 1, + testName: "executed visual assertion", + }); + return { exitCode: 0, stdout: "visual assertion passed\n", stderr: "" }; + }, + }); + + expect(result.results[0]).toMatchObject({ + failureMessage: expect.stringContaining("invalid requested test name pattern"), + status: "fail", + }); + expect(result.evidence.entries[0]?.result.status).toBe("fail"); + }); + it.each([{ executionKind: "vitest" as const }, { executionKind: "playwright" as const }])( "does not reuse a prior passing $executionKind report when the next child writes none", async ({ executionKind }) => { diff --git a/extensions/qa-lab/src/test-file-scenario-runner.ts b/extensions/qa-lab/src/test-file-scenario-runner.ts index aa56845ef890..fb35d918ef6b 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.ts @@ -29,10 +29,11 @@ import { type QaScenarioCommandResult, } from "./test-file-scenario-command-lifecycle.js"; import { isDockerE2eScenario, runDockerE2eBatch } from "./test-file-scenario-docker-batch.js"; +import { readScriptProducerEvidence } from "./test-file-scenario-script-evidence.js"; import { - readJsonFileIfExists, - readScriptProducerEvidence, -} from "./test-file-scenario-script-evidence.js"; + readNativeVitestExecutionFailure, + resolveNativeVitestReportPath, +} from "./test-file-scenario-vitest-report.js"; export type { QaScenarioCommandExecution } from "./test-file-scenario-command-lifecycle.js"; export type QaTestFileScenario = QaSeedScenarioWithSource & { @@ -101,10 +102,6 @@ export function isQaTestFileScenario( ); } -function resolveNativeVitestReportPath(scenario: QaTestFileScenario, outputDir: string): string { - return path.join(outputDir, `${scenario.id}.vitest-report.json`); -} - function vitestReporterArgs( scenario: QaTestFileScenario, context: { outputDir: string }, @@ -241,32 +238,6 @@ function withScenarioCoverage( return { ...entry, coverage: coverageForScenario(scenario) }; } -async function readNativeVitestExecutionFailure(params: { - outputDir: string; - scenario: QaTestFileScenario; -}): Promise { - const reportPath = resolveNativeVitestReportPath(params.scenario, params.outputDir); - const report = await readJsonFileIfExists(reportPath); - if (!report || typeof report !== "object") { - return `Vitest exited successfully without writing a valid JSON test report at ${reportPath}.`; - } - const { numFailedTests, numPassedTests, success } = report as { - numFailedTests?: unknown; - numPassedTests?: unknown; - success?: unknown; - }; - if ( - success !== true || - typeof numPassedTests !== "number" || - !Number.isSafeInteger(numPassedTests) || - numPassedTests < 1 || - numFailedTests !== 0 - ) { - return "Vitest exited successfully without reporting a successfully executed test."; - } - return undefined; -} - async function runScenarioCommandSteps(params: { commandTimeoutMs: number; env: NodeJS.ProcessEnv; diff --git a/extensions/qa-lab/src/test-file-scenario-vitest-report.ts b/extensions/qa-lab/src/test-file-scenario-vitest-report.ts new file mode 100644 index 000000000000..a8d277294391 --- /dev/null +++ b/extensions/qa-lab/src/test-file-scenario-vitest-report.ts @@ -0,0 +1,121 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import type { QaSeedScenarioWithSource } from "./scenario-catalog.js"; +import { readJsonFileIfExists } from "./test-file-scenario-script-evidence.js"; + +type NativeTestFileScenario = Pick & { + execution: Extract< + QaSeedScenarioWithSource["execution"], + { kind: "script" | "vitest" | "playwright" } + >; +}; + +export function resolveNativeVitestReportPath( + scenario: Pick, + outputDir: string, +): string { + return path.join(outputDir, `${scenario.id}.vitest-report.json`); +} + +async function canonicalizeNativeTestFilePath(params: { + canonicalRepoRoot: string; + repoRoot: string; + testPath: string; +}) { + const absoluteTestPath = path.resolve(params.repoRoot, params.testPath); + const canonicalTestPath = await fs.realpath(absoluteTestPath).catch(() => undefined); + if (canonicalTestPath) { + return canonicalTestPath; + } + + const repoRelativePath = path.relative(params.repoRoot, absoluteTestPath); + if ( + repoRelativePath !== ".." && + !repoRelativePath.startsWith(`..${path.sep}`) && + !path.isAbsolute(repoRelativePath) + ) { + return path.resolve(params.canonicalRepoRoot, repoRelativePath); + } + return absoluteTestPath; +} + +export async function readNativeVitestExecutionFailure(params: { + outputDir: string; + repoRoot: string; + scenario: NativeTestFileScenario; +}): Promise { + const reportPath = resolveNativeVitestReportPath(params.scenario, params.outputDir); + const report = await readJsonFileIfExists(reportPath); + if (!report || typeof report !== "object") { + return `Vitest exited successfully without writing a valid JSON test report at ${reportPath}.`; + } + const { numFailedTests, numPassedTests, success, testResults } = report as { + numFailedTests?: unknown; + numPassedTests?: unknown; + success?: unknown; + testResults?: Array<{ + name?: unknown; + assertionResults?: Array<{ fullName?: unknown; status?: unknown; title?: unknown }>; + }>; + }; + if ( + success !== true || + typeof numPassedTests !== "number" || + !Number.isSafeInteger(numPassedTests) || + numPassedTests < 1 || + numFailedTests !== 0 + ) { + return "Vitest exited successfully without reporting a successfully executed test."; + } + const expectedTestPath = await fs + .realpath(path.resolve(params.repoRoot, params.scenario.execution.path)) + .catch(() => undefined); + if (!expectedTestPath) { + return `Vitest exited successfully without an existing requested test file ${params.scenario.execution.path}.`; + } + const canonicalRepoRoot = await fs.realpath(params.repoRoot); + const matchingTestResult = ( + await Promise.all( + (Array.isArray(testResults) ? testResults : []).map(async (result) => { + if (!result || typeof result.name !== "string") { + return undefined; + } + const reportedTestPath = await canonicalizeNativeTestFilePath({ + canonicalRepoRoot, + repoRoot: params.repoRoot, + testPath: result.name, + }); + return reportedTestPath === expectedTestPath ? result : undefined; + }), + ) + ).find((result) => result !== undefined); + const passedAssertions = Array.isArray(matchingTestResult?.assertionResults) + ? matchingTestResult.assertionResults.filter((assertion) => assertion.status === "passed") + : []; + if (passedAssertions.length === 0) { + return `Vitest exited successfully without a passed assertion for the requested test file ${params.scenario.execution.path}.`; + } + const testNamePattern = + params.scenario.execution.kind === "playwright" + ? params.scenario.execution.testNamePattern + : undefined; + if (testNamePattern) { + let requestedTestName: RegExp; + try { + // Vitest resolves string --testNamePattern values with the same RegExp constructor. + requestedTestName = new RegExp(testNamePattern); + } catch { + return `Vitest exited successfully with an invalid requested test name pattern ${JSON.stringify(testNamePattern)}.`; + } + if ( + !passedAssertions.some((assertion) => { + const assertionName = + typeof assertion.fullName === "string" ? assertion.fullName : assertion.title; + return typeof assertionName === "string" && requestedTestName.test(assertionName); + }) + ) { + return `Vitest exited successfully without a passed assertion for the requested test name pattern ${JSON.stringify(testNamePattern)}.`; + } + } + return undefined; +}