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.

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

test: run test on Node.js 23 #543

Merged
merged 6 commits into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
1 change: 1 addition & 0 deletions examples/search_github.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ urllib.request('https://api.github.com/legacy/user/search/location:china', {
timeout: 10000,
}).then(response => {
console.log(response);
console.log(response.data);
});

// var https = require('https');
Expand Down
2 changes: 1 addition & 1 deletion examples/timing.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function request(index) {
}
const res = await httpClient.request(url + '?index=' + index, {
// data: { wd: 'nodejs' },
dataType: 'json',
// dataType: 'json',
});
console.log('---------------------------');
console.log('No#%d: %s, content size: %d, requestUrls: %o, socket: %o, rt: %o',
Expand Down
5 changes: 2 additions & 3 deletions package.json
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
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
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
4 changes: 2 additions & 2 deletions test/options.compressed.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { createWriteStream, createReadStream } from 'node:fs';
import { describe, it, beforeAll, afterAll, beforeEach, afterEach } from 'vitest';
import urllib from '../src/index.js';
import { startServer } from './fixtures/server.js';
import { readableToString, createTempfile, nodeMajorVersion } from './utils.js';
import { readableToString, createTempfile } from './utils.js';

describe('options.compressed.test.ts', () => {
let close: any;
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('options.compressed.test.ts', () => {
// console.error(err);
assert.equal(err.name, 'UnzipError');
assert.equal(err.message, 'Decompression failed');
if (nodeMajorVersion() >= 20) {
if (process.version !== 'v18.19.0') {
assert.equal(err.code, 'ERR__ERROR_FORMAT_PADDING_1');
} else {
assert.equal(err.code, 'ERR_PADDING_1');
Expand Down
2 changes: 1 addition & 1 deletion test/options.timeout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('options.timeout.test.ts', () => {

it('should timeout 500ms throw error', async () => {
await assert.rejects(async () => {
const response = await urllib.request(`${_url}mock-bytes?timeout=1000`, {
const response = await urllib.request(`${_url}mock-bytes?timeout=1500`, {
timeout: [ 400, 500 ],
});
console.log(response.status, response.headers, response.data);
Expand Down
9 changes: 7 additions & 2 deletions test/options.writeStream.test.ts
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