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

"commander.opts" function gets overwritten with string "test/mocha.opts" in grunt-mocha-istanbul #2752

Closed
wluu opened this issue Mar 25, 2017 · 4 comments
Labels
stale this has been inactive for a while... status: needs upstream fix defect within Mocha's dependency tree type: question support question

Comments

@wluu
Copy link

wluu commented Mar 25, 2017

I did file an issue against grunt-mocha-istanbul and was able to find a workaround for it: pocesar/grunt-mocha-istanbul#67

However, the purpose of this issue is to see if there's a legitimate fix on this side.

Environment:

  • Node: 4.7.3
  • NPM: 3.10.10
  • OS: macOS Sierra 10.12.3
  • Mocha: 3.2.0

Steps to reproduce:

  1. Download and unzip sandbox3.zip
  2. Run npm install && npm test

Observed behavior:

Running "mocha_istanbul:coverage" (mocha_istanbul) task


  commander
    1) exposes extension functions


  0 passing (9ms)
  1 failing

  1) commander exposes extension functions:
     AssertionError: expected 'test/mocha.opts' to be a function
    expected 'test/mocha.opts' to have type function
        expected 'string' to be 'function'
      at Assertion.fail (node_modules/should/cjs/should.js:258:17)
      at Assertion.Object.defineProperty.value [as Function] (node_modules/should/cjs/should.js:335:19)
      at Context.<anonymous> (commander_test.js:9:463)

Notes:

var program = null,
      should = require('should');

describe('commander', function () {
	before(function () {
		delete require.cache[require.resolve('commander')];
		program = require('commander');
	});

	it('exposes extension functions', function () {
		program.parse.should.be.a.Function();
		program.opts.should.be.a.Function();
	});
});
@ScottFreeCode
Copy link
Contributor

•If we run the above sample attachment with mocha@2.5.3 , then our assertion passes.

Only by coincidence -- Mocha 2 has an earlier version of Commander, which results in it getting a separate instance of the package. If you use Mocha 2 but remove its copy of Commander, the issue occurs.

•If we run our unit test with mocha@3.2.0 as a standalone, then our assertion passes.

If I run node_modules/.bin/mocha commander_test.js I still get the issue, so it isn't being directly caused by any Grunt plugins. Maybe you were running global Mocha, which would also have used its own instance/copy of Commander? (Grunt, on the other hand, may be running the local Mocha even if Grunt is called globally. We haven't got around to making Mocha do that, but we really should, because it surprises a lot of people that Mocha might not do what they expect if they have a global copy installed.)

Working on digging into the issue itself...

@ScottFreeCode
Copy link
Contributor

As far as I can tell, ultimately this comes down to Commander modifying the module export, which is basically a global variable in effect.

There is a Commander issue about program.opts specifically: tj/commander.js#584

However, in principle you're always going to have problems (or at least a high risk of problems) using Commander in the same process as it's already been used, since the results of the previous usage will still be on the module export. To really eliminate these sorts of issues, Commander would have to be changed such that the argument processing does not modify the module export.

@wluu
Copy link
Author

wluu commented Mar 25, 2017

@ScottFreeCode Correct, I was using global Mocha to run commander_test.js for the standalone scenario.

@drazisil drazisil added the type: question support question label Mar 30, 2017
@stale stale bot added the stale this has been inactive for a while... label Jul 29, 2017
@stale
Copy link

stale bot commented Jul 29, 2017

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@stale stale bot closed this as completed Aug 12, 2017
@ScottFreeCode ScottFreeCode added the status: needs upstream fix defect within Mocha's dependency tree label Aug 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale this has been inactive for a while... status: needs upstream fix defect within Mocha's dependency tree type: question support question
Projects
None yet
Development

No branches or pull requests

3 participants