Mark test_wasm_worker_lock_async_acquire flaky. #25271
Conversation
.circleci/config.yml
Outdated
| @@ -315,6 +315,7 @@ commands: | |||
| environment: | |||
| EMTEST_DETECT_TEMPFILE_LEAKS: "0" | |||
| EMTEST_HEADLESS: "1" | |||
| EMTEST_RETRY_FLAKY: "3" | |||
There was a problem hiding this comment.
When I added the @flaky annotation in this PR, CircleCI would just complain about the flaky test that it is failing.
The CircleCI bots don't seem to be restarting flaky tests in browser suite. Not sure if that happened after PR #25246 , or some other refactor.
So I added the EMTEST_RETRY_FLAKY directives in this PR so that CircleCI will retry the flaky browser tests.
There was a problem hiding this comment.
I see. I haven't followed all the recent work on flakiness, so sgtm but deferring to @brendandahl @sbc100
.circleci/config.yml
Outdated
| @@ -995,6 +1002,8 @@ jobs: | |||
| " | |||
| test-browser-firefox-wasm64: | |||
| executor: focal | |||
| environment: | |||
| EMTEST_RETRY_FLAKY: "3" | |||
There was a problem hiding this comment.
Can you revert the changes to this file, and we can discuss this is a seperate PR?
There is an existing mechanism in .circleci/config.yml in the form of set-retry-flaky-tests. TBH I can't remember why I wanted it only for PRs.. but any refactoring can/should be separate from marking test_wasm_worker_lock_async_acquire itself as flaky.
lgtm without the changes in this file.
There was a problem hiding this comment.
Can you revert the changes to this file, and we can discuss this is a seperate PR?
That would mean landing this in a way that breaks CircleCI. I.e. before I did this change, all the CI bots started showing red, as they started failing on this test.
There was a problem hiding this comment.
Reverted the EMTEST_RETRY_FLAKY now, but it didn't pass in CircleCI, as adding the @flaky stopped the flaky test from being re-run.
There was a problem hiding this comment.
Yes, sorry, I guess I should have said that we need some further discussion/thought about how/when/where to enable EMTEST_RETRY_FLAKY more widely.
lgtm to this change now even if the CI is still red, or if you like we can have the above discussion here before landing. either way sgtm.
juj
force-pushed
the
test_wasm_worker_lock_async_acquire_is_flaky
branch
from
38d9c00 to
fe18217
Compare
…_async_acquire_is_flaky
#25270