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

Investigate flaky test/addons/new-isolate-addon #46517

Closed
RaisinTen opened this issue Feb 6, 2023 · 4 comments
Closed

Investigate flaky test/addons/new-isolate-addon #46517

RaisinTen opened this issue Feb 6, 2023 · 4 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. windows Issues and PRs related to the Windows platform.

Comments

@RaisinTen
Copy link
Contributor

RaisinTen commented Feb 6, 2023

Test

test/addons/new-isolate-addon

Platform

Windows

Console output

Output of a test failure from test/addons/new-isolate-addon/test.js:

not ok 26 addons/new-isolate-addon/test
  ---
  duration_ms: 0.316
  severity: fail
  exitcode: 1
  stack: |-
    node:assert:124
      throw new AssertionError(obj);
      ^
    
    AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
    + actual - expected
    
      Uint8Array(1) [
    +   0
    -   66
      ]
        at Object.<anonymous> (c:\workspace\node-test-binary-windows-native-suites\node\test\addons\new-isolate-addon\test.js:8:8)
        at Module._compile (node:internal/modules/cjs/loader:1246:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1300:10)
        at Module.load (node:internal/modules/cjs/loader:1103:32)
        at Module._load (node:internal/modules/cjs/loader:942:12)
        at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
        at node:internal/main/run_main_module:23:47 {
      generatedMessage: true,
      code: 'ERR_ASSERTION',
      actual: Uint8Array(1) [ 0 ],
      expected: Uint8Array(1) [ 6...

Output of a test failure from test/addons/new-isolate-addon/test-nonodesnapshot.js:

not ok 57 addons/new-isolate-addon/test-nonodesnapshot
  ---
  duration_ms: 0.333
  severity: fail
  exitcode: 1
  stack: |-
    node:assert:124
      throw new AssertionError(obj);
      ^
    
    AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
    + actual - expected
    
      Uint8Array(1) [
    +   0
    -   66
      ]
        at Object.<anonymous> (c:\workspace\node-test-binary-windows-native-suites\node\test\addons\new-isolate-addon\test.js:8:8)
        at Module._compile (node:internal/modules/cjs/loader:1246:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1300:10)
        at Module.load (node:internal/modules/cjs/loader:1103:32)
        at Module._load (node:internal/modules/cjs/loader:942:12)
        at Module.require (node:internal/modules/cjs/loader:1127:19)
        at require (node:internal/modules/helpers:112:18)
        at Object.<anonymous> (c:\workspace\node-test-binary-windows-native-suites\node\test\addons\new-isolate-addon\test-nonodesnap...

Build links

Additional information

These tests were added in #45885, so cc @addaleax.

@RaisinTen RaisinTen added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Feb 6, 2023
@github-actions github-actions bot added the windows Issues and PRs related to the Windows platform. label Feb 6, 2023
@addaleax
Copy link
Member

addaleax commented Feb 6, 2023

I couldn’t reproduce this locally in repeated runs – it seems this is happening only on Windows? That’s surprising, given that there is no OS-specific code involved here.

@RaisinTen
Copy link
Contributor Author

RaisinTen commented Feb 6, 2023

Yea, this only affects Windows ARM and I have no idea why it happens. Does it make sense to skip this test only on the Windows ARM platform as a mitigation step?

@richardlau
Copy link
Member

According to nodejs/reliability#497 (comment) this is only failing on Windows arm64? That's not a platform we test on by default -- it's being worked on in nodejs/build#3046 (see also nodejs/build#3046 (comment) that work is currently being done on the machines).

I'm a little surprised this is being flagged for all of those PRs. I checked a few of the failing CI's from #46495, for example, and none of them failed with this test. Most of the current Windows CI failures are nodejs/build#3174, which is a left over Node.js process preventing the git checkout from being cleaned. Maybe there's a bug in the ncu-ci walk pr logic?

I suspect the tests being discussed in this issue either doesn't work on Windows on ARM64 (would not have been tested in the original PR) or it's somehow related to the current work in nodejs/build#3046. cc @nodejs/platform-windows-arm

@shnooshnoo
Copy link

according to reliability reports this test has only been failing for a brief period of time back in feb this year. All the failures were on nearform arm64 machines, which are now deprecated. I believe flakiness of this test has been related to the ongoing work around windows arm64 platform and is no longer the case. If there are no recent signs of this test being flaky I believe this issue can be closed for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

4 participants