ci: do not skip the build for lint rule pages, and check compiler docs links on docs-only pull requests

.buildkite/ci.ts

@@ -31,6 +31,7 @@ import {
   getRepositoryUrl,
   getSecret,
   isBuildkite,
+  isDocsOnlyChange,
   isFork,
   isMainBranch,
   isMergeQueue,
@@ -1985,7 +1986,7 @@ async function main() {
     } catch (e) {
       console.error(e);
     }
-    if (allFiles.length > 0 && allFiles.every(filename => filename.startsWith("docs/"))) {
+    if (isDocsOnlyChange(allFiles)) {
       console.log(`- PR is only docs, skipping tests!`);
       return;
     }

.github/workflows/source-lints.yml

@@ -24,9 +24,11 @@ on:
       - "scripts/glob-sources.ts"
       - "scripts/agent.ts"
       - "scripts/buildkite.ts"
+      - "scripts/runner.node.ts"
       - ".buildkite/ci.ts"
       - ".buildkite/scripts/upload-release.sh"
       - "packages/bun-release/src/platform.ts"
+      - "docs/**"
       - "rust-toolchain.toml"
       - "bun.lock"
       - "test/bun.lock"
@@ -53,9 +55,11 @@ on:
       - "scripts/glob-sources.ts"
       - "scripts/agent.ts"
       - "scripts/buildkite.ts"
+      - "scripts/runner.node.ts"
       - ".buildkite/ci.ts"
       - ".buildkite/scripts/upload-release.sh"
       - "packages/bun-release/src/platform.ts"
+      - "docs/**"
       - "rust-toolchain.toml"
       - "bun.lock"
       - "test/bun.lock"

scripts/buildkite.ts

@@ -426,11 +426,21 @@ export async function getLastSuccessfulBuild(): Promise<BuildkiteBuild | undefin
 interface GithubPullRequestFile {
   filename: string;
   status: string;
+  /** Where a moved file was, or what a copied file was copied from. */
+  previous_filename?: string;
 }
 
 /**
- * The files of the pull request this Buildkite build is for, and the ones
- * among them that it adds; none when the build is not for a pull request.
+ * The paths that a pull request changes with one file. GitHub lists a moved file once, under its new path. The file is
+ * gone from its old path, so that path changes too. The source of a copy does not change.
+ */
+export function pullRequestFilePaths({ filename, status, previous_filename }: GithubPullRequestFile): string[] {
+  return status === "renamed" && previous_filename ? [filename, previous_filename] : [filename];
+}
+
+/**
+ * The paths that the pull request of this Buildkite build changes, and the
+ * files among them that it adds; none when the build is not for a pull request.
  */
 export async function getPullRequestFiles(): Promise<{ allFiles: string[]; newFiles: string[] }> {
   const allFiles: string[] = [];
@@ -450,10 +460,10 @@ export async function getPullRequestFiles(): Promise<{ allFiles: string[]; newFi
       throw new Error(`GitHub API ${response.status} ${response.statusText} on page ${page}: ${JSON.stringify(files)}`);
     }
     console.log(`-> page ${page}, found ${files.length} items`);
-    for (const { filename, status } of files) {
-      allFiles.push(filename);
-      if (status === "added") {
-        newFiles.push(filename);
+    for (const file of files) {
+      allFiles.push(...pullRequestFilePaths(file));
+      if (file.status === "added") {
+        newFiles.push(file.filename);
       }
     }
     if (files.length < perPage) {
@@ -464,6 +474,24 @@ export async function getPullRequestFiles(): Promise<{ allFiles: string[]; newFi
   return { allFiles, newFiles };
 }
 
+/**
+ * Whether CI skips a pull request that changes these paths: no build of bun, and no test of the suite. Each path is
+ * under docs/, and none is under docs/lint/rules/. The build embeds the rule pages in bun (`lintRuleDocs` in
+ * scripts/glob-sources.ts), and `index.mdx` and `reports.json` beside them are written from what that bun prints.
+ *
+ * Tests of the suite also read pages of docs/compiler. Those pages stay in the skip on purpose: a docs-only edit of
+ * one is first tested on main.
+ *
+ * A query that fails leaves an empty list, so an empty list is never docs-only.
+ */
+export function isDocsOnlyChange(files: string[]): boolean {
+  return (
+    files.length > 0 &&
+    files.every(file => file.startsWith("docs/")) &&
+    !files.some(file => file.startsWith("docs/lint/rules/"))
+  );
+}
+
 /**
  * `filename` is the absolute path to the file to upload.
  */

scripts/runner.node.ts

@@ -70,6 +70,7 @@ import {
   getSecret,
   isBuildkite,
   isCI,
+  isDocsOnlyChange,
   isGithubAction,
   markBuildkiteStepReported,
   parseJunitFileSuites,
@@ -4245,10 +4246,9 @@ async function main(): Promise<void> {
   let doRunTests = true;
   if (isCI) {
     // allFiles can be empty if the GitHub API call failed (bad token, rate
-    // limit, non-PR build). [].every() is vacuously true, which would skip the
-    // entire suite and exit 0 — so require at least one file before treating
-    // the change set as docs-only.
-    if (allFiles.length > 0 && allFiles.every(filename => filename.startsWith("docs/"))) {
+    // limit, non-PR build). An empty list is never docs-only: to skip the
+    // entire suite and exit 0 then would report green without a test.
+    if (isDocsOnlyChange(allFiles)) {
       doRunTests = false;
     }
   }

test/internal/source-lints/README.md

@@ -17,8 +17,14 @@ only import built-ins, relative paths, and `harness` (resolved via
 `test/tsconfig.json` paths).
 
 The workflow only triggers for the `paths:` listed in it. A lint that reads
-files outside those paths (for example `packages/bun-types/bun.d.ts` or the
-docs) needs them added there, or a change to those files is only checked by
-whichever later PR happens to trigger the workflow.
+files outside those paths (for example `packages/bun-types/bun.d.ts`) needs
+them added there, or a change to those files is only checked by whichever
+later PR happens to trigger the workflow.
+
+`docs/**` is listed for `compiler-docs-links.test.ts`, which judges the links
+from and to `docs/compiler`, and the Compiler tab and the redirects of
+`docs/docs.json`. A pull request that changes only docs gets no Buildkite
+pipeline (`isDocsOnlyChange()` in `scripts/buildkite.ts`), so no test outside
+this directory reads its pages.
 
 To run locally: `bun test test/internal/source-lints/`.

test/cli/compiler-docs-links.test.ts → test/internal/source-lints/compiler-docs-links.test.ts

@@ -4,7 +4,7 @@ import { expect, test } from "bun:test";
 import { existsSync, readFileSync } from "fs";
 import { join } from "path";
 
-const docs = join(import.meta.dir, "..", "..", "docs");
+const docs = join(import.meta.dir, "..", "..", "..", "docs");
 const site = JSON.parse(readFileSync(join(docs, "docs.json"), "utf8"));
 const readWithoutCode = (file: string) => readFileSync(file, "utf8").replace(/^([ \t]*)```.*?^\1```/gms, "");
 const isFile = (file: string) => /\.\w+$/.test(file) && existsSync(file);

test/internal/source-lints/docs-only-change.test.ts

@@ -0,0 +1,86 @@
+// A pull request whose paths `isDocsOnlyChange()` (scripts/buildkite.ts) accepts gets no Buildkite pipeline: bun is not
+// built and the suite does not run. So it must refuse the files that the build puts into bun.
+import { expect, test } from "bun:test";
+import { readFileSync } from "node:fs";
+import { join, relative, resolve } from "node:path";
+import { isDocsOnlyChange, pullRequestFilePaths } from "../../../scripts/buildkite.ts";
+import { globAllSources } from "../../../scripts/glob-sources.ts";
+
+const root = resolve(import.meta.dir, "..", "..", "..");
+const inSourceLists = Object.values(globAllSources())
+  .flat()
+  .map(file => relative(root, file).replaceAll("\\", "/"));
+
+test("a change is docs-only when each path is under docs/ and none is under docs/lint/rules/", () => {
+  const cases: [paths: string[], docsOnly: boolean][] = [
+    [["docs/bundler/esbuild.mdx"], true],
+    [["docs/docs.json", "docs/compiler/types.mdx", "docs/lint/index.mdx"], true],
+    // What a failed query for the files leaves.
+    [[], false],
+    [["docs/bundler/esbuild.mdx", "src/cli.rs"], false],
+    [["README.md"], false],
+    [["packages/bun-types/docs/README.md"], false],
+    // The build embeds the rule pages in bun, and `bun lint --rules` of that bun gives the two files beside them.
+    [["docs/lint/rules/eslint/eqeqeq.mdx"], false],
+    [["docs/lint/rules/index.mdx"], false],
+    [["docs/lint/rules/reports.json"], false],
+    [["docs/bundler/esbuild.mdx", "docs/lint/rules/eslint/eqeqeq.mdx"], false],
+  ];
+  expect(cases.map(([paths]) => [paths, isDocsOnlyChange(paths)])).toEqual(cases);
+});
+
+test("a moved file counts under its old path too", () => {
+  const moved = (from: string, to: string) =>
+    pullRequestFilePaths({ filename: to, status: "renamed", previous_filename: from });
+  expect({
+    "a changed file": pullRequestFilePaths({ filename: "docs/index.mdx", status: "modified" }),
+    "a moved file": moved("docs/runtime/a.mdx", "docs/runtime/b.mdx"),
+    // The pull request does not change what it copies from.
+    "a copied file": pullRequestFilePaths({
+      filename: "scripts/agent-copy.ts",
+      status: "copied",
+      previous_filename: "scripts/agent.ts",
+    }),
+  }).toEqual({
+    "a changed file": ["docs/index.mdx"],
+    "a moved file": ["docs/runtime/b.mdx", "docs/runtime/a.mdx"],
+    "a copied file": ["scripts/agent-copy.ts"],
+  });
+  expect({
+    "a page moved inside docs/": isDocsOnlyChange(moved("docs/runtime/a.mdx", "docs/runtime/b.mdx")),
+    "a rule page moved out of docs/lint/rules/": isDocsOnlyChange(
+      moved("docs/lint/rules/eslint/eqeqeq.mdx", "docs/lint/eqeqeq.mdx"),
+    ),
+    "a source file moved into docs/": isDocsOnlyChange(moved("src/cli/README.md", "docs/project/cli.md")),
+  }).toEqual({
+    "a page moved inside docs/": true,
+    "a rule page moved out of docs/lint/rules/": false,
+    "a source file moved into docs/": false,
+  });
+});
+
+// This sees the source lists. It does not see a file that the build reaches another way: a fixed path in
+// scripts/build/codegen.ts, or an `include_str!`.
+test("no file in a source list of scripts/glob-sources.ts is docs-only", () => {
+  // Without a docs file in the source lists, the next assertion would hold for any rule.
+  expect(inSourceLists.some(file => file.startsWith("docs/"))).toBe(true);
+  expect(inSourceLists.filter(file => isDocsOnlyChange([file]))).toEqual([]);
+});
+
+// Both scripts do their work when they are imported, so they are read as text.
+test("the pipeline step and the test runner decide with isDocsOnlyChange()", () => {
+  // The line above each place where `statement` is: the condition that the skip is under.
+  const linesAbove = (file: string, statement: string) => {
+    const lines = readFileSync(join(root, file), "utf8")
+      .split(/\r?\n/)
+      .map(line => line.trim());
+    return lines.flatMap((line, index) => (line === statement ? [lines[index - 1]] : []));
+  };
+  expect({
+    ".buildkite/ci.ts": linesAbove(".buildkite/ci.ts", "console.log(`- PR is only docs, skipping tests!`);"),
+    "scripts/runner.node.ts": linesAbove("scripts/runner.node.ts", "doRunTests = false;"),
+  }).toEqual({
+    ".buildkite/ci.ts": ["if (isDocsOnlyChange(allFiles)) {"],
+    "scripts/runner.node.ts": ["if (isDocsOnlyChange(allFiles)) {"],
+  });
+});