-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add support for addFiles config parameter. #8
Conversation
This seems like the sort of thing that mocha itself should support directly? |
Mocha supports |
I totally agree and understand; airbnb does this by defining a I'm more saying that rather than patching it into this one runner, maybe it'd be better added to mocha core :-) |
Aha, now I understand. Thank you for the clarification. (We do the same thing at Mongo, with a mocha script). I agree, would definitely be better in core. I'll look into filing an issue in the mocha repo for this. |
Hi! Thanks for reporting and sorry for the late response, I've been sick Thanks for working on this! It looks good to me but I would love @ljharb to approve it before merging |
I'm not going to block it or anything; but I think that this sort of thing should wait on being supported in actual mocha and not just in the runner. |
So I'm writing up the issue on the Mocha repo however I just realized that this runner is not using mocha via the CLI, we're using it via the node API, which does support the ability to I'm a little lost how adding CLI support to mocha would benefit the runner? It would allow us to keep all arguments via the |
Opened mocha issue for discussion: mochajs/mocha#3181 |
I wasn’t aware mocha had this facility; that it does, and we’d match it’s name, sounds good to me. The reason to wait would be so that this runner had no command line/config options that mocha lacks, besides those that are unique to this runner. |
Mocha 5 was just released with the addition of the I'm planning on updating this PR to make use of that new flag however that will require using at least Mocha 5. I know @ljharb has an interest in keeping support back to node 0.10 (I'm curious as to the why behind supporting an unsupported version of node) so I'm looking for some guidance on how to move forward. One thought would be to move mocha to a peerDependency of mocha >3 and then have mocha 5.x installed as a dev dependency to test out the functionality. Any other suggestions? |
"supported version of node" is something that node core deals with; that a node version is unsupported in no way means individual tools must, or even should, drop support for that version. Since this runner uses the API, and not the cli, there's no reason it should use the I think that indeed the proper way to test this out is to run travis builds in all of mocha 3, 4, and 5, but only run the 4 and 5 versions in node 4+ (and to set the peer dep not to |
Updated to follow the mocha CLI option of Let me know if you want me to include the travis changes in this ticket as well. Seems unnecessary as we are just using the node API. |
You da real MVP. |
Hello!
First of all, thank you for this project! It's truly saving me from laborious long test runs when using mocha directly.
For the tests that I run I have the need to run additional files before each test file to set up some additional Mocha hooks.
i.e. in one file I have a
I was able to add this functionality through this change.
Sorry about not creating an issue first. I've been hacking on this all this day to get this working with our test suite and this is part 1 of 2 of changes that I'd love to upstream.