-
Notifications
You must be signed in to change notification settings - Fork 888
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: integrate midnight smoker in ci #1663
feat: integrate midnight smoker in ci #1663
Conversation
feat: include examples in pack & add script for midnight smoker test
chore: remove files that doesn't exists on root
Add smoke file & update ci script
package.json
Outdated
@@ -25,10 +23,11 @@ | |||
"lint": "eslint .", | |||
"prepublishOnly": "tap --no-check-coverage test/internals/version.test.js", | |||
"test": "npm run lint && npm run transpile && tap --ts && jest test/jest && npm run test-types", | |||
"test-ci": "npm run lint && npm run transpile && tap --ts --no-check-coverage --coverage-report=lcovonly && npm run test-types", | |||
"test-ci": "npm run lint && npm run transpile && tap --ts --no-check-coverage --coverage-report=lcovonly && npm run test-types && npx midnight-smoker test-ci-smoke", |
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 additional dependency is needed?
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.
Nope
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 would prefer to avoid npx
and add midnight-smoker
as a devdep.
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.
@mcollina, added midnight-smoker
as dev-dep. Included separate script as well
fix: midnight-smoker as dev dependency
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.
lgtm
@mcollina, the pipeline broke for some of the case, npm version compatibility
|
@couragecowardlydog is it possible to run midnight smoker not through "exec" command? |
@kibertoad, can I change the script like this
is that fine ? |
I see no problem with that, if it ever stops working for whatever reason, CI will fail |
Signed-off-by: couragecowardlydog <vivekanandansakthivelu@gmail.com>
@kibertoad Fixed that, can you please approve the workflow to see if CI goes through |
Signed-off-by: couragecowardlydog <vivekanandansakthivelu@gmail.com>
fix: remove smoke part of test-ci
Signed-off-by: couragecowardlydog <vivekanandansakthivelu@gmail.com>
Co-authored-by: Matteo Collina <matteo.collina@gmail.com>
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fix: #1570
@mcollina @jsumners , please review and provide your comments
Added a copy of example/basic.js in root as smoke.js