Beat itest [3/3]: fix all itest flakes #9260
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Pull reviewers statsStats of the last 30 days for lnd:
|
|
OMG, what is this sorcery? I don't think I've ever seen this on an You are my hero, @yyforyongyu! |
It's a bit of cheating as the unit tests are skipped for me to quickly get the CI results🤓 Plus I know there are two more bugs that can cause itest to fail - one is sql-related and the other is graph, but yeah, at least the flakes are fixed and we should see this as a norm in 2025! |
This was referenced Nov 18, 2024
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
315ff72 to
2c92455
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
2 times, most recently
from
be8726e to
5a07ffa
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
2c92455 to
88350c6
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
2 times, most recently
from
c2aeb68 to
67f9404
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
88350c6 to
e259d59
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
from
67f9404 to
ee869a2
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
e259d59 to
11587b8
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
from
ee869a2 to
4bdf873
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
11587b8 to
a744740
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
from
4bdf873 to
db80ec6
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
a744740 to
d8e61c8
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
2 times, most recently
from
e24d6b9 to
7fba4ab
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
d8e61c8 to
43247ea
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
2 times, most recently
from
446296c to
9f93fdd
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
43247ea to
269bfd7
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
2 times, most recently
from
7e1c2d4 to
b70b0b0
Compare
yyforyongyu
force-pushed
the
yy-beat-itest-flakes
branch
from
269bfd7 to
b1011ac
Compare
Also removed the duplicate test cases.
Also fixes a wrong usage of `ht.Subtest`.
To make the CI indicative, we now starting tracking the flaky tests found when running on Windows. As a starting point, rather than ignore the windows CI entirely, we now identify there are cases where lnd can be buggy when running in windows. We should fix the tests in the future, otherwise the windows build should be deleted.
To increase the speed from 40m per run to roughly 20m per run.
Most of the time we only need to fund the node with given number of UTXOs without concerning the amount, so we add the more efficient funding method as it mines a single block in the end.
Previous splitting logic simply put all the remainder in the last tranche, which could make the last tranche run significantly more test cases. We now change it so the remainder is evened out across tranches.
For Windows the tests run much slower so we create customized timeouts for them.
This commit removes the panic used in checking the shutdown log. Instead, the error is returned and asserted in `shutdownAllNodes` so it's easier to check which node failed in which test. We also catch all the errors returned from `StopDaemon` call to properly access the shutdown behavior.
Keep the SQL, etcd, bitcoin rpcpolling builds and non-ubuntu builds at 8 since they are less stable.
We sometimes see `timeout waiting for UTXOs` error from bitcoind-related itests due to the chain backend not synced to the miner. We now assert it's synced before continue.
The response from `ClosedChannels` may not be up-to-date, so we wrap it inside a wait closure.
yyforyongyu
force-pushed
the
yy-beat-itest-optimize
branch
from
df5d749 to
cc89b32
Compare
bhandras
approved these changes
Dec 12, 2024
| @@ -357,12 +353,6 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) { | |||
| break | |||
| } | |||
| } | |||
|
|
|||
| mts.ht.CloseChannelAssertPending(mts.bob, channelPointBobPaula, false) | |||
There was a problem hiding this comment.
I'd argue that not starting from the same clean state can alter test outcomes in the extreme case, but as a plus it may end up finding more flakes. We still probably get more readable logs if we clean things up properly.
| // be excluded from the test suite atm. | ||
| // | ||
| // TODO(yy): fix these tests and remove them from this list. | ||
| var excludedTestsWindows = []string{ |
There was a problem hiding this comment.
At least we have a nice and tidy TODO list now :)
| // Otherwise log a warning if it's mining more than 40 blocks. | ||
| desc := "!============================================!\n" | ||
|
|
||
| desc += fmt.Sprintf("Too many blocks (%v) mined in one test! Tips:\n", |
There was a problem hiding this comment.
As an alternative to tranches we could parallelize per itest, but it does require a lot of runners and can be costly. The end result is super fast test runs (see loop server itests).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix all the itest flakes to make sure the
blockbeatworks as expected. The key results,All itest flakes are now documented and fixed.
A large decrease in the time taken to run the CI, e.g., for btcd itest, previously it took 45m and now it takes around 18m.
Check #9306 for more context.
In this final PR, we focus on breaking down the large tests into smaller ones, skipping some flaky tests for windows, and minor flake fixes.
TODOs: