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

Allow deferred initialization of mocha test runner via a flag #75

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alex-seville
Copy link

This addresses the following issues:

Because mocha needs to be initialized (mocha.run()) in order to return a reference to the runner (need by Yeti to bind events), any tests that use a deferred initialization of mocha (via requirejs.require, YUI.use, or other means) will not be run correctly.

This PR addresses the issue by allowing the user to include a asyncYeti flag in their tests (ignored if not running Yeti). This flag will alert Yeti to wait until the test runner defer-redly calls mocha.run() before initializing mocha.

It's less than ideal because it requires the creation of the flag variables, and because it overrides mocha.run temporarily. However, I think these tradeoff are fair given that it works and unblocks users who are unable to use Yeti because they have deferred mocha initializations.

Mocha may or may not be updated in the future (mochajs/mocha#719), so I think leaving the Yeti binding patterns in place is better than trying to over-engineer a solution to this problem.

Let me know what you think!

@reid
Copy link
Contributor

reid commented Feb 3, 2014

The CLI functional tests appear to hang (never complete) on Node v0.10.15 on my local OS X and v0.10.24 on Travis. Can you verify by running npm test from your v0.10 install? You can also run just the functional tests with ./jake test-functional. Most likely, this is a problem with inject.js.

Thanks for submitting this patch! I'd love for this to ship this week.

@alex-seville
Copy link
Author

On my machine at home I'm running < 0.10 Node and it passed but took 300 seconds (as opposed to ~147s before).

I'll try running it on v0.10 and see what happens. Not sure why the extra tests adds so much extra time.

@alex-seville
Copy link
Author

Good news, bad news.

The tests succeed now, but they're 6 times slower than before (from what I can see in the travis build history). I'm not really familiar enough with the test code to understand why there would be such a delay. Maybe there's some code that does polling which is running infrequently?

@reid
Copy link
Contributor

reid commented Feb 3, 2014

Weird. I don't think the right approach would be to switch from setTimeout in the test: it looks like there's a problem in inject.js that causes a short async wait to fail. I'm taking a look, thanks!

@reid
Copy link
Contributor

reid commented Feb 3, 2014

@alex-seville The tests take longer to run because mocha-async is timing out. Its tests never run, which you can verify with yeti test/functional/fixture/mocha-async.html.

It looks like there isn't any way to tell Mocha to defer run() — it happens automatically and test definition must happen synchronously until the resolution of mochajs/mocha#719 lands. I'm not sure if fixing this here is even possible. If your tests are generated async, it looks like we're out of luck.

I understand Joe is a library similar to Mocha that aims to fix this very problem. Maybe it would be worthwhile to integrate Joe into Yeti, which may provide a similar API to the Mocha tests you're already using? I'm not sure and will take a look at this again later this week.

@alex-seville
Copy link
Author

I could have sworn that when I run yeti against the test case that I was seeing one test complete.

Hmm...I'll review this. I had another thought about how this could be approached.

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