search

ethereum / web3.py

A python interface for interacting with the Ethereum blockchain and ecosystem.
http://web3py.readthedocs.io
MIT License
4.91k stars 1.68k forks source link

Flaky Integration Test(s) #3366

Open reedsa opened 3 months ago

reedsa commented 3 months ago

What happened?

Every now and then we get failures in CI from tests that appear to flake. Investigate and fix the flakiness.

Running with pytest-flakefinder shows exactly where things break down. I found that running all tests with in TestGoEthereumAsyncEthModuleTest with the flakefinder, tests/integration/go_ethereum/test_goethereum_ws/test_async_ctx_manager_w3.py::TestGoEthereumAsyncEthModuleTest::test_eth_modify_transaction passes the first 30ish times or so but then hangs for a bit and starts failing.

It appears that the request ID is not being matched with the cached response properly, if the response even exists. The test itself modifies the transaction but I'm not sure if it would effectively overwrite the original transaction in the cache. Perhaps this is why the IDs get messed up or the queue is not the expected size. Still digging.

After further investigation, it seems that slowing down the test with sleep or even using wait_for_transaction_receipt will cause the failure to happen on the 14th run.

Added @flaky_geth_dev_mining but it isn't working either.

Code that produced the error

pytest tests/integration/go_ethereum/test_goethereum_ws/test_async_ctx_manager_w3.py::TestGoEthereumAsyncEthModuleTest

Full error output

No response

Fill this section in if you know how this could or should be fixed

Will need to investigate. Could be some context somewhere that's getting reused, like an object that should be copied instead of assigned.

web3 Version

No response

Python Version

No response

Operating System

No response

Output from pip freeze

No response

reedsa commented 3 months ago

We discussed changing the websocket provider so that replace_transaction handles the transaction properly. The original request should be removed from the cache so that there is no expectation for that response to come back.

fselmo commented 3 days ago

It doesn't seem like the modify transaction test is failing anymore, at least on a consistent way. We should probably try to observe this for a while and close this out if that's the case. #3440 Addressed some very flaky tests at least so our suite should be in a decent place compared to when we first rolled out the geth --dev refactor.