mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-02 10:21:34 +00:00
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 <steipete@macos.shared>
This commit is contained in:
committed by
GitHub
parent
df59af2fa6
commit
80bbdeda1c
@@ -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({
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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: "" };
|
||||
|
||||
@@ -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 }) => {
|
||||
|
||||
@@ -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<string | undefined> {
|
||||
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;
|
||||
|
||||
121
extensions/qa-lab/src/test-file-scenario-vitest-report.ts
Normal file
121
extensions/qa-lab/src/test-file-scenario-vitest-report.ts
Normal file
@@ -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<QaSeedScenarioWithSource, "id"> & {
|
||||
execution: Extract<
|
||||
QaSeedScenarioWithSource["execution"],
|
||||
{ kind: "script" | "vitest" | "playwright" }
|
||||
>;
|
||||
};
|
||||
|
||||
export function resolveNativeVitestReportPath(
|
||||
scenario: Pick<NativeTestFileScenario, "id">,
|
||||
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<string | undefined> {
|
||||
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;
|
||||
}
|
||||
Reference in New Issue
Block a user