nodejs: fix flaky test #358227
nodejs: fix flaky test #358227
Conversation
|
Maybe we should get the timeout increased upstream, since they think the issue is fixed? |
|
Sure, I'll try. Maybe I can even drop the broken pattern of "do something + assume it is done after X ms". |
|
I meant just reporting it upstream as not quite fixed would be a good step :) But thank you if you do take on the task of patching it! |
|
If it's caused by nodejs/node#54918 (and my guess is that it is), increasing the timeout won't help. |
It is not, I think. My fix nodejs/node#55959 seems to work. |
|
Can you update this PR to cherry-pick nodejs/node@b6fe731 instead of skipping the test? |
FliegendeWurst
force-pushed
the
nodejs-flaky-test
branch
from
a385fdb
to
4a3d094
Compare
FliegendeWurst
force-pushed
the
nodejs-flaky-test
branch
2 times, most recently
from
7915372
to
e9522c2
Compare
pkgs/development/web/nodejs/v20.nix
Outdated
| @@ -44,5 +44,10 @@ buildNodejs { | |||
| stripLen = 1; | |||
| hash = "sha256-6y3aEqxNC4iTQEv1oewodJrhOHxjp5xZMq1P1QL94Rg="; | |||
| }) | |||
| # fixes test failure, remove once included in release | |||
| (fetchpatch { | |||
There was a problem hiding this comment.
| (fetchpatch { | |
| (fetchpatch2 { |
There was a problem hiding this comment.
No need. In fact I was told to use fetchpatch instead of fetchpatch2 previously, since the latter requires adding ?full_index=1
pkgs/development/web/nodejs/v20.nix
Outdated
| @@ -44,5 +44,10 @@ buildNodejs { | |||
| stripLen = 1; | |||
| hash = "sha256-6y3aEqxNC4iTQEv1oewodJrhOHxjp5xZMq1P1QL94Rg="; | |||
| }) | |||
| # fixes test failure, remove once included in release | |||
| (fetchpatch { | |||
| url = "https://github.com/nodejs/node/commit/b6fe731c55eb4cb9d14042a23e5002ed39b7c8b7.patch"; | |||
There was a problem hiding this comment.
| url = "https://github.com/nodejs/node/commit/b6fe731c55eb4cb9d14042a23e5002ed39b7c8b7.patch"; | |
| url = "https://github.com/nodejs/node/commit/b6fe731c55eb4cb9d14042a23e5002ed39b7c8b7.patch?full_index=1"; |
We need the ?full_index=1 so the patch is stable (otherwise it might change as GitHub updates the length of the index as the upstream repo grows)
There was a problem hiding this comment.
Not with fetchpatch (rather than fetchpatch2), as it turns out. You get to choose between stripping index lines properly and handling renames…
(But it probably makes sense to keep to fetchpatch2 in these files since they’re already using it?)
FliegendeWurst
force-pushed
the
nodejs-flaky-test
branch
from
e9522c2
to
f634fa2
Compare
See:
Things done