Merge pull request #21598 from calixteman/use_per_test

Download the per-test coverage index

README.md

@@ -136,14 +136,31 @@ browsable HTML report instead, or pass several formats at once, e.g.
 
 ### Finding which tests cover a given line
 
-Run a browser test task with `--coverage-per-test` to build an index
+`coverage_search` lists the ref tests that exercised a specific source line or
-(`per-test-index.json`) in the coverage directory, then query it to list the
+function. It uses the per-test index (`per-test-index.json`) that is rebuilt on
-tests that exercised a specific source line or function:
+every push to `master` and published to the
+[`pdf.js.refs`](https://github.com/mozilla/pdf.js.refs/tree/gh-pages)
+repository. The index is downloaded on demand, cached locally, and only
+re-downloaded when it has changed, so no local coverage build is required:
 
-    $ npx gulp botbrowsertest --coverage-per-test
     $ npx gulp coverage_search --code="canvas.js::205"
     $ npx gulp coverage_search --code="canvas.js::drawImageAtIntegerCoords"
 
+To run — or regenerate the reference images for — only the ref tests that touch
+a given line or function, pass the same `--code` option to a browser test or
+`makeref` task:
+
+    $ npx gulp browsertest --code="canvas.js::205"
+    $ npx gulp makeref --code="canvas.js::205"
+
+Pass `--no-download` to reuse the locally cached index without contacting the
+network. The index can also be built and queried locally (the CI job that
+publishes it builds it the same way):
+
+    $ npx gulp botbrowsertest --coverage-per-test
+    $ npx gulp coverage_search --code="canvas.js::205" \
+        --index=build/coverage/per-test-index.json --no-download
+
 ### Continuous integration
 
 On every push and pull request three GitHub Actions workflows collect coverage

external/ccov/coverage_search.mjs

@@ -20,26 +20,46 @@ import path from "path";
 const __dirname = import.meta.dirname;
 const PROJECT_ROOT = path.join(__dirname, "../..");
 
-const { values } = parseArgs({
+// The per-test coverage index (which ref test exercises which source
-  args: process.argv.slice(2),
+// line/function) is rebuilt on every push to master and published to the
-  options: {
+// gh-pages branch of the pdf.js.refs repository.
-    code: { type: "string" },
+const PER_TEST_INDEX_URL =
-    "coverage-dir": { type: "string", default: "build/coverage" },
+  "https://raw.githubusercontent.com/mozilla/pdf.js.refs/gh-pages/per-test-index.json";
-    help: { type: "boolean", short: "h", default: false },
+
-  },
+let values;
-});
+try {
+  ({ values } = parseArgs({
+    args: process.argv.slice(2),
+    options: {
+      code: { type: "string" },
+      index: { type: "string", default: "build/per-test-index.json" },
+      "no-download": { type: "boolean", default: false },
+      help: { type: "boolean", short: "h", default: false },
+    },
+  }));
+} catch (error) {
+  // parseArgs is strict, so an unknown/renamed option (e.g. the removed
+  // --coverage-dir) would otherwise abort with an uncaught stack trace.
+  console.error(`Error: ${error.message}`);
+  console.error("Run with --help to see the available options.");
+  process.exit(1);
+}
 
 if (values.help || !values.code) {
   console.log(
-    "Usage: coverage_search.mjs --code=<file>::<line|function> [--coverage-dir=<path>]\n\n" +
+    "Usage: coverage_search.mjs --code=<file>::<line|function> [--index=<path>] [--no-download]\n\n" +
       "  --code          Source file and line number or function name to search for.\n" +
       "                  Examples:\n" +
       "                    --code=canvas.js::205\n" +
       "                    --code=canvas.js::drawImageAtIntegerCoords\n" +
-      "  --coverage-dir  Coverage directory containing per-test-index.json [build/coverage]\n\n" +
+      "  --index         Where to cache or read the per-test index.\n" +
+      "                  [build/per-test-index.json]\n" +
+      "  --no-download   Don't contact the network; use the cached index as-is.\n\n" +
       "Prints to stdout the IDs of tests whose coverage includes the given line or\n" +
-      "function (one ID per line).\n" +
+      "function (one ID per line).\n\n" +
-      "Run browsertest with --coverage-per-test first to generate the index."
+      "The index is downloaded from the pdf.js.refs repository and cached locally;\n" +
+      "it is only re-downloaded when the published file has changed.\n" +
+      `Source: ${PER_TEST_INDEX_URL}`
   );
   process.exit(values.help ? 0 : 1);
 }
@@ -58,18 +78,129 @@ const isLine = /^\d+$/.test(location);
 const lineNum = isLine ? parseInt(location, 10) : null;
 const funcName = isLine ? null : location;
 
-const coverageDir = path.isAbsolute(values["coverage-dir"])
+const indexPath = path.isAbsolute(values.index)
-  ? values["coverage-dir"]
+  ? values.index
-  : path.join(PROJECT_ROOT, values["coverage-dir"]);
+  : path.join(PROJECT_ROOT, values.index);
+// The ETag of the cached copy is stored alongside it, so the next run can ask
+// the server (via If-None-Match) to only re-send the file when it has changed.
+const etagPath = `${indexPath}.etag`;
+
+// Refreshes the locally cached index from the published copy, downloading it
+// only when it has changed since the last run. When the network is unavailable
+// a previously cached copy is reused if present.
+async function refreshIndex() {
+  if (values["no-download"]) {
+    return; // Freshness check disabled; the read below validates existence.
+  }
+
+  const hasCached = fs.existsSync(indexPath);
+
+  // On any download failure, fall back to a previously cached copy when one
+  // exists; otherwise there's nothing to search, so fail.
+  const fallbackOrFail = reason => {
+    if (hasCached) {
+      console.error(
+        `Warning: couldn't refresh per-test index (${reason}); using the cached copy.`
+      );
+      return;
+    }
+    console.error(`Error: couldn't download per-test index (${reason}).`);
+    process.exit(1);
+  };
+
+  const headers = {};
+  if (hasCached && fs.existsSync(etagPath)) {
+    const etag = fs.readFileSync(etagPath, "utf8").trim();
+    // Only forward a syntactically valid HTTP ETag (RFC 7232), so the cached
+    // file's contents can't be used to inject arbitrary data into the request.
+    if (/^(?:W\/)?"[\x21\x23-\x7e]*"$/.test(etag)) {
+      headers["If-None-Match"] = etag;
+    }
+  }
+
+  let response;
+  try {
+    console.error(`Fetching per-test index from ${PER_TEST_INDEX_URL} ...`);
+    response = await fetch(PER_TEST_INDEX_URL, { headers });
+  } catch (error) {
+    fallbackOrFail(error.message);
+    return;
+  }
+
+  if (response.status === 304) {
+    console.error("Per-test index is up to date.");
+    return;
+  }
+  if (!response.ok) {
+    fallbackOrFail(`HTTP ${response.status}`);
+    return;
+  }
+
+  let text;
+  try {
+    text = await response.text();
+  } catch (error) {
+    fallbackOrFail(error.message);
+    return;
+  }
+
+  // Parse the payload before caching it, and cache the re-serialized result
+  // rather than the raw response body: only well-formed JSON produced by our
+  // own JSON.stringify is ever written to disk.
+  let serialized;
+  try {
+    serialized = JSON.stringify(JSON.parse(text));
+  } catch {
+    fallbackOrFail("the downloaded index is not valid JSON");
+    return;
+  }
+
+  // Write to a temporary file and rename it into place.
+  try {
+    fs.mkdirSync(path.dirname(indexPath), { recursive: true });
+    const tmpPath = `${indexPath}.${process.pid}.tmp`;
+    fs.writeFileSync(tmpPath, serialized);
+    fs.renameSync(tmpPath, indexPath);
+
+    const etag = response.headers.get("etag");
+    if (etag) {
+      fs.writeFileSync(etagPath, etag);
+    } else {
+      fs.rmSync(etagPath, { force: true });
+    }
+  } catch (error) {
+    // A write failure (disk full, read-only dir, ...) shouldn't be fatal when
+    // a usable cached copy already exists.
+    fallbackOrFail(error.message);
+    return;
+  }
+  console.error(`Per-test index updated (${serialized.length} bytes).`);
+}
+
+await refreshIndex();
 
-const indexPath = path.join(coverageDir, "per-test-index.json");
 if (!fs.existsSync(indexPath)) {
-  console.error(`Error: index file not found: ${indexPath}`);
+  console.error(`Error: per-test index not found: ${indexPath}`);
-  console.error("Run browsertest with --coverage-per-test first.");
+  console.error(
+    "Build it locally (gulp botbrowsertest --coverage-per-test) or omit " +
+      "--no-download to fetch it from the pdf.js.refs repository."
+  );
   process.exit(1);
 }
 
-const { ids, files } = JSON.parse(fs.readFileSync(indexPath, "utf8"));
+let ids, files;
+try {
+  ({ ids, files } = JSON.parse(fs.readFileSync(indexPath, "utf8")));
+} catch (error) {
+  console.error(
+    `Error: couldn't read per-test index at ${indexPath}: ${error.message}`
+  );
+  console.error(
+    "The cached index may be corrupt; delete it and re-run without " +
+      "--no-download to refetch it."
+  );
+  process.exit(1);
+}
 
 // Find the file entry whose path matches fileName.
 let fileEntry = null;

gulpfile.mjs

@@ -793,36 +793,18 @@ function runTests(testsName, { bot = false } = {}) {
       args.push("--coveragePerTest");
     }
 
-    const codeArg = testsName === "browser" ? getArgValue("--code") : null;
+    if (testsName === "browser") {
-    if (codeArg) {
+      let shouldRun;
-      const coverageDir =
+      try {
-        getArgValue("--coverage-output") || BUILD_DIR + "coverage";
+        shouldRun = applyCodeTestFilter(args);
-      const result = spawnSync(
+      } catch (error) {
-        "node",
+        reject(error);
-        [
-          path.join(__dirname, "external/ccov/coverage_search.mjs"),
-          `--code=${codeArg}`,
-          `--coverage-dir=${coverageDir}`,
-        ],
-        { encoding: "utf8" }
-      );
-      if (result.status !== 0) {
-        reject(new Error(result.stderr?.trim() || "coverage_search failed"));
         return;
       }
-      const testIds = result.stdout.trim().split("\n").filter(Boolean);
+      if (!shouldRun) {
-      if (testIds.length === 0) {
-        console.log(`\n### No tests found covering "${codeArg}"`);
         resolve();
         return;
       }
-      console.log(
-        `\n### Found ${testIds.length} test(s) covering "${codeArg}":\n` +
-          testIds.map(id => `  ${id}`).join("\n")
-      );
-      for (const id of testIds) {
-        args.push(`-t=${id}`);
-      }
     }
 
     const testProcess = startNode(args, { cwd: TEST_DIR, stdio: "inherit" });
@@ -888,6 +870,136 @@ function collectArgs(options, args) {
   }
 }
 
+// Builds the coverage_search command line. By default the published index is
+// downloaded; an explicit --index=<path> selects a local index instead, used
+// read-only so it's never overwritten by the published copy.
+function getCoverageSearchArgs(codeArg) {
+  const searchArgs = [
+    path.join(__dirname, "external/ccov/coverage_search.mjs"),
+    `--code=${codeArg}`,
+  ];
+  const indexArg = getArgValue("--index");
+  if (indexArg === "") {
+    // An explicit but empty value (e.g. an unset shell variable expanding to
+    // `--index=`) almost certainly isn't intended; fail loudly rather than
+    // silently falling back to the downloaded index.
+    throw new Error("--index was given without a value");
+  }
+  if (indexArg) {
+    searchArgs.push(`--index=${indexArg}`, "--no-download");
+  } else if (process.argv.includes("--no-download")) {
+    searchArgs.push("--no-download");
+  }
+  return searchArgs;
+}
+
+// Returns the set of test IDs defined in the local ref-test manifest, or null
+// when it can't be read. Used to drop coverage-derived IDs that don't exist on
+// this branch (e.g. a test renamed since the published index was built), which
+// would otherwise make test.mjs reject the entire run.
+function readManifestTestIds() {
+  try {
+    const manifestFile = process.env.PDF_TEST || "test_manifest.json";
+    const manifest = JSON.parse(
+      fs.readFileSync(path.join(__dirname, TEST_DIR, manifestFile), "utf8")
+    );
+    return new Set(manifest.map(entry => entry.id));
+  } catch {
+    return null;
+  }
+}
+
+// For a --code=<file>::<line|function> argument, runs coverage_search to find
+// the ref tests that exercise that location and returns their IDs (an empty
+// array when none match locally). Returns null when --code wasn't given; throws
+// when the search itself fails.
+function resolveCodeTestIds() {
+  const codeArg = getArgValue("--code");
+  if (!codeArg) {
+    return null;
+  }
+  // Inherit stderr so the index download progress is visible; stdout is
+  // captured because it carries the matching test IDs.
+  const result = spawnSync("node", getCoverageSearchArgs(codeArg), {
+    encoding: "utf8",
+    stdio: ["ignore", "pipe", "inherit"],
+  });
+  if (result.status !== 0) {
+    // status is null when the child couldn't be spawned or was killed by a
+    // signal; surface the real cause instead of a generic message.
+    throw new Error(
+      result.error
+        ? `coverage_search failed: ${result.error.message}`
+        : `coverage_search failed (exit code ${result.status})`
+    );
+  }
+  let testIds = result.stdout.trim().split("\n").filter(Boolean);
+
+  // The published index is built from master, so some covering tests may not
+  // exist on this branch. Drop them (with a note) rather than letting test.mjs
+  // reject the whole run — and silently exit 0 — on the first unknown ID.
+  const knownIds = readManifestTestIds();
+  if (knownIds) {
+    const missing = testIds.filter(id => !knownIds.has(id));
+    if (missing.length) {
+      console.log(
+        `\n### Ignoring ${missing.length} covered test(s) not in the manifest:\n` +
+          missing.map(id => `  ${id}`).join("\n")
+      );
+      testIds = testIds.filter(id => knownIds.has(id));
+    }
+  }
+
+  if (testIds.length === 0) {
+    console.log(`\n### No tests found covering "${codeArg}"`);
+  } else {
+    console.log(
+      `\n### Found ${testIds.length} test(s) covering "${codeArg}":\n` +
+        testIds.map(id => `  ${id}`).join("\n")
+    );
+  }
+  return testIds;
+}
+
+function appendTestFilters(args, testIds) {
+  const existingIds = new Set();
+
+  // collectArgs always normalizes filters to the space-separated
+  // `-t <id>` / `--testfilter <id>` form, so that's the only shape to read.
+  for (let i = 0; i < args.length; i++) {
+    if (args[i] === "-t" || args[i] === "--testfilter") {
+      if (i + 1 < args.length) {
+        existingIds.add(args[++i]);
+      }
+    }
+  }
+  for (const id of testIds) {
+    if (!existingIds.has(id)) {
+      // Pass the id as a separate argument: node's parseArgs parses the short
+      // form `-t=<id>` as the literal value "=<id>", which matches no test and
+      // aborts the run with "Unrecognized test IDs".
+      args.push("-t", id);
+      existingIds.add(id);
+    }
+  }
+}
+
+// Applies the --code coverage filter to `args`. Returns false when the run
+// should be skipped (--code was given but resolved to no runnable tests), and
+// true otherwise (no --code, or matching tests were appended). Throws when the
+// coverage search itself fails.
+function applyCodeTestFilter(args) {
+  const codeTestIds = resolveCodeTestIds();
+  if (codeTestIds === null) {
+    return true; // No --code argument; run normally.
+  }
+  if (codeTestIds.length === 0) {
+    return false; // Nothing (runnable) covers the requested location.
+  }
+  appendTestFilters(args, codeTestIds);
+  return true;
+}
+
 function makeRef(done, bot) {
   console.log("\n### Creating reference images");
 
@@ -936,6 +1048,18 @@ function makeRef(done, bot) {
     args
   );
 
+  let shouldRun;
+  try {
+    shouldRun = applyCodeTestFilter(args);
+  } catch (error) {
+    done(error);
+    return;
+  }
+  if (!shouldRun) {
+    done();
+    return;
+  }
+
   const testProcess = startNode(args, { cwd: TEST_DIR, stdio: "inherit" });
   testProcess.on("close", function (code) {
     if (code !== 0) {
@@ -946,34 +1070,33 @@ function makeRef(done, bot) {
   });
 }
 
-// Queries the per-test coverage index built by --coverage-per-test and prints
+// Prints the IDs of tests that exercised a given source file location, using
-// the IDs of tests that exercised a given source file location. Run with
+// the per-test coverage index published to the pdf.js.refs repository. The
-// --code=<file>::<line|function>, e.g. --code=canvas.js::205
+// index is downloaded on demand, cached locally, and only re-downloaded when
+// it has changed. Run with --code=<file>::<line|function>, e.g.
+// --code=canvas.js::205 (add --no-download to reuse the cached index offline).
 gulp.task("coverage_search", function (done) {
   const codeArg = getArgValue("--code");
   if (!codeArg) {
     done(new Error('Missing --code argument, e.g. --code="canvas.js::205"'));
     return;
   }
-  const coverageDir =
+  let searchArgs;
-    getArgValue("--coverage-output") || BUILD_DIR + "coverage";
+  try {
-  const result = spawnSync(
+    searchArgs = getCoverageSearchArgs(codeArg);
-    "node",
+  } catch (error) {
-    [
+    done(error);
-      path.join(__dirname, "external/ccov/coverage_search.mjs"),
+    return;
-      `--code=${codeArg}`,
-      `--coverage-dir=${coverageDir}`,
-    ],
-    { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }
-  );
-  if (result.stderr) {
-    process.stderr.write(result.stderr);
-  }
-  if (result.stdout) {
-    process.stdout.write(result.stdout);
   }
+  const result = spawnSync("node", searchArgs, { stdio: "inherit" });
   if (result.status !== 0) {
-    done(new Error("coverage_search failed"));
+    done(
+      new Error(
+        result.error
+          ? `coverage_search failed: ${result.error.message}`
+          : `coverage_search failed (exit code ${result.status})`
+      )
+    );
     return;
   }
   done();