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

test.only is not working if code is loading after mocha.run() #2077

Closed
TheLevenCreations opened this issue Jan 27, 2016 · 6 comments
Closed
Labels
duplicate been there, done that, got the t-shirt... type: bug a defect, confirmed by a maintainer

Comments

@TheLevenCreations
Copy link

Hi, I am a newbie to use Mocha and I think it is a really really cool tool to test my javascript code. Thanks for creating such great tool.

However, I met some problem to test my AMD module code. See the following. Please let me know if it is worked as designed.

Currently, I am using mocha delay method to test AMD modules. However, I just found the test.only is not working. My way is:

  1. Use node(or electron) to start a thread to load bin/_mocha javascript file.
  2. bin/_mocha will find the test/mocha.opts file and load it, then run the tests specified in opts. (--delay is specified in opts)
  3. Actually, there is only one TestsLoader.test.js in opts. That file is to load the AMD scripts and when the loading is done, fire run() method provided by mocha

It works well until I found test.only won't work, while test.only works well in browser mode.

After some investigation, I found probably it is a design issue. The runner instance is started when mocha.run() is fired, and the options.grep is stored in runner immediately. See the following code I grabbed from mocha.js. As a result, whiling loading AMD modules after mocha.run() is fired and before run() is issued, the test.only will change the mocha.options.grep, but it won't change the runner's grep. As a result, when the run() and then runner.runTests() is issued, the grep is not working.

/**
 * Run tests and invoke `fn()` when complete.
 *
 * @api public
 * @param {Function} fn
 * @return {Runner}
 */
Mocha.prototype.run = function(fn) {
  if (this.files.length) {
    this.loadFiles();
  }
  var suite = this.suite;
  var options = this.options;
  options.files = this.files;
  var runner = new exports.Runner(suite, options.delay);
  var reporter = new this._reporter(runner, options);
  runner.ignoreLeaks = options.ignoreLeaks !== false;
  runner.fullStackTrace = options.fullStackTrace;
  runner.asyncOnly = options.asyncOnly;
  runner.allowUncaught = options.allowUncaught;
  if (options.grep) {
    runner.grep(options.grep, options.invert);
  }
  if (options.globals) {
    runner.globals(options.globals);
  }
  if (options.growl) {
    this._growl(runner, reporter);
  }
  if (options.useColors !== undefined) {
    exports.reporters.Base.useColors = options.useColors;
  }
  exports.reporters.Base.inlineDiffs = options.useInlineDiffs;

  function done(failures) {
    if (reporter.done) {
      reporter.done(failures, fn);
    } else {
      fn && fn(failures);
    }
  }

  return runner.run(done);
};

I just think it would be better to let runner to read mocha.options.grep rather than reading a temp value stored.

My current workaround is to rewrite the _mocha file and:

  1. register mocha, suite, test as global value
  2. to load the AMD scripts, set delay mode =false
  3. fire mocha.run()

Please let me know I am doing towards the right direction. Thanks :)

@boneskull
Copy link
Member

using globals is kind of heavy-handed to fix this, but if it works for you, then that's good.

would accept a PR to fix this.

@boneskull boneskull added type: bug a defect, confirmed by a maintainer status: accepting prs Mocha can use your help with this one! labels Jan 27, 2016
@ScottFreeCode
Copy link
Contributor

Rather than having Runner use mocha.options.grep, what about having Mocha.prototype.run save this._runner* instead of a local runner variable and Mocha.prototype.grep call this._runner.grep in addition to assigning this.options.grep? This keeps Runner agnostic to Mocha/mocha (Mocha already "knows about" Runner, so we're not adding any coupling there), only affects the case we're trying to fix (e.g. doesn't affect anything using Runner and/or its grep command directly, if there is any such code), and is about as trivial to implement as the most naive and potentially problematic implementation of the "Runner use mocha.options.grep" alternative.

*This isn't strictly private, but marked as intended to be private by the underscore variable name convention, which other Mocha internal variables already are too.

@boneskull
Copy link
Member

@ScottFreeCode Sounds reasonable

@stale
Copy link

stale bot commented Oct 17, 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 added the stale this has been inactive for a while... label Oct 17, 2017
@boneskull boneskull removed the stale this has been inactive for a while... label Oct 17, 2017
@ScottFreeCode
Copy link
Contributor

This is a duplicate of #1838, right?

@JoshuaKGoldberg
Copy link
Member

Looks like it! This issue also hasn't been interacted with in quite a while.

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Dec 27, 2023
@JoshuaKGoldberg JoshuaKGoldberg added duplicate been there, done that, got the t-shirt... and removed status: accepting prs Mocha can use your help with this one! labels Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate been there, done that, got the t-shirt... type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

4 participants