Disable the browser-tests in Google Chrome on the bots

[Snuffleupagus]

Given that browsertest repeatedly timeout in Google Chrome, and considering that Firefox is the primary development target, we stop running them on the bots to avoid having to repeatedly deal with this.

Note that we already disabled these tests on Windows almost three years ago, because of stability issues; see PR #14392.

[Rob--W]

Given that browsertest repeatedly timeout in Google Chrome, and considering that Firefox is the primary development target,

While Firefox is the primary development target, PDF.js is a cross-browser library so it would be nice to have broader test coverage.

we stop running them on the bots to avoid having to repeatedly deal with this.

I'd like to understand why the tests time out and see if we/I can fix that instead of disabling test coverage altogether. Do you have a list of recent failures that I can investigate? Is there a way to reproduce the issue locally?

[Snuffleupagus]

While Firefox is the primary development target, PDF.js is a cross-browser library so it would be nice to have broader test coverage.

Sure, however the current situation is impacting general development negatively and @calixteman and myself agree that running browsertest in Chrome isn't that important.

I'd like to understand why the tests time out and see if we/I can fix that instead of disabling test coverage altogether.

I think that should be done separately after we land this, since for one thing this appears to be blocking updating of Puppeteer which is necessary for the addition of new integration-tests (there's work happening on e.g. the Ink-editor that needs that).

Do you have a list of recent failures that I can investigate?

Not really a list, but please see one recent example in #19115 (comment).

Is there a way to reproduce the issue locally?

Sorry, don't know since I'm not testing in Chrome locally.

[Rob--W]

Is the Puppeteer PR at #19115 the primary motivation for this change, or are there more instances?

The failures in the Puppeteer PR look like reftest failures, not very surprising when browser versions are bumped. I don't see test timeouts in the reftests.
I see a timeout in an integration test in Chrome. I haven't looked at the test itself, but wonder whether this is an issue with the test case, the test driver, or the browser.

If it is just that single test failing in a specific browser with no time to investigate/fix, then I'd prefer skipping that single test instead of disabling everything altogether.

I don't spend as much time on PDF.js development as you all, so I won't push back too hard if the tests are more harmful than helpful for your experience. However I do want to make sure that this is indeed a fundamental issue that needs addressing, instead of an incidental case.

In the event that you want to disable it, I'd like to not disable the ability to run browser tests altogether, but to disable it by default with a way to opt in. So that "botio test" defaults to what you'd like and that "botio-chrome" or something can be used to debug the issue, with the goal of getting the tests back to a reliable state.

Diversity in test environments slows down development when there are inexplicable intermittent test failures, but can also be valuable in serving as an independent verification that the logic is as desired. Disabling all tests in Chrome increases the risk of ending up with a library that is unreliable in all but one browser.

[Snuffleupagus]

Is the Puppeteer PR at #19115 the primary motivation for this change, or are there more instances?

browsertest has been failing intermittently in Chrome for a while now, and it's really quite annoying.

The failures in the Puppeteer PR look like reftest failures, not very surprising when browser versions are bumped. I don't see test timeouts in the reftests.

The browsertest-portion of the logs contain the following line, which show that Chrome does timeout:

TEST-UNEXPECTED-FAIL | test failed chrome has not responded in 120s

Also, the errors: 111 line in #19115 (comment) indicates the number of tests that failed to run in that case.

If it is just that single test failing in a specific browser with no time to investigate/fix, then I'd prefer skipping that single test instead of disabling everything altogether.

We've already tried doing that once, and having to keep disabling tests "randomly" in Chrome really isn't a feasible solution.

In the event that you want to disable it, I'd like to not disable the ability to run browser tests altogether, but to disable it by default with a way to opt in. So that "botio test" defaults to what you'd like and that "botio-chrome" or something can be used to debug the issue, with the goal of getting the tests back to a reliable state.

They can still be run just fine locally, since these changes only affect the bots.
Besides, it's trivial to comment out the forceNoChrome = true; lines in a future PR that tries to debug/fix the tests in Chrome; hence I don't see the point in adding a new test command that (most likely) won't even be used.

[Rob--W]

Is the Puppeteer PR at #19115 the primary motivation for this change, or are there more instances?

browsertest has been failing intermittently in Chrome for a while now, and it's really quite annoying.

Can you describe in more detail the kind of failures? Reftests, integration tests, or both? I'd like to investigate and improve the situation, but haven't followed the bot tests test closely to tell what common pain points are.

The failures in the Puppeteer PR look like reftest failures, not very surprising when browser versions are bumped. I don't see test timeouts in the reftests.

The browsertest-portion of the logs contain the following line, which show that Chrome does timeout:

TEST-UNEXPECTED-FAIL | test failed chrome has not responded in 120s

@calixteman ran the tests again and the test times out again. Looks pretty consistent. Was this 100% failure rate (n=2) already that consistent before?

The failures being consistent would make it easier to debug & fix.

In the event that you want to disable it, I'd like to not disable the ability to run browser tests altogether, but to disable it by default with a way to opt in. So that "botio test" defaults to what you'd like and that "botio-chrome" or something can be used to debug the issue, with the goal of getting the tests back to a reliable state.

They can still be run just fine locally, since these changes only affect the bots.

Besides, it's trivial to comment out the forceNoChrome = true; lines in a future PR that tries to debug/fix the tests in Chrome; hence I don't see the point in adding a new test command that (most likely) won't even be used.

My local machine differs from the bot environment. Tests passing on my local machine doesn't mean as much as tests passing on the bots. With my intent to improve the reliability of tests on the bots, I need a way to run tests on exactly the same environment.

[Snuffleupagus]

Can you describe in more detail the kind of failures? Reftests, integration tests, or both?

gulp browsertest === reftests, which as already mentioned has been having timeout issues on the Linux bot for awhile.

We recently had to disable one test in PR #19024, however having to do that repeatably is an untenable situation that waste both human and machine time.

[calixteman]

The last ran test with Chrome is multiline_compress which means that issue16300 is the one which timed out.
I really don't think that the issue is directly related to this specific test (i.e. issue16300), so likely Chrome (or the OS) is in a bad state.
I really see some value to have the integration tests with Firefox and Chrome: sometimes we've some Chrome specific bugs because the scheduling is slightly different.
But for reference tests, it's different. Usually when a patch induces a drawing regression, we see a test regression with both Chrome and Firefox. As far as I can tell, I never saw a reftest regression in Chrome but not in Firefox.

[timvandermeij]

I think I agree with Calixte here. It would certainly be better to keep running Chrome too, but on the other hand the number of issues did increase lately (another example is #19032) and I also don't really recall having seen reference test failures in Chrome that weren't also visible in Firefox, so in that sense I doubt we'll lose much if any test coverage if we disable Chrome for the reference tests only. I do indeed think that we should keep running Chrome for the integration tests because that has already been of value multiple times.

How about we do the following?

1. Merge this patch first to stabilize the bots (this could help avoid missing actual issues because we have seen before that if there is a lot of noise in the test results that it's easy to miss new failures).
2. Create an issue tracking re-enabling Chrome, and do that in a separate scope. What we can then do is for example create a separate PR that enables Chrome and run it a couple of times to get a better feeling for what fails, and perhaps that can give clues as to what's going on.

The advantage of this approach is that we can both investigate the situation with Chrome while in the meantime making sure that the failures are not in the way for new PRs. Would that be feasible?

[Rob--W]

I am approving this PR because @Snuffleupagus , @calixteman, @timvandermeij and I have consensus on the proposed direction. To summarize:

Browser tests (essentially reference tests, reftests) are currently run in Firefox and Chrome. Their purpose is to detect regressions in PDF.js logic. When a regression occurs, relevant tests usually fail in both browsers.

The maintenance cost of these tests is non-trivial: these tests occasionally fail or time out for reasons unrelated to PDF.js itself (e.g. browser updates, e.g. through puppeteer updates as seen in #19115), and require attention from PDF.js developers to investigate and resolve/ignore. Since Firefox is the primary development target, and tests are currently failing frequently in Chrome, this patch stops running them in CI. This only affects reference tests, as we continue to run some other unit tests and integration tests in both browsers.

Once this PR is merged, the following behaviors change in CI:

• "botio browsertest" (gulp botbrowsertest) no longer runs reference tests in Chrome (only Firefox).
• "botio test" (gulp bottest) no longer runs reference tests in Chrome (only Firefox). It continues to run unit tests and integration tests in Chrome.
• "botio makeref" (gulp botmakeref) stops creating new reference images in Chrome. Locally, gulp makeref can still be used to create reference images as before (including Chrome).

Next steps after merging: file issue to track the following work:

• reintroduce an independent way to run the reference tests in Chrome too (or only).
• investigate the cause of the failures and improve their stability
• monitor test stability
• re-enable ref tests in Chrome by default.