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

🐛 Delayed force-exit gulp test after Karma reports test completion #14814

Merged
merged 2 commits into from
Apr 23, 2018
Merged

🐛 Delayed force-exit gulp test after Karma reports test completion #14814

merged 2 commits into from
Apr 23, 2018

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented Apr 23, 2018

There are several potential causes for the disconnect / timeout issues we're seeing on Travis, where Karma reports that all tests are complete, but gulp test doesn't terminate because something is holding it up from doing so.

For example, see https://travis-ci.org/ampproject/amphtml/jobs/370168309

Related issues:

We have a ticket open with Sauce labs to investigate this.

Meanwhile, this PR temporarily mitigates the problem by force-exiting the gulp test process after a gap of 5 seconds, so that Karma reporters can print all their output to the console.

Partial fix for #14800

@rsimha
Copy link
Contributor Author

rsimha commented Apr 23, 2018

/to @cvializ @jridgewell

@rsimha rsimha changed the title 🐛 Delayed force-exit the gulp test process after Karma reports test com… 🐛 Delayed force-exit gulp test after Karma reports test completion Apr 23, 2018
@rsimha
Copy link
Contributor Author

rsimha commented Apr 23, 2018

Tested this locally, and I can verify that gulp test now exits after all tests are complete.

Copy link
Contributor

@nainar nainar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rs lgtm

@@ -371,7 +371,10 @@ function runTests() {
log(
red('ERROR:'),
yellow('Karma test failed with exit code ' + exitCode));
process.exitCode = exitCode;
// TODO(rsimha): Remove this after Karma / Sauce ticket is resolved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably link to issues here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@rsimha rsimha merged commit f0f2b22 into ampproject:master Apr 23, 2018
@rsimha rsimha deleted the 2018-04-23-CloseBrowser branch April 23, 2018 23:46
@rsimha rsimha restored the 2018-04-23-CloseBrowser branch April 23, 2018 23:47
@rsimha rsimha deleted the 2018-04-23-CloseBrowser branch April 24, 2018 00:12
victorb pushed a commit to ipfs/aegir that referenced this pull request May 4, 2018
Karma has some issues letting go after the test run. Ref karma-runner/karma#1788 and ampproject/amphtml#14814

This fix basically forces aegir to close after the karma tests have been successfully run, so instead of a test-run taking 13 seconds for the tests to run then 30 seconds for karma to force-close, it finishes in 14 seconds. 

Solves #212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants