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.

Sign up for GitHub

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

Jump to bottom

Global beforeEach/afterEach should synchronize before module beforeEach/afterEach run #647

Closed
JamesMGreene opened this issue Sep 3, 2014 · 1 comment
Closed

Global beforeEach/afterEach should synchronize before module beforeEach/afterEach run #647

JamesMGreene opened this issue Sep 3, 2014 · 1 comment
Assignees
@leobalter
Labels
Type: Bug Something isn't working right.

Comments

@JamesMGreene
Copy link
Member

As noted in #634 (comment) and #634 (comment), there is a bug in the newly introduced beforeEach/afterEach hooks: the global beforeEach/afterEach hooks should be allowed fully synchronize before the module-level beforeEach/afterEach hooks are run. If they don't, it easily leads to race conditions.
@JamesMGreene JamesMGreene mentioned this issue Sep 3, 2014
Merged
@JamesMGreene
Copy link
Member Author

Also, just to add the idea into the mix now: related to #543, Mocha's strategy for infinitely nested modules (suites) is basically accomplished by 2 decisions:

  1. Store suite context objects in a stack
  2. Ensure that there is an unnamed root-level suite (which is what global beforeEach/afterEach hooks get attached to in Mocha)

If we are going to pursue #543 (and I really hope we do), this is very important to keep in mind.

@JamesMGreene JamesMGreene mentioned this issue Sep 3, 2014
Closed
leobalter added a commit to leobalter/qunit that referenced this issue Sep 3, 2014
@leobalter
Test: Synchronize hooks order
bd3ffd5 
Fixes qunitjs#647
leobalter added a commit to leobalter/qunit that referenced this issue Sep 3, 2014
@leobalter
Test: Synchronize accepts an array of functions
fa025a0 
Ref qunitjs#647
@leobalter leobalter mentioned this issue Sep 3, 2014
Closed
leobalter added a commit to leobalter/qunit that referenced this issue Sep 3, 2014
@leobalter
Test: Synchronize accepts an array of functions
10b2457 
Ref qunitjs#647
leobalter added a commit to leobalter/qunit that referenced this issue Sep 3, 2014
@leobalter
Test: Synchronize hooks order
1e61e3a 
Fixes qunitjs#647
leobalter added a commit to leobalter/qunit that referenced this issue Sep 4, 2014
@leobalter
Test: Synchronize accepts an array of functions
1ab0eef 
Ref qunitjs#647
jzaefferer added a commit to jzaefferer/qunit that referenced this issue Oct 10, 2014
@jzaefferer
Core: Remove the newly introduced global hooks until we have consensus
e6262d9 
Between locating the hooks in `QUnit` or `QUnit.config` and making them
simple setters and callback lists (like QUnit.done et al) and upcoming
plans for nested suites, we decided not to release this feature, for now.

I'm keeping the abstractions for hooks in place, so it should be trivial
to bring this back in whatever form we decide on later.

Effectively reverts 5ee31a0 and follow-up
commits.

Fixes qunitjs#665
Ref qunitjs#633
Ref qunitjs#635
Ref qunitjs#647
@jzaefferer jzaefferer mentioned this issue Oct 10, 2014
Merged
jzaefferer added a commit to jzaefferer/qunit that referenced this issue Oct 17, 2014
@jzaefferer
Core: Remove the newly introduced global hooks until we have consensus
acfd98c 
Between locating the hooks in `QUnit` or `QUnit.config` and making them
simple setters and callback lists (like QUnit.done et al) and upcoming
plans for nested suites, we decided not to release this feature, for now.

I'm keeping the abstractions for hooks in place, so it should be trivial
to bring this back in whatever form we decide on later.

Effectively reverts 5ee31a0 and follow-up
commits.

Fixes qunitjs#665
Ref qunitjs#633
Ref qunitjs#635
Ref qunitjs#647
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@leobalter leobalter

Labels
Type: Bug Something isn't working right.
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Test: Fix test hooks order leobalter/qunit
2 participants
@leobalter @JamesMGreene