Bugzilla

In Puppeteer upstream repository, two helpers are used in the test suite to skip tests in Firefox:
- describeFailsFirefox
- itFailsFirefox

Those helpers will completely skip the corresponding describe/it when running in Firefox.
In mozilla central, we decided to take a different approach and to run the tests, but we expect a failure, which is described in a manifest puppeteer-expected.json: https://searchfox.org/mozilla-central/source/remote/test/puppeteer-expected.json

It makes the vendoring process complex because after updating puppeteer, you need to manually replace all the describeFailsFirefox/itFailsFirefox by describe/it. The issue is that we still have some describeFailsFirefox/itFailsFirefox in tree: https://searchfox.org/mozilla-central/search?q=%28it%7Cdescribe%29FailsFirefox%5C%28&path=&case=false&regexp=true

This means you must be careful to only update the describeFailsFirefox/itFailsFirefox which were not already in tree. 

The reason why we have different approaches is that in mozilla-central we want to keep running the test and be notified in case we happen to fix something rather than leaving tests skipped forever. But upstream puppeteer prefers to completely skip the test (might also be due to limitation of the test harness used there).

It would be great if we code reduce the code changes between our puppeteer version and the upstream version. One suggestion could be to keep the test code identical, and instead only apply a patch to the helpers describeFailsFirefox/itFailsFirefox to map them to describe/it. This would be much easier to maintain. 

To do that however we'll need to check the reason why we have kept some describeFailsFirefox/itFailsFirefox in tree. If it's intended and we really want to skip those tests in mozilla-central then we'll need a slight API update to express the difference between failing tests and skipped tests in code. For instance we could propose to upstream puppeteer to have 2 sets of helpers:
- describeFailsFirefox/itFailsFirefox
- describeSkipsFirefox/itSkipsFirefox

The implementation of the 2 helpers would be the same upstream, they would skip the test completely, but in mozilla-central we could modify the "fail" version to still run the test.

In Puppeteer upstream repository, two helpers are used in the test suite to skip tests in Firefox:
- describeFailsFirefox
- itFailsFirefox

Those helpers will completely skip the corresponding describe/it when running in Firefox.
In mozilla central, we decided to take a different approach and to run the tests, but we expect a failure, which is described in a manifest puppeteer-expected.json: https://searchfox.org/mozilla-central/source/remote/test/puppeteer-expected.json

It makes the vendoring process complex because after updating puppeteer, you need to manually replace all the describeFailsFirefox/itFailsFirefox by describe/it. The issue is that we still have some describeFailsFirefox/itFailsFirefox in tree: https://searchfox.org/mozilla-central/search?q=%28it%7Cdescribe%29FailsFirefox%5C%28&path=&case=false&regexp=true

This means you must be careful to only update the describeFailsFirefox/itFailsFirefox which were not already in tree. 

The reason why we have different approaches is that in mozilla-central we want to keep running the test and be notified in case we happen to fix something rather than leaving tests skipped forever. But upstream puppeteer prefers to completely skip the test (might also be due to limitation of the test harness used there).

It would be great if we code reduce the code changes between our puppeteer version and the upstream version. One suggestion could be to keep the test code identical, and instead only apply a patch to the helpers describeFailsFirefox/itFailsFirefox to map them to describe/it. This would be much easier to maintain. 

To do that however we'll need to check the reason why we have kept some describeFailsFirefox/itFailsFirefox in tree. If it's intended and we really want to skip those tests in mozilla-central then we'll need a slight API update to express the difference between failing tests and skipped tests in code. For instance we could propose to upstream puppeteer to have 2 sets of helpers:
- describeFailsFirefox/itFailsFirefox
- describeSkipsFirefox/itSkipsFirefox

The implementation of the 2 helpers would be the same upstream, they would skip the test completely, but in mozilla-central we could modify the "fail" version to still run the test. We would still mark the test as expected to FAIL via the manifest.

In Puppeteer upstream repository, two helpers are used in the test suite to skip tests in Firefox:
- describeFailsFirefox
- itFailsFirefox

Those helpers will completely skip the corresponding describe/it when running in Firefox.
In mozilla central, we decided to take a different approach and to run the tests, but we expect a failure, which is described in a manifest puppeteer-expected.json: https://searchfox.org/mozilla-central/source/remote/test/puppeteer-expected.json

It makes the vendoring process complex because after updating puppeteer, you need to manually replace all the describeFailsFirefox/itFailsFirefox by describe/it. The issue is that we still have some describeFailsFirefox/itFailsFirefox in tree: https://searchfox.org/mozilla-central/search?q=%28it%7Cdescribe%29FailsFirefox%5C%28&path=&case=false&regexp=true

This means you must be careful to only update the describeFailsFirefox/itFailsFirefox which were not already in tree. 

The reason why we have different approaches is that in mozilla-central we want to keep running the test and be notified in case we happen to fix something rather than leaving tests skipped forever. But upstream puppeteer prefers to completely skip the test (might also be due to limitation of the test harness used there).

It would be great if we could reduce the code changes between our puppeteer version and the upstream version. One suggestion could be to keep the test code identical, and instead only apply a patch to the helpers describeFailsFirefox/itFailsFirefox to map them to describe/it. This would be much easier to maintain. 

To do that however we'll need to check the reason why we have kept some describeFailsFirefox/itFailsFirefox in tree. If it's intended and we really want to skip those tests in mozilla-central then we'll need a slight API update to express the difference between failing tests and skipped tests in code. For instance we could propose to upstream puppeteer to have 2 sets of helpers:
- describeFailsFirefox/itFailsFirefox
- describeSkipsFirefox/itSkipsFirefox

The implementation of the 2 helpers would be the same upstream, they would skip the test completely, but in mozilla-central we could modify the "fail" version to still run the test. We would still mark the test as expected to FAIL via the manifest.