pdf.js: Failing GitHub Actions font-test was marked as successful

In a recent PR the font-tests failed completely on Windows, however the GitHub Actions workflow didn’t actually catch the failure which means that the test was marked as successful: https://github.com/mozilla/pdf.js/actions/runs/7150415038/job/19473662790?pr=17395#step:8:20

That seems quite bad, since it means that we could very easily overlook a failing test. (Running gulp fonttest locally on Windows works fine though, so I’m not sure how to debug this.)

/cc @timvandermeij Any ideas here?

About this issue

  • Original URL
  • State: closed
  • Created 7 months ago
  • Comments: 16 (13 by maintainers)

Most upvoted comments

@whimboo I have created #17457 and can confirm that setting media.sanity-test.disabled to true fixes the issue on Windows. You can find the logs here: https://github.com/mozilla/pdf.js/actions/runs/7309787007/job/19917756293?pr=17457. Thank you for making the patch upstream; we’ll await a new release then and update the test code on our side as well to respect non-zero exit codes.

As a temporary solution, depending on how difficult/time-consuming a proper fix is, could we do something similar to the old botio font-test and check that the success message is being logged and manually throw an exception? https://github.com/mozilla/botio-files-pdfjs/pull/42/files#diff-aa6bfe19954403f5a8eac1c2259476609786b1c9b05bfaa81a360a87430a7710L26-L33

I have managed to reproduce the issue by putting back the code from https://github.com/mozilla/pdf.js/pull/17172#discussion_r1421038879 (it talks about an invalid browsing context in BiDi which is what we also see here) and a hothacked node_modules/puppeteer-core/lib/esm/puppeteer/bidi/Connection.js file where the exception on line 123 is raised unconditionally.

Putting back the page closing lines is required to trigger the #onContextDestroyed function and that in turn raises the exception. However, the Puppeteer exception is not bubbling up to our test runner process, which causes it to exit with status 0 and indicates success. Somewhere in Puppeteer it seems that the exception is logged and then swallowed, or it’s occurring in the browser context and not bubbling up further. In any case, this looks like a regression from enabling BiDi and sadly this doesn’t seem possible to fix on our end given that the exception in Puppeteer doesn’t even reach our code.

@whimboo @OrKoN Does this perhaps ring a bell for you, also given the comment at https://github.com/mozilla/pdf.js/pull/17172#discussion_r1421038879 which was needed for BiDi? The exception in Puppeteer is triggered as soon as await page.close(); is called, but it’s only logged and doesn’t cause this promise to reject (putting a try/catch around this does nothing). This feels like a bug in Puppeteer to me, also given the apparent behavior change that required the removal of this loop, but you probably know much more about how the Puppeteer internals handle this than I do.