mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(qa): artifact write failure preserves previous files (#122580)
* fix(qa): publish suite artifacts atomically * test(qa): consolidate artifact durability coverage
This commit is contained in:
committed by
GitHub
parent
562391b9af
commit
fea6d96378
@@ -1,6 +1,7 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import type { OpenClawCrablineChannelDriverSelection } from "@openclaw/crabline";
|
||||
import { replaceFileAtomic } from "openclaw/plugin-sdk/security-runtime";
|
||||
import { assertQaSuiteArtifactWritten } from "./artifact-assertion.js";
|
||||
import {
|
||||
hasQaCrablineArtifactPath,
|
||||
@@ -25,6 +26,28 @@ type QaCrablineChannelDriverSmokeResult = Awaited<
|
||||
ReturnType<QaCrablineRuntime["runOpenClawCrablineChannelDriverSmoke"]>
|
||||
>;
|
||||
|
||||
/** Atomically replaces each file in order; summary-last is a completion signal, not a set transaction. */
|
||||
export async function publishQaSuiteArtifactFiles(params: {
|
||||
outputDir: string;
|
||||
files: readonly { content: string | Uint8Array; filePath: string }[];
|
||||
}) {
|
||||
await fs.mkdir(params.outputDir, { recursive: true });
|
||||
const dirMode = (await fs.stat(params.outputDir)).mode & 0o7777;
|
||||
for (const file of params.files) {
|
||||
await replaceFileAtomic({
|
||||
filePath: file.filePath,
|
||||
content: file.content,
|
||||
dirMode,
|
||||
mode: 0o600,
|
||||
preserveExistingMode: true,
|
||||
tempPrefix: `${path.basename(file.filePath)}.qa-artifact`,
|
||||
syncTempFile: true,
|
||||
syncParentDir: true,
|
||||
throwOnCleanupError: true,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
export type QaSuiteSummaryJsonParams = {
|
||||
scenarios: QaSuiteScenarioResult[];
|
||||
startedAt: Date;
|
||||
@@ -264,22 +287,26 @@ export async function writeQaSuiteArtifacts(params: {
|
||||
);
|
||||
}
|
||||
const writeEvidenceFile = params.writeEvidenceFile ?? true;
|
||||
await fs.writeFile(reportPath, report, "utf8");
|
||||
if (evidence && writeEvidenceFile) {
|
||||
await fs.writeFile(evidencePath, `${JSON.stringify(evidence, null, 2)}\n`, "utf8");
|
||||
}
|
||||
await fs.writeFile(
|
||||
summaryPath,
|
||||
`${JSON.stringify(
|
||||
buildQaSuiteSummaryJson({
|
||||
...params,
|
||||
channelDriverSelection: effectiveChannelDriverSelection,
|
||||
}),
|
||||
null,
|
||||
2,
|
||||
)}\n`,
|
||||
"utf8",
|
||||
);
|
||||
await publishQaSuiteArtifactFiles({
|
||||
outputDir: params.outputDir,
|
||||
files: [
|
||||
{ filePath: reportPath, content: report },
|
||||
...(evidence && writeEvidenceFile
|
||||
? [{ filePath: evidencePath, content: `${JSON.stringify(evidence, null, 2)}\n` }]
|
||||
: []),
|
||||
{
|
||||
filePath: summaryPath,
|
||||
content: `${JSON.stringify(
|
||||
buildQaSuiteSummaryJson({
|
||||
...params,
|
||||
channelDriverSelection: effectiveChannelDriverSelection,
|
||||
}),
|
||||
null,
|
||||
2,
|
||||
)}\n`,
|
||||
},
|
||||
],
|
||||
});
|
||||
await assertQaSuiteArtifactWritten("report", reportPath);
|
||||
await assertQaSuiteArtifactWritten("summary", summaryPath);
|
||||
if (evidence && writeEvidenceFile) {
|
||||
|
||||
@@ -4,8 +4,10 @@ import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { QaSuiteInfraError } from "./errors.js";
|
||||
import type { QaLabServerHandle } from "./lab-server.types.js";
|
||||
import type { QaTransportAdapter } from "./qa-transport.js";
|
||||
import { makeQaSuiteTestScenario } from "./suite-test-helpers.js";
|
||||
import type { QaSuiteScenarioResult } from "./suite.js";
|
||||
import { throwQaSuiteCleanupErrors } from "./suite.js";
|
||||
import { qaSuiteProgressTesting, throwQaSuiteCleanupErrors } from "./suite.js";
|
||||
import type {
|
||||
QaTestFileScenario,
|
||||
QaTestFileScenarioRunResult,
|
||||
@@ -14,11 +16,13 @@ import type {
|
||||
const {
|
||||
crablineRuntimeLoads,
|
||||
prepareDockerE2eEnvironment,
|
||||
replaceFileAtomicMock,
|
||||
runQaFlowSuite,
|
||||
runQaTestFileScenarios,
|
||||
} = vi.hoisted(() => ({
|
||||
crablineRuntimeLoads: vi.fn(),
|
||||
prepareDockerE2eEnvironment: vi.fn(),
|
||||
replaceFileAtomicMock: vi.fn(),
|
||||
runQaFlowSuite: vi.fn(),
|
||||
runQaTestFileScenarios: vi.fn(),
|
||||
}));
|
||||
@@ -43,6 +47,12 @@ vi.mock("./test-file-scenario-docker-batch.js", async (importOriginal) => ({
|
||||
prepareDockerE2eEnvironment,
|
||||
}));
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/security-runtime", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("openclaw/plugin-sdk/security-runtime")>();
|
||||
replaceFileAtomicMock.mockImplementation(actual.replaceFileAtomic);
|
||||
return { ...actual, replaceFileAtomic: replaceFileAtomicMock };
|
||||
});
|
||||
|
||||
import { runQaSuite, runQaSuiteWithInfraRetry } from "./suite-launch.runtime.js";
|
||||
|
||||
const tempRoots: string[] = [];
|
||||
@@ -120,8 +130,67 @@ function mockFlowPartitionFailures(failuresByScenarioId: ReadonlyMap<string, rea
|
||||
return attempts;
|
||||
}
|
||||
|
||||
async function expectArtifactPublicationFailurePreservesPrior(params: {
|
||||
canonicalFileNames: readonly string[];
|
||||
failedFileName: string;
|
||||
outputDir: string;
|
||||
publish: () => Promise<unknown>;
|
||||
}) {
|
||||
const sentinels = new Map(
|
||||
params.canonicalFileNames.map((fileName) => [fileName, `prior ${fileName}\n`]),
|
||||
);
|
||||
await fs.mkdir(params.outputDir, { recursive: true, mode: 0o750 });
|
||||
await fs.chmod(params.outputDir, 0o750);
|
||||
for (const [fileName, sentinel] of sentinels) {
|
||||
const finalPath = path.join(params.outputDir, fileName);
|
||||
await fs.writeFile(finalPath, sentinel, { encoding: "utf8", mode: 0o640 });
|
||||
await fs.chmod(finalPath, 0o640);
|
||||
}
|
||||
const actualSecurityRuntime = await vi.importActual<
|
||||
typeof import("openclaw/plugin-sdk/security-runtime")
|
||||
>("openclaw/plugin-sdk/security-runtime");
|
||||
const publicationOrder: string[] = [];
|
||||
const failSelectedArtifact = async (options: Parameters<typeof replaceFileAtomicMock>[0]) => {
|
||||
publicationOrder.push(path.basename(options.filePath));
|
||||
return await actualSecurityRuntime.replaceFileAtomic({
|
||||
...options,
|
||||
...(path.basename(options.filePath) === params.failedFileName
|
||||
? {
|
||||
beforeRename: async ({ tempPath }: { tempPath: string }) => {
|
||||
await fs.writeFile(tempPath, "partial replacement\n", "utf8");
|
||||
throw Object.assign(new Error("injected QA artifact publication failure"), {
|
||||
code: "EIO",
|
||||
});
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
};
|
||||
|
||||
await replaceFileAtomicMock.withImplementation(failSelectedArtifact, async () => {
|
||||
await expect(params.publish()).rejects.toMatchObject({ code: "EIO" });
|
||||
});
|
||||
|
||||
const selectedPath = path.join(params.outputDir, params.failedFileName);
|
||||
await expect(fs.readFile(selectedPath, "utf8")).resolves.toBe(
|
||||
sentinels.get(params.failedFileName),
|
||||
);
|
||||
if (process.platform !== "win32") {
|
||||
expect((await fs.stat(selectedPath)).mode & 0o777).toBe(0o640);
|
||||
expect((await fs.stat(params.outputDir)).mode & 0o7777).toBe(0o750);
|
||||
}
|
||||
const selectedIndex = params.canonicalFileNames.indexOf(params.failedFileName);
|
||||
expect(publicationOrder).toEqual(params.canonicalFileNames.slice(0, selectedIndex + 1));
|
||||
expect(
|
||||
(await fs.readdir(params.outputDir)).filter((entry) =>
|
||||
entry.startsWith(`${params.failedFileName}.qa-artifact.`),
|
||||
),
|
||||
).toEqual([]);
|
||||
}
|
||||
|
||||
describe("qa suite runtime launcher", () => {
|
||||
beforeEach(() => {
|
||||
replaceFileAtomicMock.mockClear();
|
||||
runQaFlowSuite.mockReset();
|
||||
runQaTestFileScenarios.mockReset();
|
||||
prepareDockerE2eEnvironment.mockReset();
|
||||
@@ -1225,6 +1294,62 @@ describe("qa suite runtime launcher", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ kind: "report", fileName: "qa-suite-report.md" },
|
||||
{ kind: "evidence", fileName: "qa-evidence.json" },
|
||||
{ kind: "summary", fileName: "qa-suite-summary.json" },
|
||||
])(
|
||||
"preserves the prior standard $kind artifact when atomic publication fails",
|
||||
async ({ fileName }) => {
|
||||
const outputDir = await makeTempRepo("qa-suite-standard-artifact-atomic-");
|
||||
await expectArtifactPublicationFailurePreservesPrior({
|
||||
canonicalFileNames: ["qa-suite-report.md", "qa-evidence.json", "qa-suite-summary.json"],
|
||||
failedFileName: fileName,
|
||||
outputDir,
|
||||
publish: async () =>
|
||||
await qaSuiteProgressTesting.writeQaSuiteArtifacts({
|
||||
outputDir,
|
||||
startedAt: new Date("2026-08-12T00:00:00.000Z"),
|
||||
finishedAt: new Date("2026-08-12T00:01:00.000Z"),
|
||||
scenarios: [{ name: "Atomic publication", status: "pass", steps: [] }],
|
||||
scenarioDefinitions: [makeQaSuiteTestScenario("channel-chat-baseline")],
|
||||
transport: {
|
||||
id: "qa-channel",
|
||||
createReportNotes: () => [],
|
||||
} as unknown as QaTransportAdapter,
|
||||
providerMode: "mock-openai",
|
||||
primaryModel: "mock-openai/gpt-5.6-luna",
|
||||
alternateModel: "mock-openai/gpt-5.6-luna-alt",
|
||||
fastMode: true,
|
||||
concurrency: 1,
|
||||
}),
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
{ kind: "evidence", fileName: "qa-evidence.json" },
|
||||
{ kind: "report", fileName: "qa-suite-report.md" },
|
||||
{ kind: "summary", fileName: "qa-suite-summary.json" },
|
||||
])(
|
||||
"preserves the prior unified $kind artifact when atomic publication fails",
|
||||
async ({ fileName }) => {
|
||||
const repoRoot = await makeTempRepo("qa-suite-unified-artifact-atomic-");
|
||||
const outputDir = path.join(repoRoot, ".artifacts", "qa-e2e", "artifact-atomic");
|
||||
await expectArtifactPublicationFailurePreservesPrior({
|
||||
canonicalFileNames: ["qa-evidence.json", "qa-suite-report.md", "qa-suite-summary.json"],
|
||||
failedFileName: fileName,
|
||||
outputDir,
|
||||
publish: async () =>
|
||||
await runQaSuite({
|
||||
repoRoot,
|
||||
outputDir: ".artifacts/qa-e2e/artifact-atomic",
|
||||
scenarioIds: ["control-ui-chat-flow-playwright"],
|
||||
}),
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("aggregates mixed-kind progress through the parent lab", async () => {
|
||||
const repoRoot = await makeTempRepo("qa-suite-mixed-progress-");
|
||||
const scenarioRuns: Array<Parameters<QaLabServerHandle["setScenarioRun"]>[0]> = [];
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
type QaSeedScenarioWithSource,
|
||||
} from "./scenario-catalog.js";
|
||||
import { expandQaScenarioExecutionCells, type QaScenarioExecutionCell } from "./scenario-lane.js";
|
||||
import { publishQaSuiteArtifactFiles } from "./suite-artifacts.js";
|
||||
import {
|
||||
mapQaSuiteWithConcurrency,
|
||||
normalizeQaSuiteConcurrency,
|
||||
@@ -705,7 +706,6 @@ async function writeUnifiedQaSuiteArtifacts(params: {
|
||||
scenarios: readonly QaSuiteScenarioResult[];
|
||||
startedAt: Date;
|
||||
}) {
|
||||
await fs.mkdir(params.outputDir, { recursive: true });
|
||||
const evidencePath = path.join(params.outputDir, QA_EVIDENCE_FILENAME);
|
||||
const reportPath = path.join(params.outputDir, "qa-suite-report.md");
|
||||
const summaryPath = path.join(params.outputDir, "qa-suite-summary.json");
|
||||
@@ -729,9 +729,14 @@ async function writeUnifiedQaSuiteArtifacts(params: {
|
||||
scenarios: [...params.scenarios],
|
||||
startedAt: params.startedAt,
|
||||
}) satisfies QaSuiteSummaryJson;
|
||||
await fs.writeFile(evidencePath, `${JSON.stringify(params.evidence, null, 2)}\n`, "utf8");
|
||||
await fs.writeFile(reportPath, report, "utf8");
|
||||
await fs.writeFile(summaryPath, `${JSON.stringify(summary, null, 2)}\n`, "utf8");
|
||||
await publishQaSuiteArtifactFiles({
|
||||
outputDir: params.outputDir,
|
||||
files: [
|
||||
{ filePath: evidencePath, content: `${JSON.stringify(params.evidence, null, 2)}\n` },
|
||||
{ filePath: reportPath, content: report },
|
||||
{ filePath: summaryPath, content: `${JSON.stringify(summary, null, 2)}\n` },
|
||||
],
|
||||
});
|
||||
return {
|
||||
evidencePath,
|
||||
outputDir: params.outputDir,
|
||||
|
||||
@@ -608,6 +608,15 @@ describe("qa suite", () => {
|
||||
evidence?: unknown;
|
||||
};
|
||||
expect(summary.evidence).toBeUndefined();
|
||||
if (process.platform !== "win32") {
|
||||
for (const artifactPath of [
|
||||
artifacts.reportPath,
|
||||
artifacts.evidencePath,
|
||||
artifacts.summaryPath,
|
||||
]) {
|
||||
expect((await fs.stat(artifactPath)).mode & 0o777).toBe(0o600);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
await fs.rm(outputDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user