-
Notifications
You must be signed in to change notification settings - Fork 158
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
Restore the legacy tests #836
Restore the legacy tests #836
Conversation
The tests were removed in this PR ember-fastboot#805
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.
Lovely! Glad you found this.
As discussed, we will hopefully revisit the test harness setup for these tests after this PR.
@@ -90,3 +90,34 @@ jobs: | |||
run: yarn workspace basic-app test:mocha | |||
- name: Custom App | |||
run: yarn workspace custom-fastboot-app test:mocha | |||
|
|||
test-legacy-mocha: | |||
name: Legacy Mocha Tests - ${{ matrix.node-version }} |
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.
I wonder what is legacy about these
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.
I agree that we probably shouldn't call them legacy 🤔 but I wouldn't think it's a blocker
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.
looks good to me!
f07c3ef
to
d30117d
Compare
@@ -46,6 +46,7 @@ | |||
"body-parser": "^1.18.3", | |||
"broccoli-asset-rev": "^3.0.0", | |||
"broccoli-test-helper": "^1.5.0", | |||
"co": "4.6.0", |
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.
const co = require('co'); |
Looks like
co
isn't actually being used, we should be able to remove
# Remove test-packages folder so that we don't leak node_modules between apps | ||
- name: Remove test-packages | ||
run: | | ||
rm -rf test-packages |
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.
Curious why do we need to remove?
The tests were removed in this PR #805
These tests have not been exercised this the above PR.
https://github.com/ember-fastboot/ember-cli-fastboot/tree/master/packages/ember-cli-fastboot/test
Adding the npm package
[co](https://www.npmjs.com/package/co)
which is used when running the legacy tests