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

Add when() to BDD #765

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@dave-irvine

dave-irvine commented Mar 6, 2013

This is useful if you want to maintain a group of tests, only some of which
need to run in certain circumstances. For example, testing your shimmed
Array.forEach, but only on browsers that don't support it natively.

Rather than including/excluding specific test files depending on which
platform you are testing, simply wrap the questionable test suite in a
when() block.

Added when() to BDD. This allows you to specify nested suites that ar…
…e only run

if a truthy value is passed to when().

This is useful if you want to maintain a group of tests, only some of which
need to run in certain circumstances. For example, testing your shimmed
Array.forEach, but only on browsers that don't support it natively.
@stephenmathieson

This comment has been minimized.

Show comment
Hide comment
@stephenmathieson
Contributor

stephenmathieson commented Mar 10, 2013

+1

@stephenmathieson

This comment has been minimized.

Show comment
Hide comment
@stephenmathieson
Contributor

stephenmathieson commented Mar 21, 2013

@visionmedia thoughts?

@travisjeffery

This comment has been minimized.

Show comment
Hide comment
@travisjeffery

travisjeffery Dec 7, 2013

Contributor

demand seems pretty low and this implementation doesn't save you very much - could just wrap your describes in an if/else

Contributor

travisjeffery commented Dec 7, 2013

demand seems pretty low and this implementation doesn't save you very much - could just wrap your describes in an if/else

@lo1tuma

This comment has been minimized.

Show comment
Hide comment
@lo1tuma

lo1tuma Jan 29, 2014

We actually need something like this.

At the moment we have a hacky implementation where you can write the tests as the following:

describe('mySharedModule', function () {
    whenRunInBrowser.it('should do something special', function () {
        // ...
     });

    whenRunInBrowser.describe('dom events', function () {
        // ...
    });
});

lo1tuma commented Jan 29, 2014

We actually need something like this.

At the moment we have a hacky implementation where you can write the tests as the following:

describe('mySharedModule', function () {
    whenRunInBrowser.it('should do something special', function () {
        // ...
     });

    whenRunInBrowser.describe('dom events', function () {
        // ...
    });
});
@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Jan 30, 2014

Contributor

wrapping with if like @travisjeffery suggested works

Contributor

tj commented Jan 30, 2014

wrapping with if like @travisjeffery suggested works

@dave-irvine

This comment has been minimized.

Show comment
Hide comment
@dave-irvine

dave-irvine Jan 30, 2014

Just an aside (at this point I'm not fussed if this gets merged, I just patch it in manually whenever you cut a new Mocha release), a large part of the usefulness of this as opposed to wrapping the tests in an if block is that the tests still show up, but as pending tests.

This means nobody can run the tests and then ask where the tests for a specific condition are. They will show up as pending.

dave-irvine commented Jan 30, 2014

Just an aside (at this point I'm not fussed if this gets merged, I just patch it in manually whenever you cut a new Mocha release), a large part of the usefulness of this as opposed to wrapping the tests in an if block is that the tests still show up, but as pending tests.

This means nobody can run the tests and then ask where the tests for a specific condition are. They will show up as pending.

@lo1tuma

This comment has been minimized.

Show comment
Hide comment
@lo1tuma

lo1tuma Jan 30, 2014

@visionmedia yes, thats what our implementation of whenRunInBrowser.it does. But it would be too much boilerplate code if you have to write the following every time:

if (isBrowserEnvironment) {
    it('should do something special', function () {
        // ...
    });
} else {
    it('should do something special');
}

lo1tuma commented Jan 30, 2014

@visionmedia yes, thats what our implementation of whenRunInBrowser.it does. But it would be too much boilerplate code if you have to write the following every time:

if (isBrowserEnvironment) {
    it('should do something special', function () {
        // ...
    });
} else {
    it('should do something special');
}
@hallas

This comment has been minimized.

Show comment
Hide comment
@hallas

hallas Jan 30, 2014

Contributor

@lo1tuma but then just wrap the condition in something else, like your own hacky when implementation, it solves it? Mocha isn't gonna serve the need of everyone, but it'll serve the need of the majority. Most of these projects show that if we can keep them simple and slim, its easier for everyone to use and extend on their own.

Contributor

hallas commented Jan 30, 2014

@lo1tuma but then just wrap the condition in something else, like your own hacky when implementation, it solves it? Mocha isn't gonna serve the need of everyone, but it'll serve the need of the majority. Most of these projects show that if we can keep them simple and slim, its easier for everyone to use and extend on their own.

@lo1tuma

This comment has been minimized.

Show comment
Hide comment
@lo1tuma

lo1tuma Jan 30, 2014

@hallas yes it works for us. I just wanted to point out, there is a demand on a feature like that.

lo1tuma commented Jan 30, 2014

@hallas yes it works for us. I just wanted to point out, there is a demand on a feature like that.

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Jan 30, 2014

Contributor

I don't think it's necessary to add to mocha directly personally, much like express's app.configure() was really just a glorified if statement, it raised more questions than it was worth, even though it does look a bit nicer

Contributor

tj commented Jan 30, 2014

I don't think it's necessary to add to mocha directly personally, much like express's app.configure() was really just a glorified if statement, it raised more questions than it was worth, even though it does look a bit nicer

@sergeyzam

This comment has been minimized.

Show comment
Hide comment
@sergeyzam

sergeyzam Sep 26, 2016

(myCondition ? xit : it)('should', () => test);

sergeyzam commented Sep 26, 2016

(myCondition ? xit : it)('should', () => test);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment