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

Mocha.ui() now tries to import ui module if name is not found in mocha.interfaces #1022

Merged
merged 1 commit into from
Nov 3, 2013

Conversation

andreypopp
Copy link
Contributor

Hi, --require <ui-module> --ui <name of the ui exported by ui-module> doesn't work cause requires processed after settings an UI. I also think that having a --ui option to require a module is also consistent with how --reporter works and more straightforward than using --require mod --ui name even if it would work.

I believe this is better a version of #851.

travisjeffery pushed a commit that referenced this pull request Nov 3, 2013
mocha --ui will require the ui module if it doesn't exist, to be consistent with --reporter
@travisjeffery travisjeffery merged commit 0a9a681 into mochajs:master Nov 3, 2013
@giggio
Copy link
Contributor

giggio commented Jun 17, 2014

This seems to force me to have a ui interface registered globally.
I am trying to write another ui interface and unless I use an absolute path or install the module globally I can't load the ui interface. The ideal scenario is to do this per application, so when someone clones the repo the tests just work. The current way will force the developer to globally install mocha and also the module I am writing with the custom ui interface.
This happens because when you require a name, the lookup starts from the executable file location, mocha (global node_modules dir). It doesn't do the lookup from the node_modules folder where the mocha command is ran (pwd).

https://github.com/visionmedia/mocha/blob/755f05410d8bdb1218073b74755089998b98a0ca/lib/mocha.js#L153

Any idea on how to work around this problem?

@giggio
Copy link
Contributor

giggio commented Jun 17, 2014

Ok, I went on investigating a little bit more. It seems that when you add the cwd to modules.path on _mocha like so:

https://github.com/visionmedia/mocha/blob/755f05410d8bdb1218073b74755089998b98a0ca/bin/_mocha#L152

module.paths.push(cwd, join(cwd, 'node_modules'));

It will only work for immediate requires, only when they are required directly from _mocha. When mocha.js tries to load the new ui interface later it can't find it, because its module.paths is different from _mocha's module.paths.
To be able to load from the apps node_modules folder it would be needed to set module.paths from where the require for the ui is set, at mocha.js. Is this an acceptable solution?
Another option would be to somehow load the other ui from _mocha. But it would be more work to do so.
I can try it out when you have an idea of what the best path is, and then submit a PR. Just let me know if any of the options make sense so I can go in the right direction.

BTW: the problem probably also affects reporters, as they are also loaded from mocha.js, which will prevent locally installed reporters to work.

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.

3 participants