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 local modules to work as UIs and Reporters #1267

Merged
merged 1 commit into from
Jul 13, 2014
Merged

Allow local modules to work as UIs and Reporters #1267

merged 1 commit into from
Jul 13, 2014

Conversation

giggio
Copy link
Contributor

@giggio giggio commented Jul 12, 2014

(This was PR #1240, we are starting over)
The current way dependencies are handled forces us to have a ui interface (or reporter) installed 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 globally install 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

Currently mocha adds 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'));

But not on mocha.js.

This 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 a local node_modules folder it would be needed to set module.paths from where the require for the ui is set, at mocha.js.
This PR does just that.

@jbnicolai
Copy link

The current way will force the developer to globally install mocha and also globally install the module I am writing with the custom ui interface.

So this is what confused me originally. There is no need to install mocha globally, but if running mocha globally then it will only find globally installed UIs and reporters as well. However, when running locally, locally installed reporters and UIs work just find.

What this PR adds is that a globally running mocha will source locally installed or linked reporters and UIs.

@giggio does that about sum it up? ;-)

@giggio
Copy link
Contributor Author

giggio commented Jul 12, 2014

@jbnicolai Yes!

@jbnicolai
Copy link

Am I correct in understanding that this fixes UIs, but not reporters? I'm just checking against the earlier installed mocha-teamcity-reporter, and still seeing the same error.

@jbnicolai
Copy link

screen shot 2014-07-13 at 01 53 50

@giggio
Copy link
Contributor Author

giggio commented Jul 12, 2014

It fixes both. I just tested:

~/proj/rabiscos/mochatestreporter$ mocha --ui mocha-retry                                                                                                                                              

/home/giovanni/.nvm/v0.10.26/lib/node_modules/mocha/lib/mocha.js:154
  if (!this._ui) throw new Error('invalid interface "' + name + '"');
                       ^
Error: invalid interface "mocha-retry"
    at Mocha.ui (/home/giovanni/.nvm/v0.10.26/lib/node_modules/mocha/lib/mocha.js:154:24)
    at Object.<anonymous> (/home/giovanni/.nvm/v0.10.26/lib/node_modules/mocha/bin/_mocha:195:7)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:902:3
~/proj/rabiscos/mochatestreporter$ ../../mocha/bin/mocha --ui mocha-retry                                                                                                                              


  a suite
    ✓ tests 


  1 passing (7ms)

~/proj/rabiscos/mochatestreporter$ mocha -R mocha-teamcity-reporter                                                                                                                                    

/home/giovanni/.nvm/v0.10.26/lib/node_modules/mocha/lib/mocha.js:137
    if (!_reporter) throw new Error('invalid reporter "' + reporter + '"');
                          ^
Error: invalid reporter "mocha-teamcity-reporter"
    at Mocha.reporter (/home/giovanni/.nvm/v0.10.26/lib/node_modules/mocha/lib/mocha.js:137:27)
    at Object.<anonymous> (/home/giovanni/.nvm/v0.10.26/lib/node_modules/mocha/bin/_mocha:191:7)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:902:3
~/proj/rabiscos/mochatestreporter$ ../../mocha/bin/mocha -R mocha-teamcity-reporter
##teamcity[testSuiteStarted name='a suite']
##teamcity[testStarted name='tests' captureStandardOutput='true']
##teamcity[testFinished name='tests' duration='0']
##teamcity[testSuiteFinished name='a suite' duration='NaN']
##teamcity[testSuiteFinished name='mocha.suite' duration='5']
~/proj/rabiscos/mochatestreporter$ 

@giggio
Copy link
Contributor Author

giggio commented Jul 13, 2014

When you run mocha you will always get the error, because you are running the global one, not yet fixed.
To test it you either globally install the version with the fix, or run it from a separate directory (like I did above).

@jbnicolai
Copy link

... ok, that last mistake on my part was just embarrassing.

Tested, and works perfectly well indeed 😄
Again, thanks for all the work you put into this!

i request the highest of fives_a52e8f_4143575

jbnicolai pushed a commit that referenced this pull request Jul 13, 2014
Allow local modules to work as UIs and Reporters when running mocha globally
@jbnicolai jbnicolai merged commit fdce496 into mochajs:master Jul 13, 2014
@giggio
Copy link
Contributor Author

giggio commented Jul 13, 2014

Nice! Thanks for the support. I will be able to run regular mocha again. :D
I am able to help on other issues, just let me know if you need some help elsewhere.

@jbnicolai
Copy link

Nope, already happy you merged in the CDN option on chromedriver a while back ;-) fixed some issues I was having being my proxy at work.

travisjeffery added a commit that referenced this pull request Jul 24, 2014
tandrewnichols pushed a commit to tandrewnichols/mocha that referenced this pull request Dec 15, 2014
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