Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test: run test on Node.js 23 #543

Merged
merged 6 commits into from
Nov 23, 2024
Merged

test: run test on Node.js 23 #543

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2207519
test: run test on Node.js 23
fengmk2 Oct 19, 2024
78f5c5d
23
fengmk2 Oct 19, 2024
0faf0c3
fix type
fengmk2 Nov 23, 2024
c728555
f
fengmk2 Nov 23, 2024
f21e5b3
f
fengmk2 Nov 23, 2024
db3b6c1
f
fengmk2 Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ jobs:
uses: node-modules/github-actions/.github/workflows/node-test.yml@master
with:
os: 'ubuntu-latest, macos-latest, windows-latest'
version: '18.19.0, 18, 20, 22'
version: '18.19.0, 18, 20, 22, 23'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Update README to Document Node.js 23 Support

The README.md currently does not mention the supported Node.js versions. Please update the documentation to include Node.js 23 to ensure clarity for developers and users.

🔗 Analysis chain

LGTM! Consider documenting Node.js 23 support.

The addition of Node.js 23 to the test matrix is a proactive step in ensuring compatibility with the latest Node.js version. This change will help catch any potential issues early on.

Consider the following follow-up actions:

  1. Update the project's README or documentation to reflect support for Node.js 23.
  2. Verify that all project dependencies are compatible with Node.js 23.
  3. Monitor the CI runs to ensure Node.js 23 tests pass consistently, as it might be a very recent or pre-release version.

Let's check if the README mentions Node.js version support:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if README mentions Node.js version support
# Expected: README should mention supported Node.js versions

rg -i "node\.?js.*(version|support)" README.md

Length of output: 48

Script:

#!/bin/bash
# Description: Broadly search README.md for any mention of Node.js versions or compatibility
# Expected: Any line mentioning Node.js versions, compatibility, or support

rg -i "node\.?js\s*(version|versions|support|compatible|compatibility)"

Length of output: 130

secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
2 changes: 0 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,6 @@ Node.js v22.3.0

[MIT](LICENSE)

<!-- GITCONTRIBUTOR_START -->

## Contributors

[![Contributors](https://contrib.rocks/image?repo=node-modules/urllib)](https://github.com/node-modules/urllib/graphs/contributors)
Expand Down
5 changes: 2 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,10 @@
"exports": {
".": {
"import": {
"source": "./src/index.ts",
"types": "./dist/esm/index.d.ts",
"default": "./dist/esm/index.js"
},
"require": {
"source": "./src/index.ts",
"types": "./dist/commonjs/index.d.ts",
"default": "./dist/commonjs/index.js"
}
Expand All @@ -108,5 +106,6 @@
"src"
],
"types": "./dist/commonjs/index.d.ts",
"main": "./dist/commonjs/index.js"
"main": "./dist/commonjs/index.js",
"module": "./dist/esm/index.js"
}
2 changes: 1 addition & 1 deletion src/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class FetchFactory {
[symbols.kEnableRequestTiming]: !!(init.timing ?? true),
[symbols.kRequestTiming]: timing,
// [symbols.kRequestOriginalOpaque]: originalOpaque,
};
} as FetchOpaque;
const reqMeta: RequestMeta = {
requestId,
url: request.url,
Expand Down
2 changes: 1 addition & 1 deletion test/HttpClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('HttpClient.test.ts', () => {
httpClient.request(_url),
]);
console.log(httpClient.getDispatcherPoolStats());
assert.equal(httpClient.getDispatcherPoolStats()['https://registry.npmmirror.com'].connected, 1);
assert.equal(httpClient.getDispatcherPoolStats()['https://registry.npmmirror.com'].connected, 4);
assert(httpClient.getDispatcherPoolStats()[_url.substring(0, _url.length - 1)].connected > 1);
});
});
Expand Down
9 changes: 7 additions & 2 deletions test/options.writeStream.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,13 @@ describe('options.writeStream.test.ts', () => {
// only Node.js >= 18 has stream.emitError
if (err.message !== 'writeStream is destroyed') {
assert.equal(err.name, 'Error');
assert.equal(err.code, 'ENOENT');
assert.match(err.message, /no such file or directory/);
assert.match(err.code, /^ENOENT|ERR_STREAM_UNABLE_TO_PIPE$/);
if (err.code === 'ERR_STREAM_UNABLE_TO_PIPE') {
// change to ERR_STREAM_UNABLE_TO_PIPE on Node.js >= 23
assert.equal(err.message, 'Cannot pipe to a closed or destroyed stream');
} else {
assert.match(err.message, /no such file or directory/);
}
}
return true;
});
Expand Down
Loading