test: fix few e2e flaky tests #7762
Merged
Conversation
This file contains hidden or 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #7762 +/- ##
=========================================
Coverage 56.07% 56.07%
=========================================
Files 823 823
Lines 58017 58017
Branches 4465 4465
=========================================
Hits 32531 32531
Misses 25418 25418
Partials 68 68 🚀 New features to boost your workflow:
|
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
nflaig
reviewed
May 3, 2025
nazarhussain
force-pushed
the
nh/6358-flaky-e2e-tests
branch
from
63cd115 to
7e9982e
Compare
nazarhussain
commented
May 5, 2025
Comment on lines
225
to
226
| // Used only for sleep() statements | ||
| this.controller.abort(); |
Contributor
Author
There was a problem hiding this comment.
This change protects from scenarios where connection and libp2p dependencies are cleanup before properly saying goodbye. Identified through a lot of errors during e2e tests.
May-02 11:06:48.905[reqresp-main-B] verbose: Req error method=goodbye, version=1, encoding=ssz_snappy, client=Lodestar, peer=16...koSwxP, requestId=6 - REQUEST_ERROR_DIAL_ERROR
nazarhussain
commented
May 5, 2025
Comment on lines
+22
to
+25
| sequence: { | ||
| concurrent: false, | ||
| shuffle: false, | ||
| }, |
Contributor
Author
There was a problem hiding this comment.
This is not a behavior change, but will help to make sure that behavior does not change unintentionally in future.
nazarhussain
added a commit
that referenced
this pull request
May 7, 2025
**Motivation** Fix the behavior for reqresp to avoid unncessary response timeout. **Description** During debugging of the e2e flaky tests #7762 found some edge case for unnecessary response timeout. **Steps to test or reproduce** Run all tests
nazarhussain
force-pushed
the
nh/6358-flaky-e2e-tests
branch
from
f6e6567 to
d70f69d
Compare
nflaig
reviewed
May 7, 2025
Member
nflaig
left a comment
nflaig
left a comment
There was a problem hiding this comment.
Looks good overall, how do we test this? ideally we should add some load to CI and see if E2E tests in this PR don't fail due to it
|
|
||
| beforeEach(() => { | ||
| controller = new AbortController(); | ||
| }); |
Member
There was a problem hiding this comment.
does it make sense to abort the controller in afterEach?
Contributor
Author
There was a problem hiding this comment.
Had it before but does not make sense of it with these changes. Mostly it's used for sleep and peer connection, if any of those hangs up the test will already timeout.
nflaig
reviewed
May 7, 2025
nflaig
approved these changes
May 7, 2025
Contributor
Author
I see only way to is to give it a try and observe CI over few days, so we have real analysis of it. There is no way to produce/control the load on CI server on our own. |
Member
|
🎉 This PR is included in v1.30.0 🎉 |
wemeetagain
pushed a commit
that referenced
this pull request
Sep 1, 2025
**Motivation** - make e2e tests stable - peers get disconnected in e2e tests **Description** - I was not able to run `finalizeSync.test.ts` e2e tests in `mkeil/refactor-block-input-on-unstable` until I found this option added since #7762 - sometimes I found same issue with `unknownBlockSync.test.ts` e2e test, suppose it will help that test too since it uses same utils Co-authored-by: Tuyen Nguyen <[email protected]>
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.
Motivation
Make our tests more stable on CI.
Description
Fix following flaky tests
packages/beacon-node/test/e2e/network/reqresp.test.tsPartially Closes #6358
Steps to test or reproduce