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

Node.js 18 CITGM Results #894

Closed
3 of 21 tasks
BethGriggs opened this issue Mar 17, 2022 · 17 comments
Closed
3 of 21 tasks

Node.js 18 CITGM Results #894

BethGriggs opened this issue Mar 17, 2022 · 17 comments

Comments

@BethGriggs
Copy link
Member

BethGriggs commented Mar 17, 2022

Node.js 18 CITGM

Multiple platforms:

@dominykas
Copy link
Member

Anything concrete I could help with in here?

@SimenB
Copy link
Member

SimenB commented Mar 31, 2022

For Jest, the failures.test was fixed in jestjs/jest@8c52045. The coverage failure comes from source-map:

Error: You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer
    at readWasm (/Users/simen/repos/jest/node_modules/v8-to-istanbul/node_modules/source-map/lib/read-wasm.js:8:13)
    at wasm (/Users/simen/repos/jest/node_modules/v8-to-istanbul/node_modules/source-map/lib/wasm.js:25:16)
    at /Users/simen/repos/jest/node_modules/v8-to-istanbul/node_modules/source-map/lib/source-map-consumer.js:264:14
    at V8ToIstanbul.load (/Users/simen/repos/jest/node_modules/v8-to-istanbul/lib/v8-to-istanbul.js:65:26)
    at /Users/simen/repos/jest/packages/jest-reporters/build/CoverageReporter.js:614:11
    at async Promise.all (index 0)
    at CoverageReporter._getCoverageResult (/Users/simen/repos/jest/packages/jest-reporters/build/CoverageReporter.js:572:35)
    at CoverageReporter.onRunComplete (/Users/simen/repos/jest/packages/jest-reporters/build/CoverageReporter.js:220:34)
    at ReporterDispatcher.onRunComplete (/Users/simen/repos/jest/packages/jest-core/build/ReporterDispatcher.js:73:9)
    at TestScheduler.scheduleTests (/Users/simen/repos/jest/packages/jest-core/build/TestScheduler.js:353:5)

or possibly v8-to-istanbul. /cc @bcoe (to reproduce, run yarn && node scripts/build && cd e2e/coverage-provider-v8/no-sourcemap && node ../../../packages/jest/bin/jest.js --coverage-provider v8 --coverage)

@ljharb
Copy link
Member

ljharb commented Mar 31, 2022

For tape, i have no idea how to reproduce that error locally. It shouldn't block node 18 because it's part of dev scripts, but if there's a way i could get a docker image, or ssh into something, and repro it, that would help resolve it.

@Trott
Copy link
Member

Trott commented Apr 1, 2022

For tape, i have no idea how to reproduce that error locally. It shouldn't block node 18 because it's part of dev scripts, but if there's a way i could get a docker image, or ssh into something, and repro it, that would help resolve it.

Since these are failing in Jenkins, you can request access to a host by opening an issue in the build repo, similar to nodejs/build#2909. The process is documented at https://github.com/nodejs/build/blob/09308290d8401e15fcd3f7f5c6610a6ea13df75a/GOVERNANCE.md#temporary-access.

@ljharb
Copy link
Member

ljharb commented Apr 1, 2022

Thanks, I filed nodejs/build#2913

@SimenB
Copy link
Member

SimenB commented Apr 6, 2022

FWIW, the remaining test failure in Jest also happens in the test suite of v8-to-istanbul. I'm somewhat surprised c8 still works in the node repo itself - their test suite is also very broken using node 18.

image

EDIT: you probably don't use source maps actually, so doesn't trigger the code path?

@SimenB
Copy link
Member

SimenB commented Apr 6, 2022

@BethGriggs the issue is fetch being a global, it breaks source-map (with more than 160M downloads a week, although the broken v7 has less since it's async only). See https://www.runpkg.com/[email protected]/lib/read-wasm.js#1

Same thing happens using node 17 and --experimental-fetch.

npm init -y && npm install --save source-map

// file.mjs
import {SourceMapConsumer} from 'source-map'

await new SourceMapConsumer({version: '3', sources: [], mappings: []})
$ node file.mjs
$ echo $?
0
$ node --experimental-fetch file.mj
(node:21808) ExperimentalWarning: Fetch is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
/Users/simen/repos/source-map-boom/node_modules/source-map/lib/read-wasm.js:8
      throw new Error("You must provide the URL of lib/mappings.wasm by calling " +
            ^

Error: You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer
    at readWasm (/Users/simen/repos/source-map-boom/node_modules/source-map/lib/read-wasm.js:8:13)
    at wasm (/Users/simen/repos/source-map-boom/node_modules/source-map/lib/wasm.js:25:16)
    at /Users/simen/repos/source-map-boom/node_modules/source-map/lib/source-map-consumer.js:264:14
    at async file:///Users/simen/repos/source-map-boom/file.mjs:3:1

Node.js v17.8.0
$ echo $?
1

@richardlau
Copy link
Member

I'm confused with this one as we're supposed to already be skipping on AIX and Linux s390x:

citgm/lib/lookup.json

Lines 61 to 65 in 4034fb7

"blake2b-wasm": {
"maintainers": ["mafintosh", "addaleax"],
"prefix": "v",
"skip": ["win32", "aix", "s390"]
},

@richardlau
Copy link
Member

I'm confused with this one as we're supposed to already be skipping on AIX and Linux s390x:

citgm/lib/lookup.json

Lines 61 to 65 in 4034fb7

"blake2b-wasm": {
"maintainers": ["mafintosh", "addaleax"],
"prefix": "v",
"skip": ["win32", "aix", "s390"]
},

oh we haven't published a new version with #887 🤦 .

@richardlau
Copy link
Member

oh we haven't published a new version with #887 🤦 .

We have now (v8.0.6) 🙂.

@targos
Copy link
Member

targos commented Apr 6, 2022

oh we haven't published a new version with #887 🤦 .

We have now (v8.0.6) 🙂.

Please push the release commit.

@richardlau
Copy link
Member

Please push the release commit.

🤦. Done. (I'd already pushed the tag but forgot to push the main branch.)

@SimenB
Copy link
Member

SimenB commented Apr 7, 2022

I opened up nodejs/node#42638 as I think the breakage of source-map deserves some attention.

@SimenB
Copy link
Member

SimenB commented Apr 7, 2022

@BethGriggs Jest HEAD should be fixed by jestjs/jest@c6902a0. I'll be releasing a new major sometime this month which includes it, but for green CITGM maybe get head instead of latest?

@SimenB
Copy link
Member

SimenB commented Apr 20, 2022

Above was wrong due to nodejs/node#42792. However, jestjs/jest@78d4088 is green on Node 18 for Jest (modulus react-native example, but CITGM skips the examples).

I had it in my head Node 18 would come next week, but I'll try to make a stable release this weekend. HEAD should be green, tho

@targos
Copy link
Member

targos commented Dec 5, 2023

@targos targos pinned this issue Dec 6, 2023
@RafaelGSS
Copy link
Member

Closing in favour of #1060

@RafaelGSS RafaelGSS unpinned this issue May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants