Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
pront
left a comment
There was a problem hiding this comment.
This is good improvement that will reduce a lot of the flakey runs. One followup item worth looking into is if we can refactor tests and remove the need for timeouts.
| let universe = Universe::with_accelerated_time(); | ||
| let back_pressure_actor = BackPressureActor; | ||
| let back_pressure_actor = BackPressureActor { | ||
| queue_capacity: QueueCapacity::Bounded(1), |
There was a problem hiding this comment.
Isn't this test here to test that we keep track of backpressure and so increasing the queue capacity and asserting that backpressure = 0 defeats its entire purpose?
There was a problem hiding this comment.
I re-read the test and you're correct, reverting this change.
Description
Adds retries to the merge queue test runs and fixes the most common flaky tests from the last two months of CI.
Retries. Unit tests,
make test-alland coverage now use the nextestciprofile:make test-allused to stop at the first one. This means if multiple tests fail in make test-all all the failures will be reported instead of just oneFlaky test fixes. All test-side; mostly tests not waiting long enough or waiting on the wrong thing.
test_ingest_v1_happy_pathtest_ingester_close_shardstest_indexer_exceeding_max_num_partitionsHow was this PR tested?
make test-allpasses, and clippy and fmt are clean.Each fixed test was run repeatedly on an overloaded machine with retries off, before and after the fix:
test_ingester_close_shardstest_indexer_exceeding_max_num_partitionstest_retiring_indexer_receives_empty_plancrash