-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Feature] Update initializer blueprint for ember-mocha 0.14 #17411
Conversation
<% if (destroyAppExists) { %>import destroyApp from '../../helpers/destroy-app';<% } else { %>import { run } from '@ember/runloop';<% } %> | ||
|
||
describe('<%= friendlyTestName %>', function() { | ||
let hooks = setupTest(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this? we don't seem to be using anything from the test context 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, looks like the QUnit variant is doing the same thing. @rwjblue do you remember why we use it there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I made it do the same as the QUnit one for now, but also wondered about this. See my note here: #17412
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue do you? 👉 #17411 (comment)
Would love to finish this and #17412, to update the TS blueprints as well! 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, seems like we can/should remove from both mocha and qunit. Sorry for the delay here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed setupTest()
! Ready for final review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here for QUnit again: #17713
9376b05
to
430aed2
Compare
As it is not needed, see emberjs#17411 (comment)
Hmm, looks like the blueprint tests are failing still. |
As it is not needed, see emberjs#17411 (comment)
430aed2
to
84700f6
Compare
Thank you @simonihmig! |
The previous PR emberjs#17411 and its last-minute change to remove `setupTest()` introduced a very stupid error! 🤦♂️
Related to #16863