test: fix test-worker-broadcastchannel-wpt #36353
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Good catch. I got distracted and forgot that CI hadn't been run after I had rebased and updated this. +1 for fast tracking assuming it goes through. I'd also be +1 on temporarily marking this flaky or commenting out this part of the test until the ordering can be revisited
Yeah, if this doesn't pass, I'll modify it to skip on Windows and leave a TODO comment. Since the feature is experimental, that should be fine. |
Trott
force-pushed
the
fix-broadcast-test
branch
from
d3cbd20
to
4e147fc
Compare
|
Well, that fixes the test case that fails currently, but something else fails later on. Put in code to skip the thing later on and trying CI again. In the meantime, taking out of draft mode and requesting fast-track. We already have one fast-track approval from @jasnell. Another one, anyone? |
This comment has been minimized.
This comment has been minimized.
Optimistic possible fix for a broken test on Windows. PR-URL: nodejs#36353 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Trott
force-pushed
the
fix-broadcast-test
branch
from
4e147fc
to
c91e608
Compare
|
Landed in c91e608 |
Optimistic possible fix for a broken test on Windows. PR-URL: #36353 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Optimistic possible fix for a broken test on Windows. PR-URL: nodejs#36353 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Optimistic possible fix for a broken test on Windows.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes