Attempt to address some of the consistently flaky integration tests #3440
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
fselmo
force-pushed
the
further-flaky-test-adjustments
branch
from
99a169b to
cadf7f8
Compare
fselmo
force-pushed
the
further-flaky-test-adjustments
branch
4 times, most recently
from
748a686 to
687d91c
Compare
fselmo
changed the title
fselmo
force-pushed
the
further-flaky-test-adjustments
branch
from
687d91c to
28e3715
Compare
- Introduce ``RequestTimedOut`` and raise this for requests with timeout messaging. - This allows us to be more specific on flaky tests that fail with node timeout errors. Specify ``RequestTimedOut`` as the expected exception in relevant tests marked with ``@pytest.mark.xfail``.
fselmo
force-pushed
the
further-flaky-test-adjustments
branch
from
28e3715 to
8965da1
Compare
fselmo
force-pushed
the
further-flaky-test-adjustments
branch
from
d631301 to
71af107
Compare
fselmo
force-pushed
the
further-flaky-test-adjustments
branch
from
71af107 to
6d80681
Compare
fselmo
force-pushed
the
further-flaky-test-adjustments
branch
from
a3ce6dc to
5c581e4
Compare
Collaborator
Author
|
Ok @kclowes, @pacrob, @reedsa this should be ready for review. Let me know if you have any questions. This commit fixes a wrong check in some tests that the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What was wrong?
Some of our integration tests dealing with the way
geth --devmines within an interval window flaky and still not passing with a flaky decorator in some cases. I'm going to try to run this branch many times and try to address them here.How was it fixed?
xfail+strict=False(899a909)gasPriceon dynamic fee transactions was equal to themaxFeePerGas. Really, it's the minimum between themaxFeePerGasandmaxPriorityFeePerGas+baseFeePerGas(block), which is the "effective gas price". Use<=in that comparison.Feature:
Introduce a
RequestTimedOutexception for when requests to the node time out.Use this exception in
xfailtests that fail specifically with a client-side timeout to distinguish the fail case. It isn't possible to usexfailandflakyon the same test out-of-the-box and have them work well enough. This PR adds a decorator that:max_runsandmin_passesand retries the test according to flaky rules.flakyretry, if this particular exception is raised, itxfailsthe test with a givenreasonstring argument, as is also accepted byxfail.What this means is that we can retry a test up to
max_runstimes, but if by the end of it we have some flaky exception case that is sometimes expected, wexfailthe test and it silently fails for that test run.Todo:
Cute Animal Picture