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

chore: add automated CI testing with —no-intl node #3015

Merged
merged 7 commits into from
Mar 31, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
72 changes: 72 additions & 0 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,77 @@ jobs:
runs-on: ${{ matrix.runs-on }}
secrets: inherit

test-without-intl:
name: Test with Node.js ${{ matrix.version }} compiled --without-intl
strategy:
fail-fast: false
max-parallel: 0
matrix:
version: [20, 21]
runs-on: ubuntu-latest
timeout-minutes: 120
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false

# Setup node, install deps, and build undici prior to building icu-less node and testing
- name: Setup Node.js@${{ inputs.version }}
uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
with:
node-version: ${{ inputs.version }}

- name: Install dependencies
run: npm install

- name: Build undici
run: npm run build:node

- name: Determine latest release
id: release
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
with:
result-encoding: string
script: |
const req = await fetch('https://nodejs.org/download/release/index.json')
const releases = await req.json()

const latest = releases.find((r) => r.version.startsWith('v${{ matrix.version }}'))
return latest.version

- name: Download and extract source
run: curl https://nodejs.org/download/release/${{ steps.release.outputs.result }}/node-${{ steps.release.outputs.result }}.tar.xz | tar xfJ -

- name: Install ninja
run: sudo apt-get install ninja-build

- name: ccache
uses: hendrikmuhs/ccache-action@faf867a11c028c0b483fb2ae72b6fc8f7d842714 #v1.2.12
with:
key: node${{ matrix.version }}

- name: Build node
working-directory: ./node-${{ steps.release.outputs.result }}
run: |
export CC="ccache gcc"
export CXX="ccache g++"
./configure --without-intl --ninja --prefix=./final
make
make install
echo "$(pwd)/final/bin" >> $GITHUB_PATH

- name: Print version information
run: |
echo OS: $(node -p "os.version()")
echo Node.js: $(node --version)
echo npm: $(npm --version)
echo git: $(git --version)
echo icu config: $(node -e "console.log(process.config)" | grep icu)

- name: Run tests
run: npm run test:javascript:withoutintl

test-types:
name: Test TypeScript types
timeout-minutes: 15
Expand Down Expand Up @@ -97,6 +168,7 @@ jobs:
- dependency-review
- test
- test-types
- test-without-intl
- lint
runs-on: ubuntu-latest
permissions:
Expand Down
5 changes: 4 additions & 1 deletion lib/mock/pending-interceptors-formatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ const { Console } = require('node:console')
* Gets the output of `console.table(…)` as a string.
*/
module.exports = class PendingInterceptorsFormatter {
static PERSISTENT = process.versions.icu ? '✅' : 'Y '
static NOT_PERSISTENT = process.versions.icu ? '❌' : 'N '

mweberxyz marked this conversation as resolved.
Show resolved Hide resolved
constructor ({ disableColors } = {}) {
this.transform = new Transform({
transform (chunk, _enc, cb) {
Expand All @@ -29,7 +32,7 @@ module.exports = class PendingInterceptorsFormatter {
Origin: origin,
Path: path,
'Status code': statusCode,
Persistent: persist ? '✅' : '❌',
Persistent: persist ? PendingInterceptorsFormatter.PERSISTENT : PendingInterceptorsFormatter.NOT_PERSISTENT,
Invocations: timesInvoked,
Remaining: persist ? Infinity : times - timesInvoked
}))
Expand Down
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,13 @@
"lint:fix": "standard --fix | snazzy",
"test": "npm run test:javascript && cross-env NODE_V8_COVERAGE= npm run test:typescript",
"test:javascript": "node scripts/generate-pem && npm run test:unit && npm run test:node-fetch && npm run test:fetch && npm run test:cookies && npm run test:eventsource && npm run test:wpt && npm run test:websocket && npm run test:node-test && npm run test:jest",
"test:javascript:withoutintl": "node scripts/generate-pem && npm run test:unit && npm run test:node-fetch && npm run test:fetch:nobuild && npm run test:cookies && npm run test:eventsource:nobuild && npm run test:wpt:withoutintl && npm run test:node-test",
"test:cookies": "borp -p \"test/cookie/*.js\"",
"test:node-fetch": "borp -p \"test/node-fetch/**/*.js\"",
"test:eventsource": "npm run build:node && borp --expose-gc -p \"test/eventsource/*.js\"",
"test:fetch": "npm run build:node && borp --expose-gc -p \"test/fetch/*.js\" && borp -p \"test/webidl/*.js\" && borp -p \"test/busboy/*.js\"",
"test:eventsource": "npm run build:node && npm run test:eventsource:nobuild",
Copy link
Contributor

Choose a reason for hiding this comment

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

why is building bad?

Copy link
Contributor Author

@mweberxyz mweberxyz Mar 29, 2024

Choose a reason for hiding this comment

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

Build fails with icu-less node build:

CleanShot 2024-03-29 at 14 25 05@2x

But we shouldn't need to support building undici with icu-less node, only running it.

This is why the workflow installs regular node then builds undici prior to switching to the icu-less build for running tests.

I tried to minimize changes to package.json, so as to make people not need to think about keeping the test:javascript:withoutintl script up-to-date with test strategy changes.

"test:eventsource:nobuild": "borp --expose-gc -p \"test/eventsource/*.js\"",
"test:fetch": "npm run build:node && npm run test:fetch:nobuild",
"test:fetch:nobuild": "borp --expose-gc -p \"test/fetch/*.js\" && borp -p \"test/webidl/*.js\" && borp -p \"test/busboy/*.js\"",
"test:jest": "cross-env NODE_V8_COVERAGE= jest",
"test:unit": "borp --expose-gc -p \"test/*.js\"",
"test:node-test": "borp -p \"test/node-test/**/*.js\"",
Expand All @@ -81,6 +84,7 @@
"test:typescript": "tsd && tsc --skipLibCheck test/imports/undici-import.ts",
"test:websocket": "borp -p \"test/websocket/*.js\"",
"test:wpt": "node test/wpt/start-fetch.mjs && node test/wpt/start-FileAPI.mjs && node test/wpt/start-mimesniff.mjs && node test/wpt/start-xhr.mjs && node test/wpt/start-websockets.mjs && node test/wpt/start-cacheStorage.mjs && node test/wpt/start-eventsource.mjs",
"test:wpt:withoutintl": "node test/wpt/start-fetch.mjs && node test/wpt/start-mimesniff.mjs && node test/wpt/start-xhr.mjs && node test/wpt/start-cacheStorage.mjs && node test/wpt/start-eventsource.mjs",
"coverage": "npm run coverage:clean && cross-env NODE_V8_COVERAGE=./coverage/tmp npm run test:javascript && npm run coverage:report",
"coverage:ci": "npm run coverage:clean && cross-env NODE_V8_COVERAGE=./coverage/tmp npm run test:javascript && npm run coverage:report:ci",
"coverage:clean": "node ./scripts/clean-coverage.js",
Expand Down
2 changes: 1 addition & 1 deletion test/connect-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const assert = require('node:assert')
// Using describe instead of test to avoid the timeout
describe('prioritize socket errors over timeouts', async () => {
const t = tspl({ ...assert, after: () => {} }, { plan: 1 })
const client = new Pool('http://foobar.bar:1234', { connectTimeout: 1 })
const client = new Pool('http://foorbar.invalid:1234', { connectTimeout: 1 })

client.request({ method: 'GET', path: '/foobar' })
.then(() => t.fail())
Expand Down
Loading
Loading