From 8157f39c62fa0d73c734f5de187c835cdd29da9f Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 29 Oct 2023 19:43:32 +0100 Subject: [PATCH] Introduce a GitHub Actions workflow for running the font tests This commit migrates the font tests away from the bots. Not only are the font tests broken on the Windows bot since some time, they also run on Python 2 (end of life since January 2020) and `ttx` 3.19.0 (released in November 2017). The latter is installed via a submodule, which requires more complicated logic for finding and running `ttx`. We solve the issues by implementing a modern workflow that installs the most recent stable Python and `ttx` (`fonttools` package) versions. This simplifies the `ttx` driver code as well because it can now assume `ttx` is available on the path (just like we do for e.g. `node` invocations). GitHub Actions takes care of creating a virtual environment with `fonttools` in it so that the `ttx` entrypoint is available. Locally the font tests can be run in a similar way by creating and sourcing a virtual environment with `fonttools` in it before running the font tests, and a README file is included with instructions for doing so. --- .github/workflows/font_tests.yml | 64 ++++++++++++++++++++++++++++ .gitmodules | 3 -- gulpfile.mjs | 2 - test/font/README.md | 36 ++++++++++++++++ test/font/ttxdriver.mjs | 72 +++++++++++--------------------- test/ttx/README.md | 1 - test/ttx/fonttools-code | 1 - 7 files changed, 125 insertions(+), 54 deletions(-) create mode 100644 .github/workflows/font_tests.yml delete mode 100644 .gitmodules create mode 100644 test/font/README.md delete mode 100644 test/ttx/README.md delete mode 160000 test/ttx/fonttools-code diff --git a/.github/workflows/font_tests.yml b/.github/workflows/font_tests.yml new file mode 100644 index 0000000000000..34c06cbc6840c --- /dev/null +++ b/.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 diff --git a/.gitmodules b/.gitmodules deleted file mode 100644 index 2de02d7d1f3a9..0000000000000 --- a/.gitmodules +++ /dev/null @@ -1,3 +0,0 @@ -[submodule "test/ttx/fonttools-code"] - path = test/ttx/fonttools-code - url = https://github.com/behdad/fonttools.git diff --git a/gulpfile.mjs b/gulpfile.mjs index 43c66755a7190..7f17b707278bf 100644 --- a/gulpfile.mjs +++ b/gulpfile.mjs @@ -1749,7 +1749,6 @@ gulp.task( return streamqueue( { objectMode: true }, createTestSource("unit", { bot: true }), - createTestSource("font", { bot: true }), createTestSource("browser", { bot: true }), createTestSource("integration") ); @@ -1774,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") ); diff --git a/test/font/README.md b/test/font/README.md new file mode 100644 index 0000000000000..c3f31cc9e52f2 --- /dev/null +++ b/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 +``` diff --git a/test/font/ttxdriver.mjs b/test/font/ttxdriver.mjs index cdfbf7d72e5dc..c820d8ccf6ac0 100644 --- a/test/font/ttxdriver.mjs +++ b/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); diff --git a/test/ttx/README.md b/test/ttx/README.md deleted file mode 100644 index 98bfd86921a14..0000000000000 --- a/test/ttx/README.md +++ /dev/null @@ -1 +0,0 @@ -If `git clone --recursive` was not used, please run `git submodule init; git submodule update` to pull fonttools code. diff --git a/test/ttx/fonttools-code b/test/ttx/fonttools-code deleted file mode 160000 index d8170131a3458..0000000000000 --- a/test/ttx/fonttools-code +++ /dev/null @@ -1 +0,0 @@ -Subproject commit d8170131a3458ffbc19089cf33249777bde390e7