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

fix(pytest): use python proxy framework to isolate nodes instead of unix utilities #3076

Closed
wants to merge 1 commit into from

Conversation

birchmd
Copy link
Contributor

@birchmd birchmd commented Aug 4, 2020

The goal of the local_network stress test is to ensure the network functions well under the condition that a random node occasionally loses contact with the rest of the network (i.e. experiences a "local network failure"). Previously this test was implemented by restricting access to network interfaces of specific process ids at the OS level. This solution was a little hacky, platform dependent, and not entirely reliable. We now have a proxy framework as part of pytest which is exactly meant to change (or drop) messages between nodes. We now make use of this framework in the local_network stress test instead of using the Unix utilities.

@gitpod-io
Copy link

gitpod-io bot commented Aug 4, 2020

@birchmd
Copy link
Contributor Author

birchmd commented Aug 4, 2020

python tests/stress/stress.py 3 3 3 0 staking transactions local_network

@SkidanovAlex could you try running this locally? For me I find that the test fails after some time because of timeout trying to access the node's RPC even though I increased the timeout to 5s in this PR. I'm not sure if this is revealing a bug in the node or if the problem is my machine is somehow slow.

@codecov
Copy link

codecov bot commented Aug 4, 2020

Codecov Report

Merging #3076 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3076      +/-   ##
==========================================
- Coverage   87.49%   87.44%   -0.06%     
==========================================
  Files         212      212              
  Lines       41185    41185              
==========================================
- Hits        36036    36013      -23     
- Misses       5149     5172      +23     
Impacted Files Coverage Δ
neard/tests/stake_nodes.rs 85.66% <0.00%> (-12.11%) ⬇️
chain/client/src/sync.rs 86.21% <0.00%> (-1.52%) ⬇️
chain/client/tests/bug_repros.rs 94.03% <0.00%> (-1.33%) ⬇️
chain/network/tests/routing.rs 99.37% <0.00%> (-0.63%) ⬇️
chain/chain/src/doomslug.rs 99.22% <0.00%> (-0.52%) ⬇️
chain/chain/src/store.rs 86.87% <0.00%> (-0.46%) ⬇️
chain/network/src/routing.rs 92.81% <0.00%> (-0.20%) ⬇️
chain/client/src/view_client.rs 68.29% <0.00%> (-0.19%) ⬇️
chain/client/tests/process_blocks.rs 89.71% <0.00%> (-0.16%) ⬇️
chain/chain/src/chain.rs 88.51% <0.00%> (-0.11%) ⬇️
... and 9 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 957ad2f...45e2fea. Read the comment docs.

@SkidanovAlex
Copy link
Collaborator

python tests/stress/stress.py 3 3 3 0 staking transactions local_network

@SkidanovAlex could you try running this locally? For me I find that the test fails after some time because of timeout trying to access the node's RPC even though I increased the timeout to 5s in this PR. I'm not sure if this is revealing a bug in the node or if the problem is my machine is somehow slow.

Locally also hit RPC timeouts.

@birchmd
Copy link
Contributor Author

birchmd commented Aug 17, 2020

Closing, changes were included in #3175

@birchmd birchmd closed this Aug 17, 2020
@birchmd birchmd deleted the stress-proxy branch August 17, 2020 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants