describe.only runs multiple unrelated tests

[lobodpav]

When multiple unit tests have same name in describe, mocha ignores the .only setting.

Steps to reproduce:

• Create test1.js and test2.js as per template below
• Run mocha test1.js test2.js -R spec
• Expected result
  • You should see only the test1 to get run
• Actual result
  • Both test1 and test2 get run

test1.js

describe.only('Test', function () {
        it('Should run test1 only', function() {});
});

test2.js

describe('Test', function () {
        it('Should run test2', function() {});
});

[endlist]

I've noticed this also happens if the name in test2 is not the same but still includes the name in test1 (e.g. test1 is "Test" and test2 is "AnotherTest", test2 will still be run).

[tj]

yeah it's a bit of a hack right now, it uses .grep() internally, one could argue you shouldn't name the same but yea it's definitely a gotcha

[lobodpav]

Yeah, I intentionally never have two tests with the same name. I often copy&paste unit tests to use it as a template for new module's. And sometimes I forget to rename the unit test. This is why I found this bug :)

However, in large projects this problem may occur more easily.

[kavish28]

I am new to Mocha. so ignore if something i say that doesnot make sense and I am coming from TestNG world.

the way .only works is, for a given file, it skips all other tests and run the test marked describe.only.
I assumed if you have two files and each has same (or different) describe name and u mark one as only, both should run

[lobodpav]

Currently, when I use .only in two different files with two different describes, only one is run. However, if the Strings in describes are equal, then both tests will be run.

In the Mocha doc, .only is described as exclusivity feature and also only one .only() call is respected.

[kavish28]

okay, got it. Thanks for the clarification

[dbushong]

This is actually worse than just the exact same name; grep does a substring match, so describe.only('roll', ...) will also trigger your describe('rollback', ....) in another file.

[boneskull]

sometimes it's nice, and sometimes it isn't.

[drFabio]

Oh . So it's this what is happenning.It was driving me crazy! At least i know how to avoid now

[boneskull]

If anyone has a convincing reason why this behavior should be changed, please let me know--I can see it being used effectively either way.

[drFabio]

I have. For example i have two files one has the file A.js and the other B.js.
Inside A.js i have describe.only('Applicant Consolidation') And all tests i want to run.
Inside B i have describe('CompanyApplicant') and inside it describe('Consolidation CompanyApplicants (step by step) ')

All tests Inside B bellow that point are running.
Besides this is not how anyone would imagine it would work.
If you have a large suit of tests you have to keep an eye on all names, of all describes to guarantee that some name is not "Similar enough"

[boneskull]

Besides this is not how anyone would imagine it would work.

Not in your use case, no. This even suggests to me there's something hinky about the grep implementation.

However I can envision people actually wanting behavior like that.

If we're gonna break the API, we should probably keep the current only() implementation, and rename it to grep() (or something else if somebody has a better suggestion). Then, only() will have a new implementation, which will be much less "fuzzy".

So, any PR will not be merged very soon. PR #1445, if that gets in, may alleviate some of the issues here.

[lobodpav]

Someone should fix this instead of these endless talks about an obvious bug. Will do it myself if I get a bit of time this week. The easiest fix is to just run the first .only occurence and ignore the others, right?

[boneskull]

Not sure what the fix is. You can rush to fix it but it will necessitate a major release.

[lobodpav]

From my perspective, .skip() means to skip a test, .only() means this is the only single one test which shall be run.
If there is a need for another feature like being able to selectively choose limited amount of tests to run, then should there be a new function name introduced? For example .run()

I mean whatever the naming is, I am most wondering what people think .only() should do or what is was originally supposed to do. As I understood from @tj answer it seemed to him as an erroneous behaviour.

[boneskull]

Sure, it's weird and probably erroneous, but it's been around so long that it's probably going to break something for somebody.

[drFabio]

Finally figured out why my test is running. It gets the full name of the describe and performs a grep on each test. So describe.only('Applicant Consolidation') greps the string 'CompanyApplicant '+'Consolidation' of the other test.

So not only on each describe it is an issue, but on all nested describe combinations.

I agree with @boneskull on the fact that if it's going on for a while somebody may be relying on it. But since is more of an undocumented behaviour i don't think that it would enter on a Backward compatibility break.

[travisjeffery]

how .only works shouldn't be affected by the name imo. if someone made a pr fixing this that'd be great.

[lobodpav]

Not sure if fixing this 'bug' will break anybody's code.
The main question is, why people are using .some()? I guess the answer is when wanting to run specific test which fails because there is a bug in the code. I.e. normally I would assume the .only() would never be pushed into any GIT/SVN repo.

[boneskull]

People push .only code to their repos all the time, but usually not on purpose.

Anyway if @travisjeffery doesn't want to keep the current behavior in some manner then that's fine. Reading over the docs it doesn't appear that grep-like behavior was necessarily intended.

I'm not eager to merge this into a patch or minor release, just because of some of the changes we thought were "non-breaking" caused a shitstorm.

[demisx]

This substring matching is a pain in the rear for us as well. When we mark describe as .only we'd expect that only that particular describe block will run. I understand maybe running another describe if the string matches 100%, but substring matches should be excluded IMHO.

[BobbieBarker]

+1, I think this issue is sabotaging my tests right now as well.

[domarmstrong]

+1 this is really needs fixing. I would consider this a bug fix, the documented behaviour is:

The exclusivity feature allows you to run only the specified suite or test-case by appending .only() to the function.

I don't think prioritising upsetting someone relying on a bug over everyone else expecting documented behaviour is a good reason to not patch it.

[boneskull]

if @mochajs/mocha can agree it's a bug, then it could go into a minor

[jbnicolai]

@boneskull rather than discuss that (although I could be convinced to go put it in a minor [that sounds worrying]), how about we change #1782 to become a release of v3.0.0 in stead, merge #1632 and #1481 asap, lift the rest of milestone V3.0.0 to the next major release and ship v3.0.0? :)

[ghost]

This has bitten me multiple times and kind of stinks. Is there a better way we could do preprocessing of all .only() and build out a tree? The desired behavior is such that I can mark a 'top level' suite with only, then as I work through sub-level describes I can attach another only without having to remove the top-level marker.

[evoyy]

This issue is causing problems for me because I want to describe my tests in a hierarchical structure like below, but then it is not possible to run one suite exclusively.

describe('components/Form', function() { ... })
describe('components/FormField', function() { ... })
describe('components/FormControls', function() { ... })

[demisx]

@PerlPong Why not restructure the specs to be something like this:

context('components', function () {
  describe('Form', function () {...});
  describe('FormField', function () {...});
  describe('FormControls', function () {...});
});

[evoyy]

@demisx thanks for the tip, but unfortunately it does not work around this bug. If you execute the following with mocha, both suites will run:

var expect = require('chai').expect;

context('components', function() {
    describe.only('Form', function() {
        it('should be true', function () {
            expect(true).to.be.true;
        });
    });
    describe('FormField', function() {
        it('should be false', function () {
            expect(false).to.be.false;
        });
    });
});

  components
    Form
      ✓ should be true
    FormField
      ✓ should be false

  2 passing (12ms)

[demisx]

@PerlPong I see. Yeah, I believe if the spec describe string matches or starts with the string that you set .only on, it will run that test too. Ran into this inconvenience in the past myself and had to ensure my describes always start with a unique string.

[dasilvacontin]

Closing in favor of #1481. PR #1807 (referencing that other issue) is in the works .