test: stop two fixtures reading the machine they run on
Both CI legs were red, each on a different file, and both for the same
reason: a fixture asserted against ambient git state that happens to be
one way on a developer's machine and another on a runner.
`banner.test.ts` ran every case with `cwd` set to `process.cwd()`, and its
header said why that was safe — the repo is a repo, so the unrelated "not
a git repository" warning never fires. That stopped being true one commit
earlier: the launch warning is unconditional now and `gitStatus` also
checks for a commit identity, which `actions/checkout` does not configure.
So seven `expect(out).toBe("")` cases picked up a git line on ubuntu and
none locally. Each case is about a *flag* warning, so the git half has to
be a constant: the file builds its own repository — work tree, one commit,
a local identity that outranks whatever the machine has — and asserts
against that.
`REPO` is minted at declaration rather than in `beforeAll`, because the
teardown is a recursive force-delete of whatever it names and a binding
assigned in a hook can be something else entirely if that hook throws
first. This is not hypothetical; it deleted `apps/cli` once while this was
being written.
`diff-capture.test.ts` had no `core.autocrlf` pin, and `core.autocrlf=true`
is the Git-for-Windows default. `fileAtHead` reads through `cat-file
--filters`, which applies the conversion a checkout would, so on the
windows leg the LF blob came back CRLF and two `before` assertions differed
by every line terminator. The code is doing exactly what it was changed to
do; the fixture was a repo whose disk is LF while its config says CRLF,
which is not a shape a real checkout takes. Pinned the way
`@airship/git`'s fixture already pins it, with the same reasoning. The
`--filters` conversion keeps its own coverage there, driven by a
`.gitattributes` that reproduces it on every platform.
Also the case that was missing: a work tree with a HEAD and no identity —
exactly what a CI checkout is — now has a test, with the global and system
config scrubbed so a developer's own identity cannot make it pass
vacuously.
Verified by reproducing both failures locally: `core.autocrlf=true` in a
stand-in global config fails the same two diff-capture cases with the same
assertion text as the windows leg, and passes with the pin.
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
@@ -24,11 +25,28 @@ import { setColorEnabled } from "./terminal";
|
||||
* sonnet`, which opencode cannot resolve and drops on the floor, launched clean.
|
||||
* Nothing failed, because nothing was looking.
|
||||
*
|
||||
* `cwd` is this repo throughout, so the unrelated "not a git repository" warning
|
||||
* never fires and each case asserts on the one line it is about.
|
||||
* Every case runs against a repository this file builds, rather than against
|
||||
* `process.cwd()`. The git warning is unconditional now and `gitStatus` checks
|
||||
* for a commit identity, and a CI checkout has none — `actions/checkout` sets
|
||||
* no `user.name`/`user.email` — so asserting an empty stderr against the ambient
|
||||
* repo passed on a developer's machine and failed on every runner. What each
|
||||
* case is about is the flag warning, so the git half has to be a constant.
|
||||
*/
|
||||
|
||||
const REPO = process.cwd();
|
||||
/**
|
||||
* A repository `gitStatus` reports no error for: work tree, HEAD, identity.
|
||||
*
|
||||
* Minted here rather than in `beforeAll`, and `const`, so that it is a freshly
|
||||
* created temp directory from the first statement of this file onwards. The
|
||||
* teardown below is a recursive force-delete of whatever this names, and a
|
||||
* `let` assigned in a hook can be something else entirely if that hook throws
|
||||
* before reaching the assignment.
|
||||
*/
|
||||
const REPO = mkdtempSync(join(tmpdir(), "airship-banner-repo-"));
|
||||
|
||||
function git(cwd: string, ...args: string[]): void {
|
||||
execFileSync("git", args, { cwd, stdio: "ignore" });
|
||||
}
|
||||
|
||||
/** Everything written to stderr while `run` executes. */
|
||||
function stderrFrom(run: () => void): string {
|
||||
@@ -47,6 +65,25 @@ function stderrFrom(run: () => void): string {
|
||||
return out;
|
||||
}
|
||||
|
||||
beforeAll(() => {
|
||||
// `-b main` keeps `init.defaultBranch` advice off stderr, and the identity is
|
||||
// set locally so it outranks whatever the machine running this has — or does
|
||||
// not have, which is the case that broke.
|
||||
git(REPO, "init", "-q", "-b", "main");
|
||||
git(REPO, "config", "user.email", "test@example.com");
|
||||
git(REPO, "config", "user.name", "Test");
|
||||
git(REPO, "config", "commit.gpgsign", "false");
|
||||
writeFileSync(join(REPO, "file.txt"), "one\n");
|
||||
git(REPO, "add", "-A");
|
||||
// Without HEAD, `gitStatus` reports "no commits yet" and every case below
|
||||
// picks up a warning it is not about.
|
||||
git(REPO, "commit", "-q", "-m", "initial");
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
rmSync(REPO, { force: true, maxRetries: 3, recursive: true });
|
||||
});
|
||||
|
||||
describe("warnBackendLimits", () => {
|
||||
beforeAll(() => {
|
||||
// Assertions match on characters, not on ANSI.
|
||||
@@ -228,6 +265,63 @@ describe("warnBackendLimits", () => {
|
||||
stderrFrom(() => warnBackendLimits({ agent: "claude", cwd: REPO }))
|
||||
).toBe("");
|
||||
});
|
||||
|
||||
/*
|
||||
* A work tree with a HEAD and no `user.email`/`user.name` — which is what
|
||||
* every CI checkout is, since `actions/checkout` configures neither. It is
|
||||
* a repository by every other measure, so it reads healthy right up to the
|
||||
* commit that fails, and it is the case that turned this file red on the
|
||||
* runner while passing on the machine that wrote it.
|
||||
*/
|
||||
it("warns for a repository with no commit identity", () => {
|
||||
const anon = mkdtempSync(join(tmpdir(), "airship-banner-anon-"));
|
||||
// The identity is set only long enough to make the commit, then removed:
|
||||
// `gitStatus` wants a HEAD before it looks at the identity at all.
|
||||
git(anon, "init", "-q", "-b", "main");
|
||||
git(anon, "config", "user.email", "test@example.com");
|
||||
git(anon, "config", "user.name", "Test");
|
||||
git(anon, "config", "commit.gpgsign", "false");
|
||||
writeFileSync(join(anon, "file.txt"), "one\n");
|
||||
git(anon, "add", "-A");
|
||||
git(anon, "commit", "-q", "-m", "initial");
|
||||
git(anon, "config", "--unset", "user.email");
|
||||
git(anon, "config", "--unset", "user.name");
|
||||
|
||||
// The developer's own global config must not decide this — with one, the
|
||||
// repo resolves an identity and the case passes vacuously.
|
||||
// `GIT_CONFIG_GLOBAL=/dev/null` is not portable; an empty file is.
|
||||
const empty = join(anon, ".gitconfig-empty");
|
||||
writeFileSync(empty, "");
|
||||
const priorGlobal = process.env.GIT_CONFIG_GLOBAL;
|
||||
const priorSystem = process.env.GIT_CONFIG_SYSTEM;
|
||||
process.env.GIT_CONFIG_GLOBAL = empty;
|
||||
process.env.GIT_CONFIG_SYSTEM = empty;
|
||||
try {
|
||||
const out = stderrFrom(() =>
|
||||
warnBackendLimits({ agent: "claude", cwd: anon })
|
||||
);
|
||||
|
||||
expect(out).toContain("no commit identity");
|
||||
expect(out).toContain(anon);
|
||||
// The hint is the fix, and it is the whole reason this warns at launch
|
||||
// rather than at the first click on Commit.
|
||||
expect(out).toContain("git config --global user.email");
|
||||
} finally {
|
||||
// `delete`, not `= undefined`: assigning to `process.env` coerces, so
|
||||
// that would leave the literal string "undefined" behind as a path.
|
||||
if (priorGlobal === undefined) {
|
||||
delete process.env.GIT_CONFIG_GLOBAL;
|
||||
} else {
|
||||
process.env.GIT_CONFIG_GLOBAL = priorGlobal;
|
||||
}
|
||||
if (priorSystem === undefined) {
|
||||
delete process.env.GIT_CONFIG_SYSTEM;
|
||||
} else {
|
||||
process.env.GIT_CONFIG_SYSTEM = priorSystem;
|
||||
}
|
||||
rmSync(anon, { force: true, maxRetries: 3, recursive: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -37,6 +37,15 @@ beforeEach(() => {
|
||||
git("config", "user.email", "test@example.com");
|
||||
git("config", "user.name", "Test");
|
||||
git("config", "commit.gpgsign", "false");
|
||||
// Pinned, so a runner or developer with `core.autocrlf=true` does not turn
|
||||
// the non-EOL cases into a coin flip. `fileAtHead` reads through `cat-file
|
||||
// --filters`, which applies the same conversion a checkout would — so under
|
||||
// the Git-for-Windows default an LF blob comes back CRLF and every `before`
|
||||
// asserted here differs by its line terminators. The CRLF cases below write
|
||||
// their own bytes and never go through git, so they are unaffected; the
|
||||
// `--filters` conversion itself is covered in @airship/git, against a
|
||||
// `.gitattributes` that reproduces it on every platform.
|
||||
git("config", "core.autocrlf", "false");
|
||||
write("committed.txt", "one\ntwo\nthree\n");
|
||||
write("dirty.txt", "original\n");
|
||||
git("add", "-A");
|
||||
|
||||
Reference in New Issue
Block a user