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

Updates mocha.js to address #1700 #1701

Closed
wants to merge 1 commit into from
Closed

Updates mocha.js to address #1700 #1701

wants to merge 1 commit into from

Conversation

eddies
Copy link

@eddies eddies commented May 15, 2015

Fixes #1700

@dasilvacontin
Copy link
Contributor

Hi @eddies! Could you provide tests that pass thanks to this change? Cheers!

@eddies
Copy link
Author

eddies commented May 24, 2015

@dasilvacontin Honestly, I'm not sure how to provide a unit-like test for this. I was only triggering this issue as part of running a test w/ PhantomJS and Grunt and and various other dependencies. I did create https://github.com/eddies/mocha-test-case as a minimal example, which I believe definitively isolates the error to the specific one-line commit to mocha I highlighted.

If someone could take a look at the mocha-test-case repo and provide some suggestions as to how that might be translated into a test for the mochajs repo, I could try running with it from there.

@danielstjules
Copy link
Contributor

@eddies Please make the change against runnable.js, rather than the compiled/disted mocha.js.

// Discard the resolution if this test has already failed asynchronously

Beyond that, how does this impact #1578 and #1540 ? After updating runnable.js, I wonder if the build fails.

Edit: It looks like the tests continue to pass

jbnicolai pushed a commit that referenced this pull request Jun 5, 2015
This fixes #1700 and closes #1701.
Thanks to: Edwin Shin [eddies].
@jbnicolai
Copy link

Picked up @danielstjules's comment and merged in a separate branch.

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.

"PhantomJS timed out, possibly due to a missing Mocha run() call" regression
4 participants