-
-
Notifications
You must be signed in to change notification settings - Fork 993
ci: add test for windows #1029
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
base: master
Are you sure you want to change the base?
ci: add test for windows #1029
Conversation
32d90f3
to
229e609
Compare
a57b64f
to
c82db2c
Compare
Apparently there's an issue with npm. I have no idea why, but it shouldn't be related to this PR. However, it's something we need to fix if it's affecting the CI. |
a858dca
to
5ddb90a
Compare
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.
Some shell scripting notes, including notes from shellcheck.
6c83144
to
3f081b2
Compare
package.json
Outdated
@@ -26,7 +26,7 @@ | |||
"eslint-plugin-markdown": "3.0.1", | |||
"express": "4.17.3", | |||
"mocha": "10.2.0", | |||
"nyc": "15.1.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.
To fix a small issue with Windows during installation
27e866d
to
8fc9cce
Compare
The CI passes 🥳, wow, i really struggled with the old versions of Node.js. |
package.json
Outdated
@@ -39,7 +39,7 @@ | |||
}, | |||
"scripts": { | |||
"lint": "eslint . && node ./scripts/lint-readme.js", | |||
"test": "./test/support/gencert.sh && mocha --require test/support/env --check-leaks --bail --no-exit --reporter spec test/", | |||
"test": "bash test/support/gencert.sh && mocha --require test/support/env --check-leaks --reporter spec test/", |
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.
And while we're at it, let's remove '--bail' so we can see which tests fail simultaneously instead of one by one
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 (specially the shell part). I verified on my macos with npm run test
and ./test/support/gencert.sh
.
I just tried it now that I have Windows, and this doesn't work. |
a678afe
to
bc8e000
Compare
Signed-off-by: Sebastian Beltran <[email protected]>
bc8e000
to
695134e
Compare
Related nodejs/citgm#1095 (comment)