Skip to content
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

Fixing flakey tests #1883

Closed
wants to merge 3 commits into from
Closed

Fixing flakey tests #1883

wants to merge 3 commits into from

Conversation

andrei-toptal
Copy link

Description

Fixed flakey tests in TestEthClient and TestGethClient.

Other changes

Semantic of the existing code/tests was not changed.

Tested

Consistently failed before the fix and consistently survive these commands with the fix.
go test -race -count=100 -run ^TestEthClient$ github.com/celo-org/celo-blockchain/ethclient
go test -race -count=100 -run ^TestGethClient$ github.com/celo-org/celo-blockchain/ethclient/gethclient

Related issues

Backwards compatibility

NA

@andrei-toptal andrei-toptal requested a review from a team as a code owner April 14, 2022 14:56
@andrei-toptal andrei-toptal requested review from hbandura and 37ng and removed request for a team April 14, 2022 14:56
TestEthClient and TestGethClient
@codecov
Copy link

codecov bot commented Apr 14, 2022

Codecov Report

Merging #1883 (d9a4cb8) into master (178faa1) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head d9a4cb8 differs from pull request most recent head 46b2332. Consider uploading reports for the commit 46b2332 to get more accurate results

@@            Coverage Diff             @@
##           master    #1883      +/-   ##
==========================================
+ Coverage   54.38%   54.44%   +0.05%     
==========================================
  Files         673      674       +1     
  Lines       88954    88983      +29     
==========================================
+ Hits        48382    48448      +66     
+ Misses      36925    36875      -50     
- Partials     3647     3660      +13     
Impacted Files Coverage Δ
ethclient/testutils.go 100.00% <100.00%> (ø)
p2p/discover/ntp.go 66.66% <0.00%> (-9.53%) ⬇️
p2p/enode/iter.go 90.55% <0.00%> (-2.37%) ⬇️
les/utils/exec_queue.go 89.13% <0.00%> (-2.18%) ⬇️
eth/downloader/queue.go 79.31% <0.00%> (-1.61%) ⬇️
consensus/istanbul/core/handler.go 81.88% <0.00%> (-1.58%) ⬇️
les/utils/limiter.go 91.26% <0.00%> (-1.46%) ⬇️
trie/trie.go 78.32% <0.00%> (-1.40%) ⬇️
eth/protocols/snap/sync.go 71.59% <0.00%> (-0.90%) ⬇️
les/vflux/client/serverpool.go 76.75% <0.00%> (-0.74%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da90e93...46b2332. Read the comment docs.

Copy link
Contributor

@37ng 37ng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, it's been a pain that nodes weren't listening : )
Thanks for contributing @andrei-toptal

@37ng 37ng added the automerge label Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants