-
Notifications
You must be signed in to change notification settings - Fork 375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
flakey protocol test beforeEach fix #4651
Merged
Merged
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
alecps
requested review from
asaj,
cmcewen,
nategraf and
timmoreton
as code owners
August 8, 2020 02:02
alecps
force-pushed
the
alecps/protocol-tests
branch
from
August 8, 2020 23:12
50bef0b
to
6b3570c
Compare
alecps
changed the title
flakey protocol beforeEach triage
flakey protocol test beforeEach fix
Aug 9, 2020
nambrot
reviewed
Aug 10, 2020
alecps
requested review from
nambrot
and removed request for
i1skn,
jeanregisser and
jmrossy
August 10, 2020 17:03
alecps
force-pushed
the
alecps/protocol-tests
branch
from
August 10, 2020 18:23
51a7d16
to
a113730
Compare
nambrot
approved these changes
Aug 11, 2020
ewilz
pushed a commit
to ewilz/celo-monorepo
that referenced
this pull request
Sep 29, 2020
### Description - This change should fix the flakey `error: Invalid JSON RPC response: ""` error that we often see in the protocol test beforeEach hooks (particularly those for the Attestations contract). - Implements a custom beforeEachWithRetries hook for the protocol tests package that will optionally retry beforeEach hooks and add sleep in between retries. This is necessary to avoid flakiness that seems to be caused by port exhaustion in CI and a mishandling of the error by web3. (See web3/web3.js#3425 and web3/web3.js#926). - I've only added `beforeEachWithRetries` to the Attestations tests because this error appears there the most. We can add it elsewhere in the future as needed. ### Other changes None ### Tested To see that this works, search logs for "retry" in https://circleci.com/gh/celo-org/celo-monorepo/248635 ### Related issues - Fixes celo-org#4570 ### Backwards compatibility Not applicable
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.
Description
This change should fix the flakey
error: Invalid JSON RPC response: ""
error that we often see in the protocol test beforeEach hooks (particularly those for the Attestations contract).Implements a custom beforeEachWithRetries hook for the protocol tests package that will optionally retry beforeEach hooks and add sleep in between retries. This is necessary to avoid flakiness that seems to be caused by port exhaustion in CI and a mishandling of the error by web3. (See XMLHttpRequest errors are not handled correctly web3/web3.js#3425 and Many contract function calls/transactions via Web3 will eventually cause TCP ephemeral port exhaustion web3/web3.js#926).
I've only added
beforeEachWithRetries
to the Attestations tests because this error appears there the most. We can add it elsewhere in the future as needed.Other changes
None
Tested
To see that this works, search logs for "retry" in https://circleci.com/gh/celo-org/celo-monorepo/248635
Related issues
Backwards compatibility
Not applicable