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

Feature Request: ES2015 Generator Support #2024

Closed
airportyh opened this issue Dec 26, 2015 · 8 comments
Closed

Feature Request: ES2015 Generator Support #2024

airportyh opened this issue Dec 26, 2015 · 8 comments

Comments

@airportyh
Copy link
Contributor

There are a couple of modules (mocha-co, mocha-generators) out there that patch Mocha to allow writing tests as generator functions.
This is a highly desirable feature for me, and I think potentially for many other folks as well. The mocha-as-promised precedent makes me think that this feature request aligns with the philosophy of Mocha and worth it to pursue.

@danielstjules
Copy link
Contributor

Related: #1575 #1891 #1748 #1032

@airportyh
Copy link
Contributor Author

@danielstjules thanks for the info!

Looks like the main challenge was getting co to work in the browser back in March. From what I've learned digging around in the source, Mocha has switched to using browserify to build its browser version since that time, which means we are closer to getting this to work. I did a quick check to see that co actually will work with browserify as a required module and it did.

In the process of trying to start working on this, I am getting some lint errors and integration test errors from make test and make test-all. Anyone else aware of these or is it just me? I would have thought they'd be caught by CI.

@danielstjules
Copy link
Contributor

Did you just checkout that branch and rebase master? The PR was opened before either linting or integration tests were added to mocha, so if you're running into issues - that's why. It could def use a refactor too. :)

In any case, I think the bigger issue is that it wasn't felt it belonged in core mocha. https://github.com/blakeembrey/co-mocha is a great alternative.

@airportyh
Copy link
Contributor Author

@danielstjules thanks, I figured out the issue, and have put out this PR for review. This incorporates the code in your #1575 and @blakeembrey's https://github.com/blakeembrey/co-mocha. The tests are passing, and I have tested it in Chrome with its native generator support as well as with the Babel transpiler plus the regenerator runtime - it works in both cases. I haven't updated the browser version mocha.js because it seems out of date wrt to the source in the rest of the repo. When I did make clean; make build mocha.js, I noticed a bug that causes assertion errors to be rendered twice on the test page at test/browser/index.html. I would be happy to investigate further, but I believe this is separate from the coroutines discussion because I saw this same bug when I went back to the master branch.

To address whether this features belongs in mocha, I feel hopeful that given the precedent of promises support - it can. I have been using coroutines and generators with promises for various projects and it's been very helpful to the clarity of my code, productivity, and it's just been a joy to use. I want to share this joy and excitement with others. Back in 2014, @tj implied that generators is a more appealing feature than promises. Now that even promises has made the cut, it should be easy to make the case for generators. Having this feature be built-in to Mocha - the most popular testing framework in the ecosystem - will not only help folks get started easier, but it will also give coroutines/generators legitimacy.

To weigh the upside against the downside: similar to the addition of promises support, this change is 100% backwards compatible. It is also doesn't require any architecture change to Mocha - the change is incremental.

@mekdev
Copy link

mekdev commented Mar 15, 2016

+1 would love to get this in, we are also using co-mocha to help with clarifying the code.

@blakeembrey
Copy link

I just wanted to leave a comment about having this inside of Mocha core, instead of leaving it in user-land. I thought I'd left a more detailed comment in the past, but can't find it now. At least, my last comment looks like it was #1748 (comment).

I see promises support as aligning with the future. It was, and still is, on an upward usage trajectory. Generators, however, are not. The current use is literally a hack to be make it do what async/await does out of the box. Plus, async/await already works with Mocha since they're just promises. Generators, however, should be promoted in more creative ways than just serial async code.

That said, I don't have an issue either way, but the proper implementation should go a bit further than co-mocha does and should not use the same function toString tricks on Mocha.

@wmertens
Copy link

Indeed, I was using co-mocha and now I'm just using async/await which works fine in Mocha out of the box. No need for the generator hack…

@danielstjules
Copy link
Contributor

Both TJ and Travis previously mentioned wanting to wait for async/await, and I think a compelling argument has been made in support of this. Closing for now. Thanks though!

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

No branches or pull requests

5 participants