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

test-structuredclone-jstransferable is flaky #50260

Closed
anonrig opened this issue Oct 18, 2023 · 2 comments · Fixed by #54115
Closed

test-structuredclone-jstransferable is flaky #50260

anonrig opened this issue Oct 18, 2023 · 2 comments · Fixed by #54115
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

@anonrig
Copy link
Member

anonrig commented Oct 18, 2023

Test

test-structuredclone-jstransferable

Platform

Windows

Console output

not ok 922 pummel/test-structuredclone-jstransferable
  ---
  duration_ms: 2662.02700
  severity: fail
  exitcode: 134
  stack: |-
    Administrator: 5680[4512]: C:\workspace\node-compile-windows\node\src\histogram.cc:29: Assertion `(0) == (hdr_init(options.lowest, options.highest, options.figures, &histogram))'

Build links

Additional information

No response

@anonrig anonrig added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Oct 18, 2023
@github-actions github-actions bot added the windows Issues and PRs related to the Windows platform. label Oct 18, 2023
@H4ad
Copy link
Member

H4ad commented Oct 18, 2023

@legendecas Maybe this one is interesting to you.

14:25:05  failed 2 out of 10
14:25:05 not ok 922 pummel/test-structuredclone-jstransferable
14:25:05   ---
14:25:05   duration_ms: 2662.02700
14:25:05   severity: fail
14:25:05   exitcode: 134
14:25:05   stack: |-
14:25:05     Administrator: 5680[4512]: C:\workspace\node-compile-windows\node\src\histogram.cc:29: Assertion `(0) == (hdr_init(options.lowest, options.highest, options.figures, &histogram))' failed.

Failing at:

node/src/histogram.cc

Lines 24 to 31 in e0bbe56

Histogram::Histogram(const Options& options) {
hdr_histogram* histogram;
CHECK_EQ(0, hdr_init(options.lowest,
options.highest,
options.figures,
&histogram));
histogram_.reset(histogram);
}

nodejs-github-bot pushed a commit that referenced this issue Oct 19, 2023
PR-URL: #50261
Refs: #50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
targos pushed a commit that referenced this issue Oct 23, 2023
PR-URL: #50261
Refs: #50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
PR-URL: nodejs#50261
Refs: nodejs#50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
targos pushed a commit that referenced this issue Nov 11, 2023
PR-URL: #50261
Refs: #50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
targos pushed a commit that referenced this issue Nov 27, 2023
PR-URL: #50261
Refs: #50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
mertcanaltin pushed a commit to mertcanaltin/node that referenced this issue Dec 19, 2023
PR-URL: nodejs#50261
Refs: nodejs#50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

meta: move Trott to TSC regular member

At the current time, I'm not able to give Node.js the time and attention
it needs and deserves from a voting TSC member. I'm proud of the work
and efforts I've made as a TSC voting member, and I want to leave before
I'm less happy with my efforts. Thanks for all the trust and good will
over the years.

PR-URL: nodejs#50297
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Richard Lau <rlau@redhat.com>

node-api: refactor napi set property function for improved performance

fix: lint

fix: lint

refactor: Improve performance by using internalized property keys

refactor: Update node_api_create_property_key_utf16 signature and remove napi_set_property_utf16

lint

lint

fix: Resolve compilation error in node_api_create_property_key_utf16

fix: Resolve type conversion error in node_api_create_property_key_utf16

refactor: Simplify node_api_create_property_key_utf16 implementation

lint

add node_api_create_property_key_utf16 property name

added doc for node_api_create_property_key_utf16

lint

lint

update napi doc for node_api_create_property_key_utf16

update: code snipet

test: added test for node_api_create_property_key_utf16

lint

lint

lint

call node_api_create_property_key_utf16

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

added TestPropertyKeyUtf16 napi_property_descriptor

lint doc

lint cpp

minor updates to PR nodejs#50282

feat: removed not used function for test

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>
mertcanaltin pushed a commit to mertcanaltin/node that referenced this issue Dec 19, 2023
PR-URL: nodejs#50261
Refs: nodejs#50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

meta: move Trott to TSC regular member

At the current time, I'm not able to give Node.js the time and attention
it needs and deserves from a voting TSC member. I'm proud of the work
and efforts I've made as a TSC voting member, and I want to leave before
I'm less happy with my efforts. Thanks for all the trust and good will
over the years.

PR-URL: nodejs#50297
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Richard Lau <rlau@redhat.com>

node-api: refactor napi set property function for improved performance

fix: lint

fix: lint

refactor: Improve performance by using internalized property keys

refactor: Update node_api_create_property_key_utf16 signature and remove napi_set_property_utf16

lint

lint

fix: Resolve compilation error in node_api_create_property_key_utf16

fix: Resolve type conversion error in node_api_create_property_key_utf16

refactor: Simplify node_api_create_property_key_utf16 implementation

lint

add node_api_create_property_key_utf16 property name

added doc for node_api_create_property_key_utf16

lint

lint

update napi doc for node_api_create_property_key_utf16

update: code snipet

test: added test for node_api_create_property_key_utf16

lint

lint

lint

call node_api_create_property_key_utf16

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

added TestPropertyKeyUtf16 napi_property_descriptor

lint doc

lint cpp

minor updates to PR nodejs#50282

feat: removed not used function for test

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>
mertcanaltin pushed a commit to mertcanaltin/node that referenced this issue Dec 19, 2023
PR-URL: nodejs#50261
Refs: nodejs#50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

meta: move Trott to TSC regular member

At the current time, I'm not able to give Node.js the time and attention
it needs and deserves from a voting TSC member. I'm proud of the work
and efforts I've made as a TSC voting member, and I want to leave before
I'm less happy with my efforts. Thanks for all the trust and good will
over the years.

PR-URL: nodejs#50297
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Richard Lau <rlau@redhat.com>

node-api: refactor napi set property function for improved performance

fix: lint

fix: lint

refactor: Improve performance by using internalized property keys

refactor: Update node_api_create_property_key_utf16 signature and remove napi_set_property_utf16

lint

lint

fix: Resolve compilation error in node_api_create_property_key_utf16

fix: Resolve type conversion error in node_api_create_property_key_utf16

refactor: Simplify node_api_create_property_key_utf16 implementation

lint

add node_api_create_property_key_utf16 property name

added doc for node_api_create_property_key_utf16

lint

lint

update napi doc for node_api_create_property_key_utf16

update: code snipet

test: added test for node_api_create_property_key_utf16

lint

lint

lint

call node_api_create_property_key_utf16

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

added TestPropertyKeyUtf16 napi_property_descriptor

lint doc

lint cpp

minor updates to PR nodejs#50282

feat: removed not used function for test

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
PR-URL: nodejs/node#50261
Refs: nodejs/node#50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
PR-URL: nodejs/node#50261
Refs: nodejs/node#50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
@StefanStojanovic
Copy link
Contributor

Hey all, I haven't seen this test flaking for some time, so I've run stress test (1000 runs) on it and there were no failures. Is this proof enough that we can remove the flaky mark from it (I'll open a PR after getting a confirmation here), or should something else be done to validate that it is no longer flaky (e.g. more runs in the stress test, or something else)?

StefanStojanovic added a commit to JaneaSystems/node that referenced this issue Jul 30, 2024
nodejs-github-bot pushed a commit that referenced this issue Aug 6, 2024
Fixes: #50260
PR-URL: #54115
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit that referenced this issue Aug 14, 2024
Fixes: #50260
PR-URL: #54115
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit that referenced this issue Sep 21, 2024
Fixes: #50260
PR-URL: #54115
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit that referenced this issue Oct 2, 2024
Fixes: #50260
PR-URL: #54115
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
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

Successfully merging a pull request may close this issue.

3 participants