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

Modernize the TTX driver code #17890

Merged
merged 2 commits into from
Apr 8, 2024

Conversation

timvandermeij
Copy link
Contributor

The commit messages contain more details about the individual changes.

The original `test.py` code, see
https://github.com/mozilla/pdf.js/blob/c2376e5cea1b3cee89d77e6a4e8b0d9b5b8591be/test/test.py,
did not have any timeout logic for TTX, but it got introduced when
`test.py` was ported from Python to JavaScript as `test.js` in
mozilla@c2376e5#diff-a561630bb56b82342bc66697aee2ad96efddcbc9d150665abd6fb7ecb7c0ab2f.

However, I don't think we've ever actually seen TTX timing out.
Moreover, back then we used a very old version of TTX and ran the font
tests on the bots (where a hanging process would block other jobs and
would require a manual action to fix), so this code was most likely
only included defensively.

Fortunately, nowadays it should not be necessary anymore because we use
the most recent version of TTX (which either returns the result or
errors out, but isn't known to hang on inputs) and we run the font tests
on GitHub Actions which doesn't block other jobs anymore and also
automatically times the job out for us in the unlikely event that a hang
would ever occur.

In short, we can safely remove this logic to simplify the code and to get
rid of a callback.
This commit removes the final callbacks in this code by switching to a
promises-based interface, overall simplifying the code. Moreover, we
document why we write to files on disk and modernize the code using e.g.
template strings.
@timvandermeij timvandermeij changed the title Improve the TTX driver code Modernize the TTX driver code Apr 5, 2024
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.

r=me, thanks.

@timvandermeij timvandermeij merged commit fb21c42 into mozilla:master Apr 8, 2024
9 checks passed
@timvandermeij timvandermeij deleted the font-tests-callbacks branch April 8, 2024 18:12
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.

2 participants