Conversation
theengineear
force-pushed
the
fix-test-action
branch
from
f17761d to
7e1c182
Compare
|
Note that this is now failing — which is what we want! Previously, we were missing some test coverage which should have tanked our tests. Before 
After 
|
| timeout: 10000, | ||
| // opt-in to the new Chrome headless implementation | ||
| // ref: https://developer.chrome.com/articles/new-headless/ | ||
| headless: 'new', |
There was a problem hiding this comment.
I think this setting is outdated now?
| args: [ | ||
| // Disables interactive prompt: Do you want to the application Chromium.app to accept incoming network connections? | ||
| // ref: https://github.com/puppeteer/puppeteer/issues/4752#issuecomment-586599843 | ||
| '--disable-features=DialMediaRouteProvider', |
There was a problem hiding this comment.
I checked the linked issue and it seemed like it was closed.
| ], | ||
| // These args fix “Error: Failed to launch the browser process!” when | ||
| // run in GitHub. We are only running our own code here, so this is ok. | ||
| args: ['--no-sandbox', '--disable-setuid-sandbox'], |
There was a problem hiding this comment.
Would love a cross-check on this one. I think this is what we need to do now in GitHub based on our settings?
|
@klebba — Would you mind taking a look at this? |
See #272 for a discussion on our tests in GH.
See #272 for a discussion on our tests in GH.
theengineear
force-pushed
the
fix-test-action
branch
from
7e1c182 to
7b08ace
Compare
|
Back to passing now that underlying tests are actually fixed (versus just not running at all). |
Previously, the tests weren’t actually being run by GitHub actions for some time now. They would run locally, but the browser would silently fail to launch in GitHub when the action was run. Additionally, we now “Bail out!” within “test.js” if any exceptions are caught — these are magic words that our TAP parser will recognize and they instruct the parser to immediately consider the test a failure.
theengineear
force-pushed
the
fix-test-action
branch
from
7b08ace to
e35fec0
Compare
| } catch (err) { | ||
| // Ensure that we “Bail out!” (see TAP specification) if script fails. Note | ||
| // that the tap stream is being read on stdout. | ||
| console.log('Bail out!'); // eslint-disable-line no-console |
There was a problem hiding this comment.
FYI @klebba — we should bake this sort of thing into all our test.js files across projects.
|
Note that in addition to fixing this action, we also ensure that it will break in the right way in the future if the browser fails to launch. 
|
2 participants
Previously, the tests weren’t actually being run by GitHub actions for some time now. They would run locally, but the browser would silently fail to launch in GitHub when the action was run.
Additionally, we now “Bail out!” within “test.js” if any exceptions are caught — these are magic words that our TAP parser will recognize and they instruct the parser to immediately consider the test a failure.