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

beforeEach / afterEach only execute at the current describe/Suite level, not child levels #67

Closed
RoystonS opened this issue Jul 10, 2013 · 18 comments
Labels
bug Something that's not working as intended enhancement A new or improved feature
Milestone

Comments

@RoystonS
Copy link

It's quite difficult to arrange setup + teardown behaviour with well-nested describe contexts in Intern: Intern only runs the beforeEach/afterEach for tests at the same describe/Suite level rather than for tests in descendant levels as well.

Other frameworks that offer BDD-interfaces with functions 'beforeEach' / 'afterEach' (e.g. Jasmine, RSpec) run the beforeEach functions before every test in the same level and all descendant levels.

With beforeEach not executing code before each and every test, I'm ending up having to construct a hierarchy of nested setup + teardown functions, which is very error-prone. I thought at first this was just my mistake, but having seen that at least Jasmine and RSpec drive beforeEach as I'd expect, this feels more like an issue in Intern? (I can't find an Intern test that is asserting its current nesting behaviour so I'm not sure it's intended.)

Here's some test code to illustrate in slightly more concrete terms:

define(["intern!bdd", "intern/chai!expect"], function(bdd, expect) {
    bdd.describe("Top-level describe", function() {
        bdd.before(function() {
            console.log("Top-level before");
        });

        bdd.beforeEach(function() {
            console.log("Top-level beforeEach");
        });

        bdd.after(function() {
            console.log("Top-level after");
        });

        bdd.afterEach(function() {
            console.log("Top-level afterEach");
        });

        bdd.it("Top-level it", function() {
            console.log("Top-level it #1");
        });

        bdd.it("Top-level it", function() {
            console.log("Top-level it #2");
        });

        bdd.describe("Nested describe", function() {
            bdd.before(function() {
                console.log("Nested-level before");
            });

            bdd.beforeEach(function() {
                console.log("Nested-level beforeEach");
            });

            bdd.after(function() {
                console.log("Nested-level after");
            });

            bdd.afterEach(function() {
                console.log("Nested-level afterEach");
            });

            bdd.it("Nested-level it", function() {
                console.log("Nested-level it #1");
            });

            bdd.it("Nested-level it", function() {
                console.log("Nested-level it #2");
            });
        });
    });
});

I would expect the following output:

Top-level before
Top-level beforeEach
Top-level it #1
PASS: Top-level it (1ms)
Top-level afterEach
Top-level beforeEach
Top-level it #2
PASS: Top-level it (1ms)
Top-level afterEach
Nested-level before
Top-level beforeEach
Nested-level beforeEach
Nested-level it
PASS: Nested-level it
Nested-level afterEach
Top-level afterEach
Top-level beforeEach
Nested-level beforeEach
Nested-level it
PASS: Nested-level it
Nested-level afterEach
Top-level afterEach
Nested-level after
Top-level after

(Not quite sure about some of the detail, but I'm definitely expecting to see the top-level beforeEach just before each nested-level beforeEach.)

however Intern 1.1 gives me:

Top-level before
Top-level beforeEach
Top-level it #1
PASS: Top-level it (1ms)
Top-level afterEach
Top-level beforeEach
Top-level it #2
PASS: Top-level it (1ms)
Top-level afterEach
Top-level beforeEach
Nested-level before
Nested-level beforeEach
Nested-level it
PASS: Nested-level it
Nested-level afterEach
Nested-level beforeEach
Nested-level it
PASS: Nested-level it
Nested-level afterEach
Nested-level after
Top-level afterEach
Top-level after

There's a good sample of RSpec behaviour described here: http://www.wulftone.com/2012/01/22/rspec-gotchas-before-after-all-and-each/

And another page which describes the usefulness of per-test beforeEach functions in Jasmine here: http://thatextramile.be/blog/2011/08/take-advantage-of-your-bdd-framework/

Thanks!

@RoystonS
Copy link
Author

Can anybody comment on this? (I've also just confirmed that Mocha's beforeEach also works the same way as Jasmine and RSpec, and runs before every test rather than everything at the current level.)

@csnover
Copy link
Member

csnover commented Jul 24, 2013

I can comment on it insofar to confirm that nested beforeEach/afterEach are not all executed. It’s not really a defect insofar that Intern never intended to execute beforeEach/afterEach from parent suites. I need to do more research to understand whether or not it’s something we really ought to change.

@RoystonS
Copy link
Author

Thanks Colin.

When you're working with nested contexts and the bdd interface, it's certainly extremely desirable that beforeEach does indeed run before each test, otherwise in the face of nested describes, beforeEach loses almost all utility as it then works in almost the same way as before.

With the current beforeEach semantics, we ended up writing, at each level of describe, a function that sets up the context for the current describe, but firstly calling the same function from the previous level. Very ugly, error-prone and manual.

To test it all out, I've actually written a local extension which I've called beforeEvery/afterEvery, which works as the beforeEach in Mocha, Jasmine et al, and indeed runs before/after every test. With that, we've ended up deleting all of that manual setup chaining. In the face of beforeEvery/afterEvery, we no longer ever use the plain official Intern beforeEach as it's never a useful concept.

(Btw, I'd much rather see Intern change its beforeEach semantics, of course, as that'd make it easier to switch between Intern and Mocha/Jasmine, and make the semantics match the name and expectations from other frameworks. The local extension isn't really suitable for a pull request, I'm afraid, as it just fits into the bdd layer rather than modifying the underlying Suite object.)

@csnover
Copy link
Member

csnover commented Jul 30, 2013

This is scheduled for 1.3.

@RoystonS
Copy link
Author

Lovely. Thanks very much.

@csnover
Copy link
Member

csnover commented Aug 8, 2013

I’m going to go ahead and upgrade this to a bughancement since it seems like it’s working the way nobody expects it to work. :)

edhager added a commit to edhager/intern that referenced this issue Aug 26, 2013
edhager added a commit to edhager/intern that referenced this issue Aug 29, 2013
@RoystonS
Copy link
Author

I ran this against some unit tests I already had for a temporary 'beforeEvery/afterEvery' implementation I already had, and it's exposing some unexpected afterEach rewind behaviour with the current version of this patch:

If I have three levels of beforeEach/afterEach (called 'top', 'middle' and 'bottom'), then I'd expect code to run in the following order for tests in the 'bottom' level:

top-level beforeEach
middle-level beforeEach
bottom-level beforeEach
bottom-level test
bottom-level afterEach
middle-level afterEach
top-level afterEach

However, the current version of this request produces the following order instead:

top-level beforeEach
middle-level beforeEach
bottom-level beforeEach
bottom-level test
top-level afterEach
middle-level afterEach
bottom-level afterEach

That is, the afterEach unwinds top-to-bottom rather than bottom-to-top, which seems rather unexpected.

@csnover
Copy link
Member

csnover commented Oct 3, 2013

@RoystonS Do you expect remaining beforeEach/afterEach methods from parent suites to continue to execute if one of the earlier ones fails?

@csnover
Copy link
Member

csnover commented Oct 3, 2013

@RoystonS Also, do you expect that beforeEach/afterEach will not run when a child suite is entered? i.e. in the original example do you expect to see “top-level beforeEach” before “Nested describe” and then again before “Nested-level it”, or only before “Nested-level it”?

@RoystonS
Copy link
Author

RoystonS commented Oct 3, 2013

@csnover Excellent question about calling parent beforeEach/afterEach functions. I'd expect that if a beforeEach fails in a particular level, that level is dead: any tests in that level, and child suites below that level, would be expecting that beforeEach to have completed successfully, so they should not be run at all.

afterEach is a bit more interesting: afterEach is rather like a 'finally' block, so I think I'd expect that it'd still try to run parent afterEach functions to get them to clear up even if ones lower down failed. As a concrete example, if I had a top-level beforeEach that opened up a tab in a UI, then a nested series of describes where one of the afterEach calls failed, I'd still hope that as the suites unwound, the top-level afterEach would be called which would close that top-level tab again.

I'm not sure I understand your second question there. I thought that all the code directly in a describe ran at test registration time, and then the beforeEach/it/afterEach comes along during test execution time?

@csnover
Copy link
Member

csnover commented Oct 3, 2013

@RoystonS So the second question is basically, do you expect beforeEach/afterEach of a parent suite to execute when a child suite or child test is executed, or only when a child test is executed?

@RoystonS
Copy link
Author

RoystonS commented Oct 3, 2013

I'm not absolutely 100% on Intern's mapping between suites and bdd describe blocks, so I'll talk in terms of the latter: If I created a describe block containing a beforeEach and an afterEach, but had no tests or child describes in it, I'd not expect that beforeEach/afterEach to be called. But if I did add a child describe with a test in it, I'd expect them to be called around the test. I think I'm saying that beforeEach/afterEach are just tied to tests rather than suites. Code directly inside a describe function isn't called each time up or down the chain - just once at registration time: beforeEach/afterEach are, I think, short for beforeEachTest/afterEachTest. What would it mean in BDD interface terms if they were to execute when suite or test were executed rather than just test?

@csnover
Copy link
Member

csnover commented Oct 3, 2013

each describe is a new suite, and each it is a new test. So what you are describing matches what I expected, that beforeEach/afterEach would not execute on a child suite, only on a child test.

@csnover csnover closed this as completed in 95ce922 Oct 3, 2013
@csnover
Copy link
Member

csnover commented Oct 3, 2013

@RoystonS Please check current master and confirm that everything is working as expected.

@RoystonS
Copy link
Author

RoystonS commented Oct 4, 2013

Code in master now passes the unit tests I had for my implementation of beforeEvery + afterEvery. I'll try to get our full system tests running through against it later today.

@csnover
Copy link
Member

csnover commented Oct 10, 2013

@RoystonS Glad to hear it is passing! Looks like this fix is good then, but please let me know if you run into any problems with your full system tests.

@RoystonS
Copy link
Author

Yep, all is good. We switched all our tests over from using my hokey beforeEvery/afterEvery to the new beforeEach/afterEach and all is well. Thanks!

@atesgoral
Copy link

Fellow search result travellers.

Whatever you do, don't waste hours like I did by doing this:

var beforeEach = bdd.before; // NO.

Instead do:

var beforeEach = bdd.beforeEach; // YES.

stdavis pushed a commit to stdavis/intern that referenced this issue Jun 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that's not working as intended enhancement A new or improved feature
Projects
None yet
Development

No branches or pull requests

3 participants