-
Notifications
You must be signed in to change notification settings - Fork 566
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
ci: re-enable testing in Node.js 12 #1646
Conversation
Codecov ReportBase: 94.60% // Head: 94.89% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1646 +/- ##
==========================================
+ Coverage 94.60% 94.89% +0.29%
==========================================
Files 53 53
Lines 4803 4803
==========================================
+ Hits 4544 4558 +14
+ Misses 259 245 -14
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@@ -55,7 +57,7 @@ test('request timeout with readable body', (t) => { | |||
t.type(err, errors.HeadersTimeoutError) | |||
}) | |||
}) | |||
}) | |||
}, { skip: nodeMajor < 14 }) |
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.
As suggested by Matteo: #1645 (comment)
Although I wonder if there's a way to feature detect writableNeedDrain
? I'm not familiar with Node's internals, but it seems I could not find a way to see if the getter for it is declared or not?
@@ -49,7 +49,7 @@ | |||
"test": "npm run test:tap && npm run test:node-fetch && npm run test:fetch && npm run test:jest && tsd", | |||
"test:node-fetch": "node scripts/verifyVersion.js 16 || mocha test/node-fetch", | |||
"test:fetch": "node scripts/verifyVersion.js 16 || (npm run build:node && tap test/fetch/*.js && tap test/webidl/*.js)", | |||
"test:jest": "jest", | |||
"test:jest": "node scripts/verifyVersion.js 14 || jest", |
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.
jest@29+
, used here, does not support Node.js 12...
Seems GH actions are stuck for this one job, wasting precious build minutes? I don't have access to stop/restart that job... |
PTAL #1652 |
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
* ci: re-enable testing in Node.js 12 * ci: bump codecov version * test: disable failing test in node@v12 * test: disable testing in Jest in Node < 14
* ci: re-enable testing in Node.js 12 * ci: bump codecov version * test: disable failing test in node@v12 * test: disable testing in Jest in Node < 14
Closes #1645