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 15 failures #836

Closed
2 of 6 tasks
BethGriggs opened this issue Oct 19, 2020 · 14 comments
Closed
2 of 6 tasks

Node.js 15 failures #836

BethGriggs opened this issue Oct 19, 2020 · 14 comments

Comments

@BethGriggs
Copy link
Member

BethGriggs commented Oct 19, 2020

  • Node Version: v15.0.0
  • CitGM Version: v7.1.9

All platforms:

macOS:

@ljharb
Copy link
Member

ljharb commented Oct 19, 2020

What are the failures with tape? Which core modules is resolve failing to detect?

@BethGriggs
Copy link
Member Author

tape@5.0.1 from https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2523/nodes=ubuntu1804-64/testReport/(root)/citgm/tape_v5_0_1/:

        not ok 3 - should be equivalent
           ---
           found: |
             $TEST/async-await/sync-error.js:7
                 throw new Error('oopsie');
                 ^
             Error: oopsie
                 at Test.myTest ($TEST/async-await/sync-error.js:$LINE:$COL)
                 at Test.bound [as _cb] ($TAPE/lib/test.js:$LINE:$COL)
                 at Test.run ($TAPE/lib/test.js:$LINE:$COL)
                 at Test.bound [as run] ($TAPE/lib/test.js:$LINE:$COL)
                 at processImmediate (node:internal/timers:$LINE:$COL)
           wanted: |
             $TEST/async-await/sync-error.js:7
                 throw new Error('oopsie');
                 ^
             Error: oopsie
                 at Test.myTest ($TEST/async-await/sync-error.js:$LINE:$COL)
                 at Test.bound [as _cb] ($TAPE/lib/test.js:$LINE:$COL)
                 at Test.run ($TAPE/lib/test.js:$LINE:$COL)
                 at Test.bound [as run] ($TAPE/lib/test.js:$LINE:$COL)
           at:
             line: 189
             column: 11
             file: test/async-await.js
           stack: |
             test/async-await.js:189:11
             ChildProcess.<anonymous> (test/common.js:90:9)
             ChildProcess.emit (node:events:327:20)
             Process.ChildProcess._handle.onexit (node:internal/child_process:277:12)
           source: |
             t.same(stripFullStack(stderr), [
           ...
         1..3
         # failed 1 of 3 tests
     not ok 6 - sync-error # time=119.536ms
       ---
       at:
         line: 170
         column: 5
         file: test/async-await.js
       results:
         plan:
           start: 1
           end: 3
         count: 3
         pass: 2
         ok: false
         fail: 1
         time: 119.536
       source: |
         tap.test('sync-error', function (t) {
       ...
     # Subtest: async-error
         ok 1 - should be equivalent
         ok 2 - should be equivalent
         ok 3 - should be equivalent
         1..3
     ok 7 - async-error # time=125.421ms
     1..7
     # failed 1 of 7 tests
     # time=998.3ms
 not ok 4 - test/async-await.js # time=1120.491ms
   ---
   timeout: 30000
   file: test/async-await.js
   results:
     ok: false
     count: 7
     pass: 6
     fail: 1
     plan:
       start: 1
       end: 7
   exitCode: 1
   command: /home/iojs/build/workspace/citgm-smoker/nodes/ubuntu1804-64/smoker/bin/node
   arguments:
     - test/async-await.js
   ...

resolve@1.17.0 from https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2523/nodes=ubuntu1804-64/testReport/(root)/citgm/resolve_v1_17_0/

not ok 190 assert/strict is a core module
...
not ok 210 dns/promises is a core module
...
not ok 250 stream/promises is a core module
...
not ok 258 timers/promises is a core module

@ljharb
Copy link
Member

ljharb commented Oct 19, 2020

For tape, looks like node's error stacks now include a at processImmediate (node:internal/timers:$LINE:$COL) line where they didn't before - is that intentional and/or necessary?

@ljharb
Copy link
Member

ljharb commented Oct 19, 2020

For resolve, v1.18.0 is now published; I believe that will pass the tests until new core modules are merged.

@BethGriggs
Copy link
Member Author

For tape, looks like node's error stacks now include a at processImmediate (node:internal/timers:$LINE:$COL) line where they didn't before - is that intentional and/or necessary?

I have just found the PR that introduced the change, see nodejs/node#35498 (comment)

@ljharb
Copy link
Member

ljharb commented Oct 20, 2020

It's not the node: part that i'm noticing tho; it's the extra line entirely.

@richardlau
Copy link
Member

It's not the node: part that i'm noticing tho; it's the extra line entirely.

@ljharb It's the following line in tape's test that is affected by the node: part and no longer filtering out the line:
https://github.com/substack/tape/blob/898302b3e914c93b407088d36a224355b898bb0c/test/async-await.js#L184

> /\(internal\/timers.js:/.test('at processImmediate (internal/timers.js:462:21)')
true
> /\(internal\/timers.js:/.test('at processImmediate (node:internal/timers:462:21)')
false
>

@SimenB
Copy link
Member

SimenB commented Oct 20, 2020

I'm assuming it's the same thing that made Jest fail: tapjs/stack-utils#54 (that probably wouldn't affect tape, but possibly tap?)

@richardlau
Copy link
Member

I'm assuming it's the same thing that made Jest fail: tapjs/stack-utils#54 (that probably wouldn't affect tape, but possibly tap?)

Yes, looks like it. I've raised tapjs/stack-utils#55.

@ljharb
Copy link
Member

ljharb commented Oct 20, 2020

@richardlau thanks for clarifying, it’s now fixed in the repo but not yet published.

@SimenB
Copy link
Member

SimenB commented Oct 20, 2020

Thanks @richardlau!

@lpinca
Copy link
Member

lpinca commented Oct 21, 2020

The problem with ws is that npm no longer installs the utf-8-validate dev dependency. Probably because it is also listed as optional peer dependency.

I'm not sure how to fix this.

Refs: npm/cli#1998

@lpinca
Copy link
Member

lpinca commented Oct 28, 2020

ws issue is fixed in npm@7.0.4.

@targos
Copy link
Member

targos commented Mar 15, 2021

Closing in favor of #852

@targos targos closed this as completed Mar 15, 2021
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

6 participants