Merge pull request #17263 from timvandermeij/font-tests

Introduce a GitHub Actions workflow for running the font tests

.github/workflows/font_tests.yml

@@ -0,0 +1,64 @@
+name: Font tests
+on:
+  push:
+    paths:
+      - 'gulpfile.mjs'
+      - 'src/**'
+      - 'test/test.mjs'
+      - 'test/font/**'
+      - '.github/workflows/font_tests.yml'
+    branches:
+      - master
+  pull_request:
+    paths:
+      - 'gulpfile.mjs'
+      - 'src/**'
+      - 'test/test.mjs'
+      - 'test/font/**'
+      - '.github/workflows/font_tests.yml'
+    branches:
+      - master
+  workflow_dispatch:
+permissions:
+  contents: read
+
+jobs:
+  test:
+    name: Test
+
+    strategy:
+      fail-fast: false
+      matrix:
+        node-version: [lts/*]
+        os: [windows-latest, ubuntu-latest]
+
+    runs-on: ${{ matrix.os }}
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
+
+      - name: Install Gulp
+        run: npm install -g gulp-cli
+
+      - name: Install other dependencies
+        run: npm install
+
+      - name: Use Python 3.12
+        uses: actions/setup-python@v4
+        with:
+          python-version: '3.12'
+          cache: 'pip'
+
+      - name: Install Fonttools
+        run: pip install fonttools
+
+      - name: Run font tests
+        run: gulp fonttest --headless

.gitmodules

@@ -1,3 +0,0 @@
-[submodule "test/ttx/fonttools-code"]
-	path = test/ttx/fonttools-code
-	url = https://github.com/behdad/fonttools.git

gulpfile.mjs

@@ -684,6 +684,9 @@ function createTestSource(testsName, { bot = false, xfaOnly = false } = {}) {
     if (process.argv.includes("--noChrome") || forceNoChrome) {
       args.push("--noChrome");
     }
+    if (process.argv.includes("--headless")) {
+      args.push("--headless");
+    }
 
     const testProcess = startNode(args, { cwd: TEST_DIR, stdio: "inherit" });
     testProcess.on("close", function (code) {
@@ -712,6 +715,9 @@ function makeRef(done, bot) {
   if (process.argv.includes("--noChrome") || forceNoChrome) {
     args.push("--noChrome");
   }
+  if (process.argv.includes("--headless")) {
+    args.push("--headless");
+  }
 
   const testProcess = startNode(args, { cwd: TEST_DIR, stdio: "inherit" });
   testProcess.on("close", function (code) {
@@ -1743,7 +1749,6 @@ gulp.task(
     return streamqueue(
       { objectMode: true },
       createTestSource("unit", { bot: true }),
-      createTestSource("font", { bot: true }),
       createTestSource("browser", { bot: true }),
       createTestSource("integration")
     );
@@ -1768,7 +1773,6 @@ gulp.task(
     return streamqueue(
       { objectMode: true },
       createTestSource("unit", { bot: true }),
-      createTestSource("font", { bot: true }),
       createTestSource("browser", { bot: true, xfaOnly: true }),
       createTestSource("integration")
     );

test/font/README.md

@@ -0,0 +1,36 @@
+# Font tests
+
+The font tests check if PDF.js can read font data correctly. For validation
+the `ttx` tool (from the Python `fonttools` library) is used that can convert
+font data to an XML format that we can easily use for assertions in the tests.
+In the font tests we let PDF.js read font data and pass the PDF.js-interpreted
+font data through `ttx` to check its correctness. The font tests are successful
+if PDF.js can successfully read the font data and `ttx` can successfully read
+the PDF.js-interpreted font data back, proving that PDF.js does not apply any
+transformations that break the font data.
+
+## Running the font tests
+
+The font tests are run on GitHub Actions using the workflow defined in
+`.github/workflows/font_tests.yml`, but it is also possible to run the font
+tests locally. The current stable versions of the following dependencies are
+required to be installed on the system:
+
+- Python 3
+- `fonttools` (see https://pypi.org/project/fonttools and https://github.com/fonttools/fonttools)
+
+The recommended way of installing `fonttools` is using `pip` in a virtual
+environment because it avoids having to do a system-wide installation and
+therefore improves isolation, but any other way of installing `fonttools`
+that makes `ttx` available in the `PATH` environment variable also works.
+
+Using the virtual environment approach the font tests can be run locally by
+creating and sourcing a virtual environment with `fonttools` installed in
+it before running the font tests:
+    
+```
+python3 -m venv venv
+source venv/bin/activate
+pip install fonttools
+gulp fonttest
+```

test/font/ttxdriver.mjs

@@ -14,65 +14,43 @@
  * limitations under the License.
  */
 
-import { fileURLToPath } from "url";
 import fs from "fs";
+import os from "os";
 import path from "path";
 import { spawn } from "child_process";
 
-const __dirname = path.dirname(fileURLToPath(import.meta.url));
+let ttxTaskId = Date.now();
 
-const ttxResourcesHome = path.join(__dirname, "..", "ttx");
-
-let nextTTXTaskId = Date.now();
-
-function runTtx(ttxResourcesHomePath, fontPath, registerOnCancel, callback) {
-  fs.realpath(ttxResourcesHomePath, function (error, realTtxResourcesHomePath) {
-    const fontToolsHome = path.join(realTtxResourcesHomePath, "fonttools-code");
-    fs.realpath(fontPath, function (errorFontPath, realFontPath) {
-      const ttxPath = path.join("Lib", "fontTools", "ttx.py");
-      if (!fs.existsSync(path.join(fontToolsHome, ttxPath))) {
-        callback("TTX was not found, please checkout PDF.js submodules");
-        return;
-      }
-      const ttxEnv = {
-        PYTHONPATH: path.join(fontToolsHome, "Lib"),
-        PYTHONDONTWRITEBYTECODE: true,
-      };
-      const ttxStdioMode = "ignore";
-      const python = process.platform !== "win32" ? "python2" : "python";
-      const ttx = spawn(python, [ttxPath, realFontPath], {
-        cwd: fontToolsHome,
-        stdio: ttxStdioMode,
-        env: ttxEnv,
-      });
-      let ttxRunError;
-      registerOnCancel(function (reason) {
-        ttxRunError = reason;
-        callback(reason);
-        ttx.kill();
-      });
-      ttx.on("error", function (errorTtx) {
-        ttxRunError = errorTtx;
-        callback("Unable to execute ttx");
-      });
-      ttx.on("close", function (code) {
-        if (ttxRunError) {
-          return;
-        }
-        callback();
-      });
-    });
+function runTtx(fontPath, registerOnCancel, callback) {
+  const ttx = spawn("ttx", [fontPath], { stdio: "ignore" });
+  let ttxRunError;
+  registerOnCancel(function (reason) {
+    ttxRunError = reason;
+    callback(reason);
+    ttx.kill();
+  });
+  ttx.on("error", function (errorTtx) {
+    ttxRunError = errorTtx;
+    callback(
+      "Unable to execute `ttx`; make sure the `fonttools` dependency is installed"
+    );
+  });
+  ttx.on("close", function (code) {
+    if (ttxRunError) {
+      return;
+    }
+    callback();
   });
 }
 
 function translateFont(content, registerOnCancel, callback) {
   const buffer = Buffer.from(content, "base64");
-  const taskId = (nextTTXTaskId++).toString();
-  const fontPath = path.join(ttxResourcesHome, taskId + ".otf");
-  const resultPath = path.join(ttxResourcesHome, taskId + ".ttx");
+  const taskId = (ttxTaskId++).toString();
+  const fontPath = path.join(os.tmpdir(), `pdfjs-font-test-${taskId}.otf`);
+  const resultPath = path.join(os.tmpdir(), `pdfjs-font-test-${taskId}.ttx`);
 
   fs.writeFileSync(fontPath, buffer);
-  runTtx(ttxResourcesHome, fontPath, registerOnCancel, function (err) {
+  runTtx(fontPath, registerOnCancel, function (err) {
     fs.unlinkSync(fontPath);
     if (err) {
       console.error(err);

test/test.mjs

@@ -99,6 +99,12 @@ function parseOptions() {
       describe: "Uses default answers (intended for CLOUD TESTS only!).",
       type: "boolean",
     })
+    .option("headless", {
+      default: false,
+      describe:
+        "Run the tests in headless mode, i.e. without visible browser windows.",
+      type: "boolean",
+    })
     .option("port", {
       default: 0,
       describe: "The port the HTTP server should listen on.",
@@ -250,8 +256,11 @@ function updateRefImages() {
 function examineRefImages() {
   startServer();
 
-  const startUrl = `http://${host}:${server.port}/test/resources/reftest-analyzer.html#web=/test/eq.log`;
-  startBrowser("firefox", startUrl).then(function (browser) {
+  startBrowser({
+    browserName: "firefox",
+    headless: false,
+    startUrl: `http://${host}:${server.port}/test/resources/reftest-analyzer.html#web=/test/eq.log`,
+  }).then(function (browser) {
     browser.on("disconnected", function () {
       stopServer();
       process.exit(0);
@@ -339,26 +348,28 @@ function startRefTest(masterMode, showRefImages) {
     server.hooks.POST.push(refTestPostHandler);
     onAllSessionsClosed = finalize;
 
-    const startUrl = `http://${host}:${server.port}/test/test_slave.html`;
-    await startBrowsers(function (session) {
-      session.masterMode = masterMode;
-      session.taskResults = {};
-      session.tasks = {};
-      session.remaining = manifest.length;
-      manifest.forEach(function (item) {
-        var rounds = item.rounds || 1;
-        var roundsResults = [];
-        roundsResults.length = rounds;
-        session.taskResults[item.id] = roundsResults;
-        session.tasks[item.id] = item;
-      });
-      session.numRuns = 0;
-      session.numErrors = 0;
-      session.numFBFFailures = 0;
-      session.numEqNoSnapshot = 0;
-      session.numEqFailures = 0;
-      monitorBrowserTimeout(session, handleSessionTimeout);
-    }, makeTestUrl(startUrl));
+    await startBrowsers({
+      baseUrl: `http://${host}:${server.port}/test/test_slave.html`,
+      initializeSession: session => {
+        session.masterMode = masterMode;
+        session.taskResults = {};
+        session.tasks = {};
+        session.remaining = manifest.length;
+        manifest.forEach(function (item) {
+          var rounds = item.rounds || 1;
+          var roundsResults = [];
+          roundsResults.length = rounds;
+          session.taskResults[item.id] = roundsResults;
+          session.tasks[item.id] = item;
+        });
+        session.numRuns = 0;
+        session.numErrors = 0;
+        session.numFBFFailures = 0;
+        session.numEqNoSnapshot = 0;
+        session.numEqFailures = 0;
+        monitorBrowserTimeout(session, handleSessionTimeout);
+      },
+    });
   }
   function checkRefsTmp() {
     if (masterMode && fs.existsSync(refsTmpDir)) {
@@ -793,29 +804,18 @@ function onAllSessionsClosedAfterTests(name) {
   };
 }
 
-function makeTestUrl(startUrl) {
-  return function (browserName) {
-    const queryParameters =
-      `?browser=${encodeURIComponent(browserName)}` +
-      `&manifestFile=${encodeURIComponent("/test/" + options.manifestFile)}` +
-      `&testFilter=${JSON.stringify(options.testfilter)}` +
-      `&xfaOnly=${options.xfaOnly}` +
-      `&delay=${options.statsDelay}` +
-      `&masterMode=${options.masterMode}`;
-    return startUrl + queryParameters;
-  };
-}
-
 async function startUnitTest(testUrl, name) {
   onAllSessionsClosed = onAllSessionsClosedAfterTests(name);
   startServer();
   server.hooks.POST.push(unitTestPostHandler);
 
-  const startUrl = `http://${host}:${server.port}${testUrl}`;
-  await startBrowsers(function (session) {
-    session.numRuns = 0;
-    session.numErrors = 0;
-  }, makeTestUrl(startUrl));
+  await startBrowsers({
+    baseUrl: `http://${host}:${server.port}${testUrl}`,
+    initializeSession: session => {
+      session.numRuns = 0;
+      session.numErrors = 0;
+    },
+  });
 }
 
 async function startIntegrationTest() {
@@ -823,9 +823,12 @@ async function startIntegrationTest() {
   startServer();
 
   const { runTests } = await import("./integration-boot.mjs");
-  await startBrowsers(function (session) {
-    session.numRuns = 0;
-    session.numErrors = 0;
+  await startBrowsers({
+    baseUrl: null,
+    initializeSession: session => {
+      session.numRuns = 0;
+      session.numErrors = 0;
+    },
   });
   global.integrationBaseUrl = `http://${host}:${server.port}/build/generic/web/viewer.html`;
   global.integrationSessions = sessions;
@@ -901,10 +904,12 @@ function unitTestPostHandler(req, res) {
   return true;
 }
 
-async function startBrowser(browserName, startUrl = "") {
+async function startBrowser({ browserName, headless, startUrl }) {
   const options = {
     product: browserName,
-    headless: false,
+    // Note that using `headless: true` gives a deprecation warning; see
+    // https://github.com/puppeteer/puppeteer#default-runtime-settings.
+    headless: headless === true ? "new" : false,
     defaultViewport: null,
     ignoreDefaultArgs: ["--disable-extensions"],
     // The timeout for individual protocol (CDP) calls should always be lower
@@ -971,7 +976,7 @@ async function startBrowser(browserName, startUrl = "") {
   return browser;
 }
 
-async function startBrowsers(initSessionCallback, makeStartUrl = null) {
+async function startBrowsers({ baseUrl, initializeSession }) {
   // Remove old browser revisions from Puppeteer's cache. Updating Puppeteer can
   // cause new browser revisions to be downloaded, so trimming the cache will
   // prevent the disk from filling up over time.
@@ -995,12 +1000,25 @@ async function startBrowsers(initSessionCallback, makeStartUrl = null) {
       closed: false,
     };
     sessions.push(session);
-    const startUrl = makeStartUrl ? makeStartUrl(browserName) : "";
 
-    await startBrowser(browserName, startUrl)
+    // Construct the start URL from the base URL by appending query parameters
+    // for the runner if necessary.
+    let startUrl = "";
+    if (baseUrl) {
+      const queryParameters =
+        `?browser=${encodeURIComponent(browserName)}` +
+        `&manifestFile=${encodeURIComponent("/test/" + options.manifestFile)}` +
+        `&testFilter=${JSON.stringify(options.testfilter)}` +
+        `&xfaOnly=${options.xfaOnly}` +
+        `&delay=${options.statsDelay}` +
+        `&masterMode=${options.masterMode}`;
+      startUrl = baseUrl + queryParameters;
+    }
+
+    await startBrowser({ browserName, headless: options.headless, startUrl })
       .then(function (browser) {
         session.browser = browser;
-        initSessionCallback?.(session);
+        initializeSession(session);
       })
       .catch(function (ex) {
         console.log(`Error while starting ${browserName}: ${ex.message}`);

test/ttx/README.md

@@ -1 +0,0 @@
-If `git clone --recursive` was not used, please run `git submodule init; git submodule update` to pull fonttools code.

test/ttx/fonttools-code

@@ -1 +0,0 @@
-Subproject commit d8170131a3458ffbc19089cf33249777bde390e7