-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
test(base.spec.js): Fix Base reporter test failure due to timeout #3527
Conversation
… to timeout The `Base` reporter's specification verifies reporter can interpret Chai custom error messages. This test takes ~480ms (lightly loaded CPU), which is _way_ too close to the 500ms timeout. Change doubles this timeout. mochajs#3524
@@ -312,6 +312,7 @@ describe('Base reporter', function() { | |||
}); | |||
|
|||
it('should interpret Chai custom error messages', function() { | |||
this.timeout(1000); | |||
var chaiExpect = require('chai').expect; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead increasing timeout, how about move require('chai')
to outside it
? Requiring chai takes most of time.
var chaiExpect = require('chai').expect;
it('should interpret Chai custom error messages', function() {
try {
chaiExpect(43, 'custom error message').to.equal(42);
} catch (err) {
...
In my local, original test time took 46ms
but moving require('chai')
took only 8ms
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@boneskull went to the effort to minimize the scope to just this test.
This was considered, but didn't wish to increase its scope.
I have no idea about possible interactions between Chai
and Unexpected
, and have no desire to go down that rabbit hole. But you can borrow my flashlight...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What interactions between chai
and unexpected
?
@boneskull, please weigh in. |
lgtm, thanks |
Description of the Change
The
Base
reporter's specification verifies reporter can interpret Chai custom error messages.This test takes ~480ms on my somewhat memory-starved computer, which is way too close to the 500ms timeout.
Change doubles this timeout.
Alternate Designs
Benefits
Prevention of random test failure due to timing issue.
Failure strikes often on Appveyor (for me anyway).
Possible Drawbacks
None to knowledge.
Applicable issues
Fixes #3524
semver-patch