test: attempt to remove flakiness from convergence tests#7370
test: attempt to remove flakiness from convergence tests#7370brice-stacks wants to merge 2 commits into
Conversation
brice-stacks
commented
Jun 30, 2026
brice-stacks
commented
- Increase timeout to 15 minutes
- Reduce poll step to 10ms
- Increase neighbor count for a denser graph (converging in fewer gossip rounds
- Increase timeout to 15 minutes - Reduce poll step to 10ms - Increase neighbor count for a denser graph (converging in fewer gossip rounds
Coverage Report for CI Build 28524034873Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.4%) to 86.088%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions9072 previously-covered lines in 132 files lost coverage.
Coverage Stats
💛 - Coveralls |
federico-stacks
left a comment
federico-stacks
left a comment
There was a problem hiding this comment.
LGTM. I just added a small check.
Also, heads-up: the license job in CI seems to be stuck.
| let mut peer_configs = vec![]; | ||
| let peer_count: usize = 15; | ||
| let neighbor_count: usize = 3; | ||
| let neighbor_count: usize = 8; |
There was a problem hiding this comment.
Sanity check: I just noticed these three tests still use the original neighbor_count = 3. Was that intentional?
test_walk_ring_15_org_biasedtest_walk_line_15_org_biasedtest_walk_star_15_org_biased
There was a problem hiding this comment.
That was deliberate. I left the org-biased tests at 3 since that neighbor_count / 2 is used to set the soft_max_neighbors_per_org, so it has even more effect on those tests. But looking again, I just realized that those tests are the ones that are the most flaky, so maybe it is worth bumping that number up as well.
The main problem is that I don't have a good intuition for what is a reasonable number to use for these to get reasonable test coverage AND reasonable test duration.
There was a problem hiding this comment.
what would you think about just hard-coding soft_max_neighbors_per_org = 1 for these tests, and then setting the neighbor_count to 8 like the rest?
There was a problem hiding this comment.
In this case we should also set soft_max_neighbors_per_host = 1
|
An alternative suggestion was made at today's sync that we instead just don't run these all the time, and only run them on release testing. |
3 participants
