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

Clarify how `only` behaves #155

Closed
geek opened this issue Aug 12, 2014 · 14 comments · Fixed by #158
Closed

Clarify how `only` behaves #155

geek opened this issue Aug 12, 2014 · 14 comments · Fixed by #158
Assignees
Labels
Milestone

Comments

@geek
Copy link
Member

@geek geek commented Aug 12, 2014

Related to #110 (comment)

var Lab = require('lab');
var lab = exports.lab = Lab.script();

lab.describe('Level 1', function() {

  lab.it('should not run', function() {
    throw new Error();
  });

  lab.describe.only('Level 2', function() {
    lab.it('should run', function(done) {
      done();
    });
  });
});
@geek geek added the bug label Aug 12, 2014
@geek geek added this to the 4.0.2 milestone Aug 12, 2014
@geek geek self-assigned this Aug 13, 2014
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 13, 2014

This is not a bug.

@geek

This comment has been minimized.

Copy link
Member Author

@geek geek commented Aug 13, 2014

The solution is to clarify how only behaves.

@geek geek closed this in #158 Aug 13, 2014
@geek geek changed the title Experiments marked with only aren't always the only ones executed Clarify how `only` behaves Aug 13, 2014
@geek geek added documentation and removed bug labels Aug 13, 2014
@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Aug 14, 2014

This is misleading for those coming from mocha but ok, then according to the new description, that snippet should still pass or am I wrong ? If I put the only on the 2nd test it will still fail. I still don't understand how it behaves.

@geek

This comment has been minimized.

Copy link
Member Author

@geek geek commented Aug 14, 2014

When they are at the same level in the tree only one will run, for example:

describe('Test CLI Not Only', function () {

    it('should not run', function (done) {

        throw new Error();
    });
});


describe.only('Test CLI Only', function () {

    it('should run', function (done) {

        done();
    });
});
@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Aug 14, 2014

OK so that's a matter of level, that's much clearer this way. It won't be as useful as I expected it to be then. What motivated that choice ?

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 14, 2014

Are you expecting only to work across experiments and tests? So if a test is next to an experiment with only, that test is not executed?

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Aug 14, 2014

I expect the 1st only encountered to be the only thing that runs ever, be it a test or an experiment, and across scripts as well. It should also run only the necessary before/after as well.
I thought my intent was clear in the original PR, maybe I did a poor job at explaining what it was doing.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Aug 14, 2014

Unit tests included in #116 are what I expect, the only missing test is across scripts, I don't think they existed when I did that PR.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 15, 2014

You change touched the runner which is not something I am willing to do. The runner has to stay simple.

So you want only to basically run a single test no matter where it is?

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Aug 15, 2014

A single test, or experiment with all its nested tests or experiments, yes.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Aug 15, 2014

BTW I put it in the runner because that's the only obvious point where all the tests were loaded, if you see anywhere else to put it that's fine as well.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 15, 2014

I think the entire feature is misguided. This is basically a hack to allow developers to trick automated test triggers to run only one test. We should document that lab's only works differently from mocha's. I am not going to change the current behavior. I don't care about feature parity with mocha.

What we can do is add a cli override mechanism. I'll start a new issue for that.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Aug 15, 2014

It's not about feature parity at all, I've already explained at great lengths the use case, the feature is just not that useful to you it seems. Maybe being able to override -g or -i inside the test suite would be enough.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 15, 2014

I understand the feature, I just don't agree with how mocha chose to implement it. Try out the new cli override feature and see if that works. We can also enhance test selection by introducing tags or labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.