ci(auto-format): run the format step through the steps script (#496)

Author: Vivswan

.github/scripts/auto-format-steps.ts

@@ -1,16 +1,37 @@
 /**
- * The auto-format.yml push step: the formatting patch the format job cut on HEAD_SHA, committed and pushed to the
- * PR branch under a lease on that commit.
+ * The two auto-format.yml steps: the format job's run of the formatter, cut into a patch for the push job, and
+ * that job's commit of the patch to the PR branch under a lease on the commit it was cut on.
  *
- *   push   push job   GH_TOKEN, HEAD_REF, HEAD_SHA, RUNNER_TEMP, GITHUB_REPOSITORY   -> the commit on the PR branch
+ *   format   format job   RUNNER_TEMP, GITHUB_OUTPUT                                   -> format.patch, head=, changed=
+ *   push     push job     GH_TOKEN, HEAD_REF, HEAD_SHA, RUNNER_TEMP, GITHUB_REPOSITORY   -> the commit on the PR branch
  *
  * Node builtins only: the push job installs nothing, so no git hook or lifecycle script exists where the write
  * token is.
  */
 
+import { statSync } from "node:fs";
 import { join } from "node:path";
 import { configureBotIdentity, leasePush } from "./lib/pr-branch.js";
-import { capture, requireEnv, run } from "./lib/workflow-step.js";
+import { capture, requireEnv, run, setOutput } from "./lib/workflow-step.js";
+
+function format(): void {
+  // The patch belongs to this commit alone; the push job refuses any other head.
+  setOutput("head", capture(["git", "rev-parse", "HEAD"]));
+  run(["bun", "run", "lint:fix"]);
+  // Anything the formatter left staged is not this workflow's fix: start from an empty index, then stage
+  // modifications and deletions of tracked files alone (a formatter adds nothing).
+  run(["git", "reset", "-q"]);
+  run(["git", "add", "-u"]);
+  const patch = join(requireEnv("RUNNER_TEMP"), "format.patch");
+  run(["git", "diff", "--cached", "--binary"], { stdoutFile: patch });
+  if (statSync(patch).size === 0) {
+    console.log("nothing to format");
+    setOutput("changed", "false");
+    return;
+  }
+  run(["git", "diff", "--cached", "--stat"]);
+  setOutput("changed", "true");
+}
 
 function push(): void {
   // bun has already read its bunfig.toml and .env from the trusted checkout it started in; only git runs in the
@@ -42,11 +63,13 @@ function push(): void {
 
 if (import.meta.main) {
   const command = process.argv[2];
-  if (command === "push") {
+  if (command === "format") {
+    format();
+  } else if (command === "push") {
     push();
   } else {
     console.error(
-      `auto-format-steps: unknown command ${JSON.stringify(command ?? null)}; expected push`,
+      `auto-format-steps: unknown command ${JSON.stringify(command ?? null)}; expected format | push`,
     );
     process.exit(1);
   }

.github/workflows/auto-format.yml

@@ -40,22 +40,7 @@ jobs:
       - uses: ./.github/actions/setup
       - name: Format (biome) and stage the patch
         id: format
-        run: |
-          # The patch belongs to this commit alone; the push job refuses any other head.
-          echo "head=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
-          bun run lint:fix
-          # Anything the formatter left staged is not this workflow's fix: start from an empty index, then stage
-          # modifications and deletions of tracked files alone (a formatter adds nothing).
-          git reset -q
-          git add -u
-          git diff --cached --binary > "$RUNNER_TEMP/format.patch"
-          if [ -s "$RUNNER_TEMP/format.patch" ]; then
-            git diff --cached --stat
-            echo "changed=true" >> "$GITHUB_OUTPUT"
-          else
-            echo "nothing to format"
-            echo "changed=false" >> "$GITHUB_OUTPUT"
-          fi
+        run: bun .github/scripts/auto-format-steps.ts format
       - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         if: steps.format.outputs.changed == 'true'
         with:

test/scripts/auto-format-steps.test.ts

@@ -1,10 +1,12 @@
 /**
- * auto-format.yml's push step against local fixture repositories: the branches the workflow text cannot show,
- * which checkout the patch is committed on, under whose identity, and what a moved branch does to the push.
+ * auto-format.yml's two steps against local fixture repositories: the branches the workflow text cannot show.
+ * For the format step, what the patch holds and when the step stops; for the push step, which checkout the patch
+ * is committed on, under whose identity, and what a moved branch does to the push.
  */
 
 import { describe, expect, setDefaultTimeout, test } from "bun:test";
 import { execFileSync } from "node:child_process";
+import { existsSync, readFileSync } from "node:fs";
 import { join } from "node:path";
 import {
   clone,
@@ -22,6 +24,67 @@ installReleasePipelineFixture();
 
 const REMOTE_URL = "https://x-access-token:t0ken@github.com/o/r.git";
 
+/** The format job's checkout of a same-repo PR branch: its lint:fix runs whatever LINT_FIX says. */
+function formatFixture(): { pr: string; headSha: string; temp: string } {
+  const fx = seedFixture();
+  const pr = clone(fx.root, fx.origin, "pr");
+  write(
+    pr,
+    "package.json",
+    `${JSON.stringify(
+      { name: "fixture", version: "0.0.0", scripts: { "lint:fix": 'sh -c "$LINT_FIX"' } },
+      null,
+      2,
+    )}\n`,
+  );
+  const headSha = commitAll(pr, "feat: a pull request");
+  return { pr, headSha, temp: join(fx.root, "runner-temp") };
+}
+
+const format = (pr: string, temp: string, lintFix: string) =>
+  runStep("auto-format-steps", pr, temp, { LINT_FIX: lintFix }, "format");
+
+function patchPaths(temp: string): string[] {
+  return [...readFileSync(join(temp, "format.patch"), "utf8").matchAll(/^diff --git a\/(\S+)/gm)]
+    .map((match) => match[1] ?? "")
+    .sort();
+}
+
+describe("format", () => {
+  test("a formatter that changes nothing leaves an empty patch and changed=false, and an addition staged before it is not the fix", () => {
+    const { pr, headSha, temp } = formatFixture();
+    write(pr, "staged.ts", "export const staged = 1;\n");
+    git(pr, "add", "staged.ts");
+    const result = format(pr, temp, "true");
+    expect(result).toMatchObject({
+      status: 0,
+      stdout: "nothing to format\n",
+      outputs: [`head=${headSha}`, "changed=false"],
+    });
+    expect(readFileSync(join(temp, "format.patch"), "utf8")).toBe("");
+    expect(git(pr, "diff", "--cached", "--name-only")).toBe("");
+  });
+
+  test("the formatter's modifications and deletions of tracked files enter the patch; a file it adds does not", () => {
+    const { pr, headSha, temp } = formatFixture();
+    const result = format(
+      pr,
+      temp,
+      "echo 'export const marker=1' > src/marker.ts && rm .github/dependabot.yml && echo new > added.ts",
+    );
+    expect(result).toMatchObject({ status: 0, outputs: [`head=${headSha}`, "changed=true"] });
+    expect(result.stdout).toContain("2 files changed, 1 insertion(+), 3 deletions(-)");
+    expect(patchPaths(temp)).toEqual([".github/dependabot.yml", "src/marker.ts"]);
+  });
+
+  test("a failing formatter ends the step with its status after head= is written, with no patch cut", () => {
+    const { pr, headSha, temp } = formatFixture();
+    const result = format(pr, temp, "exit 3");
+    expect(result).toMatchObject({ status: 3, outputs: [`head=${headSha}`] });
+    expect(existsSync(join(temp, "format.patch"))).toBe(false);
+  });
+});
+
 /** The push job's checkout of a same-repo PR branch, with the format job's patch downloaded beside it. */
 function prFixture(): { fx: Fixture; pr: string; headSha: string; temp: string } {
   const fx = seedFixture();