mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 09:39:25 +00:00
fix(maintainer): keep outer PR controls out of test fixtures (#157755)
* fix(update): restore npm-pack helper typechecking (#157659) Use the canonical UpdateStepResult type owner after its concurrent mainline extraction. The old module no longer exports the type. This corrects TS2459 without changing emitted runtime JavaScript. Follow-up to #153236. Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com> * test(maintainer): isolate nested PR fixture environments --------- Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
This commit is contained in:
Peter Steinberger
GitHub
parent
136ce55f29
commit
b1b0b487f7
3 changed files with 50 additions and 25 deletions
|
|
@ -38,7 +38,11 @@ import {
|
|||
validReview,
|
||||
writeReviewArtifacts,
|
||||
} from "./pr-review-artifact-fixture.js";
|
||||
import { copyPrWrapperSources, linkPrWrapperDependencies } from "./pr-wrapper.test-support.js";
|
||||
import {
|
||||
copyPrWrapperSources,
|
||||
createIndependentPrFixtureEnv,
|
||||
linkPrWrapperDependencies,
|
||||
} from "./pr-wrapper.test-support.js";
|
||||
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
const freshMainTemplateDirs = useAutoCleanupTempDirTracker(afterAll);
|
||||
|
|
@ -78,7 +82,11 @@ function realpathSpecialFixtureWithNode(filePath: string): string {
|
|||
}
|
||||
|
||||
function spawnDetached(command: string, args: readonly string[], options: SpawnOptions = {}) {
|
||||
const child = spawn(command, args, { ...options, detached: true });
|
||||
const child = spawn(command, args, {
|
||||
env: createIndependentPrFixtureEnv(),
|
||||
...options,
|
||||
detached: true,
|
||||
});
|
||||
detachedChildren.add(child);
|
||||
if (child.pid) {
|
||||
goneProcessGroups.delete(child.pid);
|
||||
|
|
@ -210,16 +218,9 @@ function expectMaterializedWorktree(worktreeDir: string) {
|
|||
).toBe("false");
|
||||
}
|
||||
|
||||
function bashSource(repoDir: string, supervised = false) {
|
||||
function bashSource(repoDir: string) {
|
||||
return [
|
||||
"set -euo pipefail",
|
||||
...(supervised
|
||||
? []
|
||||
: [
|
||||
"unset OPENCLAW_PR_LOCK_NOTIFY_FD",
|
||||
"unset OPENCLAW_PR_LOCK_SUPERVISOR_PID",
|
||||
"unset OPENCLAW_PR_GITHUB_SNAPSHOT_ROOT",
|
||||
]),
|
||||
`source '${worktreeScript}'`,
|
||||
`source '${lockScript}'`,
|
||||
`source '${commonScript}'`,
|
||||
|
|
@ -241,7 +242,7 @@ function writeOperationFixture(repoDir: string, name: string, commands: string[]
|
|||
const fixture = writeFixtureFile(
|
||||
repoDir,
|
||||
name,
|
||||
["#!/usr/bin/env bash", ...bashSource(repoDir, true), ...commands].join("\n"),
|
||||
["#!/usr/bin/env bash", ...bashSource(repoDir), ...commands].join("\n"),
|
||||
);
|
||||
chmodSync(fixture, 0o755);
|
||||
return fixture;
|
||||
|
|
@ -385,7 +386,7 @@ async function runSupervisedFixture(
|
|||
],
|
||||
{
|
||||
cwd: repoDir,
|
||||
env: { ...process.env, ...options.env },
|
||||
env: { ...createIndependentPrFixtureEnv(), ...options.env },
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
},
|
||||
);
|
||||
|
|
@ -455,7 +456,7 @@ function runLockShell(
|
|||
) {
|
||||
return spawnSync("bash", ["-c", [...bashSource(repoDir), ...commands].join("\n")], {
|
||||
cwd: repoDir,
|
||||
env: parentEnv,
|
||||
env: createIndependentPrFixtureEnv(parentEnv),
|
||||
detached: true,
|
||||
encoding: "utf8",
|
||||
timeout: 10_000,
|
||||
|
|
@ -789,7 +790,7 @@ describe("scripts/pr process-group platform guard", () => {
|
|||
const child = spawnDetached("bash", [fixture], {
|
||||
cwd: repoDir,
|
||||
env: {
|
||||
...process.env,
|
||||
...createIndependentPrFixtureEnv(),
|
||||
OPENCLAW_PR_DEDICATED_PROCESS_GROUP: "1",
|
||||
OPENCLAW_PR_LOCK_NOTIFY_FD: "3",
|
||||
OPENCLAW_PR_LOCK_SUPERVISOR_PID: String(process.pid),
|
||||
|
|
@ -808,11 +809,13 @@ describe("scripts/pr process-group platform guard", () => {
|
|||
|
||||
const describePosix = process.platform === "win32" ? describe.skip : describe;
|
||||
describePosix("scripts/pr per-PR operation lock", () => {
|
||||
it("isolates unsupervised candidate-source fixtures from unrelated supervisor bindings", () => {
|
||||
it("isolates independent fixtures from inherited maintainer bindings", () => {
|
||||
const repoDir = createRepo();
|
||||
const result = runLockShell(
|
||||
repoDir,
|
||||
[
|
||||
'test -z "${OPENCLAW_PR_GIT-}"',
|
||||
'test -z "${GIT_EXEC-}"',
|
||||
'test -z "${OPENCLAW_PR_GITHUB_SNAPSHOT_ROOT-}"',
|
||||
'test -z "${OPENCLAW_PR_LOCK_NOTIFY_FD-}"',
|
||||
'test -z "${OPENCLAW_PR_LOCK_SUPERVISOR_PID-}"',
|
||||
|
|
@ -822,6 +825,8 @@ describePosix("scripts/pr per-PR operation lock", () => {
|
|||
],
|
||||
{
|
||||
...process.env,
|
||||
OPENCLAW_PR_GIT: "/bin/false",
|
||||
GIT_EXEC: "/bin/false",
|
||||
OPENCLAW_PR_GITHUB_SNAPSHOT_ROOT: tempDirs.make("unrelated-lock-snapshot-"),
|
||||
OPENCLAW_PR_LOCK_NOTIFY_FD: "3",
|
||||
OPENCLAW_PR_LOCK_SUPERVISOR_PID: "1",
|
||||
|
|
@ -1446,7 +1451,7 @@ describePosix("scripts/pr per-PR operation lock", () => {
|
|||
{
|
||||
cwd: repoDir,
|
||||
encoding: "utf8",
|
||||
env: { ...process.env, PATH: binDir },
|
||||
env: { ...createIndependentPrFixtureEnv(), PATH: binDir },
|
||||
},
|
||||
);
|
||||
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
|
||||
|
|
@ -1474,12 +1479,10 @@ describePosix("scripts/pr per-PR operation lock", () => {
|
|||
writeFileSync(reviewScript, `${readFileSync(reviewScript, "utf8")}\nreview_init() { :; }\n`);
|
||||
installRequiredPrCommandStubs(binDir);
|
||||
const env: NodeJS.ProcessEnv = {
|
||||
...process.env,
|
||||
...createIndependentPrFixtureEnv(),
|
||||
OPENCLAW_PR_DEDICATED_PROCESS_GROUP: "1",
|
||||
PATH: `${binDir}:${process.env.PATH ?? ""}`,
|
||||
};
|
||||
delete env.OPENCLAW_PR_LOCK_NOTIFY_FD;
|
||||
delete env.OPENCLAW_PR_LOCK_SUPERVISOR_PID;
|
||||
const result = spawnSync(cli, ["review-init", "42"], {
|
||||
cwd: repoDir,
|
||||
encoding: "utf8",
|
||||
|
|
@ -1504,7 +1507,7 @@ describePosix("scripts/pr per-PR operation lock", () => {
|
|||
const result = spawnSync(cli, ["review-init", "42"], {
|
||||
cwd: repoDir,
|
||||
encoding: "utf8",
|
||||
env: { ...process.env, PATH: `${binDir}:${process.env.PATH ?? ""}` },
|
||||
env: { ...createIndependentPrFixtureEnv(), PATH: `${binDir}:${process.env.PATH ?? ""}` },
|
||||
});
|
||||
|
||||
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(1);
|
||||
|
|
@ -1529,7 +1532,7 @@ describePosix("scripts/pr per-PR operation lock", () => {
|
|||
const result = spawnSync(cli, ["review-init", "42"], {
|
||||
cwd: repoDir,
|
||||
encoding: "utf8",
|
||||
env: { ...process.env, PATH: `${binDir}:${process.env.PATH ?? ""}` },
|
||||
env: { ...createIndependentPrFixtureEnv(), PATH: `${binDir}:${process.env.PATH ?? ""}` },
|
||||
});
|
||||
|
||||
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(1);
|
||||
|
|
@ -2036,7 +2039,7 @@ describePosix("scripts/pr per-PR operation lock", () => {
|
|||
// Linked landing verifies the full transitive anchor before starting cleanup.
|
||||
timeout: wrapper === "linked" ? 120_000 : 15_000,
|
||||
env: {
|
||||
...process.env,
|
||||
...createIndependentPrFixtureEnv(),
|
||||
canonical_repo_root: join(repoDir, "untrusted-root"),
|
||||
OPENCLAW_GH_BIN: gh,
|
||||
GH_REPO: "fixture/repo",
|
||||
|
|
@ -2947,7 +2950,7 @@ describePosix("scripts/pr per-PR operation lock", () => {
|
|||
cwd: repoDir,
|
||||
// The test deliberately kills the relay before its managed claim can release.
|
||||
// Only this fixture's independent group census may dispose its retained inputs.
|
||||
env: { ...process.env, TMPDIR: repoDir, TMP: repoDir, TEMP: repoDir },
|
||||
env: { ...createIndependentPrFixtureEnv(), TMPDIR: repoDir, TMP: repoDir, TEMP: repoDir },
|
||||
stdio: "ignore",
|
||||
});
|
||||
let nestedPgid: number | undefined;
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import {
|
|||
import { join } from "node:path";
|
||||
import { afterAll, afterEach, describe, expect, it } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";
|
||||
import { createIndependentPrFixtureEnv } from "./pr-wrapper.test-support.js";
|
||||
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
const templateDirs = useAutoCleanupTempDirTracker(afterAll);
|
||||
|
|
@ -167,7 +168,7 @@ function runShell(fixture: Fixture, commands: string[], env?: NodeJS.ProcessEnv)
|
|||
reviewScript,
|
||||
fixture.root,
|
||||
],
|
||||
{ cwd: fixture.root, encoding: "utf8", env: { ...process.env, ...env } },
|
||||
{ cwd: fixture.root, encoding: "utf8", env: { ...createIndependentPrFixtureEnv(), ...env } },
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -278,7 +279,11 @@ describePosix("scripts/pr worktree containment", () => {
|
|||
fixture.root,
|
||||
String(pr),
|
||||
],
|
||||
{ cwd: fixture.root, stdio: ["ignore", "pipe", "pipe"] },
|
||||
{
|
||||
cwd: fixture.root,
|
||||
env: createIndependentPrFixtureEnv(),
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
},
|
||||
);
|
||||
let output = "";
|
||||
child.stdout.on("data", (chunk) => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,23 @@
|
|||
import { cpSync, lstatSync, mkdirSync, readFileSync, realpathSync, symlinkSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
export function createIndependentPrFixtureEnv(
|
||||
parentEnv: NodeJS.ProcessEnv = process.env,
|
||||
): NodeJS.ProcessEnv {
|
||||
const env = { ...parentEnv };
|
||||
// Independent fixtures own their Git selection and create fresh supervisor bindings.
|
||||
for (const key of [
|
||||
"OPENCLAW_PR_GIT",
|
||||
"GIT_EXEC",
|
||||
"OPENCLAW_PR_GITHUB_SNAPSHOT_ROOT",
|
||||
"OPENCLAW_PR_LOCK_NOTIFY_FD",
|
||||
"OPENCLAW_PR_LOCK_SUPERVISOR_PID",
|
||||
]) {
|
||||
delete env[key];
|
||||
}
|
||||
return env;
|
||||
}
|
||||
|
||||
export function copyPrWrapperSources(destination: string): string[] {
|
||||
// Keep fixture sources and commits on the production inventory. Extracted
|
||||
// execution tests catch missing dependencies without a second source list.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue