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

Adding a default timeout for QUnit tests. #58

Merged
merged 2 commits into from
Apr 14, 2015
Merged

Adding a default timeout for QUnit tests. #58

merged 2 commits into from
Apr 14, 2015

Conversation

MichielDean
Copy link
Contributor

So, we had a developer check in a test that hung our build all weekend.
I would like to propose putting in a default test timeout into this package.

I think that 5 minutes is a really long time for qunit tests, but I'm not sure what kind of long running tests people might have.

I'm open to suggestions regarding the duration of the default timeout.

@@ -3,6 +3,7 @@
QUnit.config.urlConfig.push({ id: 'nocontainer', label: 'Hide container'});
QUnit.config.urlConfig.push({ id: 'nojshint', label: 'Disable JSHint'});
QUnit.config.urlConfig.push({ id: 'doccontainer', label: 'Doc test pane'});
QUnit.config.testTimeout = 300000 //Default Test Timeout 5 Minutes
Copy link
Member

Choose a reason for hiding this comment

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

Lets swap to 60000ms (60 seconds) as a default, folks can increase it from there if they need to.

@rwjblue
Copy link
Member

rwjblue commented Apr 14, 2015

👍 for this as a default, it is pretty easy for folks to change the number via:

// tests/test-helper.js

QUnit.config.testTimeout = undefined; // or whatever

rwjblue added a commit that referenced this pull request Apr 14, 2015
Adding a default timeout for QUnit tests.
@rwjblue rwjblue merged commit 20595e9 into ember-cli:master Apr 14, 2015
@MichielDean
Copy link
Contributor Author

I like the 60 second timeout a lot more. I just didn't want to step on toes =p

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

Successfully merging this pull request may close these issues.

2 participants