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

Error in modules's nested executeNow call forces subsequent modules to become children #1478

Closed
Eric162 opened this issue Aug 27, 2020 · 2 comments · Fixed by #1644
Closed
Assignees
Labels
Component: Core For module, test, hooks, and runner. Type: Bug

Comments

@Eric162
Copy link

Eric162 commented Aug 27, 2020

The executeNow.call is not protected, which means when that if that fails, the rest of processModule is not executed.

executeNow.call( module.testEnvironment, moduleFns );

Because of this, the module which failed becomes the currentModule, AKA the parent of the rest of the modules, and since it has the { skip: true } option, its children will all be skipped (which is now every module loaded after this one).

See this CodePen for an example:
https://codepen.io/eric162/pen/mdPmjJZ?editors=1010

The issue is especially apparent with module.skip, as it would appear that all tests have passed unless a close inspection of the test output is made.

With tools like ember-exam which do load balancing and load modules dynamically, this could mean that 1 module may be skipped, or hundreds/thousands in large test suites. Developers unfamiliar with the usual number of skipped tests in a repo would not know how many are supposed to be skipped, and even those who are familiar may not notice if it happens to be a small increase.

Though this explanation references how Ember uses qunit, the issue may be best resolved here.

Links from codepen comments below.

Ember-cli-test-loader:
https://github.com/ember-cli/ember-cli-test-loader/blob/bb4f22a525d3698454c14f8091be4d90522adfff/addon-test-support/index.js#L19

Ember-qunit:
https://github.com/emberjs/ember-qunit/blob/ef3e5ee10880be849509c1084436b311aa875e58/addon-test-support/test-loader.js#L35

@Krinkle Krinkle added Component: Core For module, test, hooks, and runner. Type: Bug labels Aug 27, 2020
@Krinkle
Copy link
Member

Krinkle commented Aug 27, 2020

I suspect this maybe related to #1407 or #1416, which I'm currently working on fixing for the next patch release.

@Krinkle
Copy link
Member

Krinkle commented Mar 13, 2021

#1416 has been fixed and released (in QUnit 2.13), but I can still reproduce this bug with QUnit 2.14:

https://codepen.io/Krinkle/pen/ExNMxrg

Krinkle added a commit to Krinkle/qunit that referenced this issue Aug 2, 2021
This asserts the undesirable status quo, where the stack is left
in a bad state if the module closure encounters a global error while
test suites are still being loaded.

Ref qunitjs#1478.
@Krinkle Krinkle self-assigned this Aug 2, 2021
Krinkle added a commit to Krinkle/qunit that referenced this issue Aug 24, 2021
This asserts the undesirable status quo, where the stack is left
in a bad state if the module closure encounters a global error while
test suites are still being loaded.

Ref qunitjs#1478.
Krinkle added a commit that referenced this issue Aug 24, 2021
This asserts the undesirable status quo, where the stack is left
in a bad state if the module closure encounters a global error while
test suites are still being loaded.

Ref #1478.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core For module, test, hooks, and runner. Type: Bug
Development

Successfully merging a pull request may close this issue.

2 participants