Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a GitHub Actions workflow for running the font tests #17263

Merged
merged 4 commits into from
Nov 13, 2023

Conversation

timvandermeij
Copy link
Contributor

@timvandermeij timvandermeij commented Nov 11, 2023

The commit messages contain more details about the individual changes.

Note that this depends on mozilla/botio-files-pdfjs#42 being merged and deployed to the bots.

Fixes #11802.
Fixes a part of #11851.

test/test.mjs Outdated Show resolved Hide resolved
timvandermeij added a commit to timvandermeij/botio-files-pdfjs that referenced this pull request Nov 11, 2023
If mozilla/pdf.js#17263 is in place, we will no
longer run the font tests on the bots, which means that the font tests
command should be removed. Moreover, we can get rid of all submodule
installations because the project won't have any submodules anymore
after that patch, which simplifies the code and reduces runtime of all
tests a bit (note that the submodules were also installed for tests
that didn't need them).
The current logic is more complicated than it needs to be because it's
passing a callback function to `startBrowsers` instead of a string.
This commit simplifies the logic by passing the base URL as a string to
`startBrowsers` and having it do further augmentation internally,
thereby removing all indirection of the function calls to `makeTestUrl`
and the inner function it returned.
…st.mjs`

This commit prepares for the introduction of extra options in later
commits by changing the function signatures of the `startBrowser(s)`
functions to take parameter objects instead of plain parameters. This
makes the call sites explicitly state which parameters they pass,
improving overall readability as well.
@timvandermeij
Copy link
Contributor Author

timvandermeij commented Nov 12, 2023

I have rebased and updated the patch. It now includes a README file about the purpose of the font tests and how to run them locally, as well as a generic --headless mode this is applicable to all tests (while keeping headful mode the default) and an improved file paths list in the workflow.

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've successfully tested this patch locally, on Windows 11.

r=me, this is awesome; thank you!

gulpfile.mjs Show resolved Hide resolved
test/font/README.md Outdated Show resolved Hide resolved
This commit prepares for running the font tests on GitHub Actions where
we can't spin up headful browsers because there are no display
capabilities on the workers. This will also be useful for porting other
test targets to GitHub Actions at a later time, as well as running the
tests locally in headless mode.
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.
@Snuffleupagus Snuffleupagus merged commit 44cde3c into mozilla:master Nov 13, 2023
9 checks passed
@timvandermeij timvandermeij deleted the font-tests branch November 18, 2023 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade ttx
2 participants