Skip to content

test: add possible-flaky test-perf-hooks entry to SmartOS/illumos #57240

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 7,543 commits into from
Closed

test: add possible-flaky test-perf-hooks entry to SmartOS/illumos #57240

wants to merge 7,543 commits into from

Conversation

danmcd
Copy link
Contributor

@danmcd danmcd commented Feb 28, 2025

Per #57000 , the SmartOS/illumos failure seems due to jitter in one of the three terms of the test assertion. Until we determine if it's an underpowered VM, disparate sources of high-res-time, or some other factor, I'd like to put a "flaky" entry for test-perf-hooks.

StefanStojanovic and others added 30 commits January 30, 2025 10:58
@StefanStojanovic
test: set test-fs-cp as flaky
304bb9c 
PR-URL: nodejs#56799
Refs: nodejs#56794
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@aduh95
2025-01-30, Version 23.7.0 (Current)
c2c7f08 
Notable changes:

crypto:
  * update root certificates to NSS 3.107 (Node.js GitHub Bot) nodejs#56566
fs:
  * (SEMVER-MINOR) allow `exclude` option in globs to accept glob patterns (Daeyeon Jeong) nodejs#56489
module:
  * (SEMVER-MINOR) add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX (Marco Ippolito) nodejs#56610
sqlite:
  * (SEMVER-MINOR) support TypedArray and DataView in `StatementSync` (Alex Yang) nodejs#56385
src:
  * (SEMVER-MINOR) add --disable-sigusr1 to prevent signal i/o thread (Rafael Gonzaga) nodejs#56441
src,worker:
  * (SEMVER-MINOR) add isInternalWorker (Carlos Espa) nodejs#56469
test_runner:
  * (SEMVER-MINOR) add TestContext.prototype.waitFor() (Colin Ihrig) nodejs#56595
  * (SEMVER-MINOR) add t.assert.fileSnapshot() (Colin Ihrig) nodejs#56459
  * (SEMVER-MINOR) add assert.register() API (Colin Ihrig) nodejs#56434

PR-URL: nodejs#56800
@KhafraDev
events: getMaxListeners detects 0 listeners
752c0b8 
PR-URL: nodejs#56807
Refs: nodejs/undici#4032
Refs: nodejs@c1ccade
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@nektro
test: test-crypto-scrypt.js doesn't need internals
be9b614 
PR-URL: nodejs#56673
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@targos
tools: update sccache and sccache-action
1faf8c6 
Respectively to v0.9.1 and v0.0.7.

PR-URL: nodejs#56815
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos
deps: update V8 to 13.0.245.25
5edec0e 
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
build: reset embedder string to "-node.0"
1f654e6 
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
src: update NODE_MODULE_VERSION to 134
7917b67 
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 13.0.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
deps: always define V8_EXPORT_PRIVATE as no-op
1142f78 
dllexport introduces issues when compiling with MSVC.

PR-URL: nodejs#47251
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
@targos
deps: disable V8 concurrent sparkplug compilation
4f8fd56 
It introduces process hangs on some platforms because Node.js doesn't
tear down V8 correctly.
Disable it while we work on a solution.

Refs: nodejs#47297
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13902
PR-URL: nodejs#47450
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
@targos
deps: patch V8 to avoid duplicated zlib symbol
a4e0fce 
PR-URL: nodejs#54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@StefanStojanovic @targos
deps: patch V8 to support compilation with MSVC
d0361f0 
Co-Authored-By: Michaël Zasso <[email protected]>
PR-URL: nodejs#54536
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@StefanStojanovic @targos
deps: fix FP16 bitcasts.h
b3c1b63 
PR-URL: nodejs#53134
Refs: nodejs#52809
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
deps: always define V8_NODISCARD as no-op
e8f55f7 
It's causing compiler errors with some classes on Xcode 11
and the attribute should have no runtime effect.

PR-URL: nodejs#54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@StefanStojanovic @targos
deps: define V8_PRESERVE_MOST as no-op on Windows
d85d2f8 
It's causing linker errors with node.lib in node-gyp and potentially
breaks other 3rd party tools

PR-URL: nodejs#56238
Refs: nodejs#55784
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
@targos
deps: V8: cherry-pick 0c11feeeca4a
5d8011d 
Original commit message:

    [turboshaft][tsa] specify namespace for Block

    It is ambiguous otherwise. There is `v8::internal::Block` and
    `v8::internal::compiler::turboshaft::Block`.
    This change is also consistent with the other types used in the macro.

    Change-Id: Ica7e5a09de955d8f38756fe26ab5f7e93e7e16e2
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5878257
    Reviewed-by: Camillo Bruni <[email protected]>
    Commit-Queue: Michaël Zasso <[email protected]>
    Reviewed-by: Igor Sheludko <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#96278}

Refs: v8/v8@0c11fee
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@anonrig @targos
deps: V8: cherry-pick 0d5d6e71bbb0
99ffe35 
Original commit message:

    Remove `--js-promise-withresolvers` runtime flag

    Co-authored-by: Antoine du Hamel <[email protected]>
    Bug: 42204122
    Change-Id: I017a0d1ae0f8225513206ffb7806a4250be75d4c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5843972
    Reviewed-by: Igor Sheludko <[email protected]>
    Commit-Queue: Erik Corry <[email protected]>
    Reviewed-by: Shu-yu Guo <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#96215}

Refs: v8/v8@0d5d6e7
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@o- @targos
deps: V8: cherry-pick f915fa4c9f41
52d3944 
Original commit message:

    [osr] Ensure trying to osr does not skip loop interrupts

    Fixed: 374013413
    Change-Id: I52d7b4e165e0abd0bd517a81d2e8ef3f1f802bfb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5946288
    Commit-Queue: Darius Mercadier <[email protected]>
    Auto-Submit: Olivier Flückiger <[email protected]>
    Reviewed-by: Darius Mercadier <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#96708}

Refs: v8/v8@f915fa4
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
tools: update V8 gypfiles for 13.0
6f96526 
Refs: v8/v8@568f50d
Refs: v8/v8@6437539
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
build: remove support for ppc 32-bit
44b0e42 
V8 removed support for it.

Refs: v8/v8@6437539
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@etiennep-chromium @targos
src: update GetForegroundTaskRunner override
7e87520 
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5758110
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
build: link V8 with atomic library
89f661d 
Closes: nodejs/node-v8#290
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
test: disable fast API call count checks
6857dbc 
It seems that the arguments are no longer of type `FastOneByteString`.

PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@targos
tools: run macOS test workflow with Xcode 16.1
f05d287 
We have to remove `macos-13` as it doesn't have Xcode 16 available.

Refs: nodejs#56824
PR-URL: nodejs#56831
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
@anonrig
src: modernize cleanup queue to use c++20
581b444 
PR-URL: nodejs#56063
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@nodejs-github-bot
deps: update simdjson to 3.11.6
261624b 
PR-URL: nodejs#56250
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Lemire <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@ywave620
http: be more generational GC friendly
ba4587c 
Avoid any potential ref to Buffer in new generation
from old generation

PR-URL: nodejs#56767
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@cjihrig @jasnell
test_runner: automatically wait for subtests to finish
aa3523e 
This commit updates the test runner to automatically wait for
subtests to finish. This makes the experience more consistent
with suites and removes the need to await anything.

PR-URL: nodejs#56664
Fixes: nodejs#51292
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@cjihrig @jasnell
test_runner: remove promises returned by test()
9671826 
This commit updates the test() and suite() APIs to no longer
return a Promise.

Fixes: nodejs#51292
PR-URL: nodejs#56664
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@cjihrig @jasnell
test_runner: remove promises returned by t.test()
1a2eb15 
This commit updates the TestContext.prototype.test() API to no
longer return a Promise.

Fixes: nodejs#51292
PR-URL: nodejs#56664
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Feb 28, 2025
geeksilva97 and others added 2 commits February 28, 2025 14:26
@geeksilva97
doc: add @geeksilva97 to collaborators
ac5afbc 
Fixes: nodejs#57032
PR-URL: nodejs#57241
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
@guybedford
esm: support source phase imports for WebAssembly
db4dcc0 
PR-URL: nodejs#56919
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
@danmcd danmcd changed the title Add possible-flaky test-perf-hooks entry to solaris/SmartOS/illumos test: Add possible-flaky test-perf-hooks entry to solaris/SmartOS/illumos Feb 28, 2025
@lpinca
Copy link
Member

lpinca commented Feb 28, 2025

See https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines for the commit message.

@danmcd danmcd changed the title test: Add possible-flaky test-perf-hooks entry to solaris/SmartOS/illumos test: add possible-flaky test-perf-hooks entry to Solaris/SmartOS/illumos Feb 28, 2025
@danmcd danmcd changed the title test: add possible-flaky test-perf-hooks entry to Solaris/SmartOS/illumos test: add possible-flaky test-perf-hooks entry to SmartOS/illumos Feb 28, 2025
@codecov Codecov
Copy link

codecov bot commented Mar 1, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (v18.x-staging@51c8fbb). Learn more about missing BASE report.

Additional details and impacted files 
@@               Coverage Diff                @@
##             v18.x-staging   #57240   +/-   ##
================================================
  Coverage                 ?   90.24%           
================================================
  Files                    ?      630           
  Lines                    ?   184921           
  Branches                 ?    36193           
================================================
  Hits                     ?   166885           
  Misses                   ?    11058           
  Partials                 ?     6978
🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 1, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 1, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65494/

@aduh95
Copy link
Contributor

aduh95 commented Mar 1, 2025

Do we have any evidence that the test is flaky on main?

@aduh95
Copy link
Contributor

aduh95 commented Mar 2, 2025 •
edited
Loading

Here some pieces of evidence: https://ci.nodejs.org/job/node-test-commit-osx/63985/nodes=osx13-x64/testReport/junit/(root)/sequential/test_perf_hooks/, https://ci.nodejs.org/job/node-test-commit-osx/63994/nodes=osx13-x64/testReport/junit/(root)/sequential/test_perf_hooks/
However, that's not the SmartOS host, it's the macOS one

@danmcd
Copy link
Contributor Author

danmcd commented Mar 13, 2025

ping?

@lpinca lpinca added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 13, 2025
@lpinca
Copy link
Member

lpinca commented Mar 13, 2025 •
edited
Loading

There seem to be no recent failure on SmartOS (main branch). I'm ok with landing, but I don't see why if it does not fail.

@danmcd
Copy link
Contributor Author

danmcd commented Mar 13, 2025 •
edited by lpinca
Loading

There seem to be no recent failure on SmartOS (main branch). I'm ok with landing, but I don't see why if it does not fail.

I'm okay with not pushing this, as long as I understand if #57000 (v18 I think?) is actually okay now as well.

@lpinca
Copy link
Member

lpinca commented Mar 13, 2025 •
edited
Loading

I don't know but it might make more sense to change the base branch to v18.x-staging. In any case, the v18.x release line is close to reaching EOL.

@danmcd
Copy link
Contributor Author

danmcd commented Mar 13, 2025

I don't know but it might make more sense to change the base branch to v18.x-staging.

Cool. I'm still new to this end of things, so I really appreciate your patience.

@danmcd danmcd changed the base branch from main to v18.x-staging March 13, 2025 19:45
@danmcd
Copy link
Contributor Author

danmcd commented Mar 13, 2025

Uggh. CLosing this.

@danmcd danmcd closed this Mar 13, 2025
@danmcd
Copy link
Contributor Author

danmcd commented Mar 13, 2025

@lpinca ==> looks like this took care of it on v18:

a1f428a

@lpinca
Copy link
Member

lpinca commented Mar 13, 2025

Oh, ok. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

Assignees
No one assigned
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.