-
Notifications
You must be signed in to change notification settings - Fork 21
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
test(mempool_p2p): test tx received from p2p reach mempool #1990
base: main
Are you sure you want to change the base?
test(mempool_p2p): test tx received from p2p reach mempool #1990
Conversation
AlonLStarkWare
commented
Nov 12, 2024
- chore: derive Hash on RpcTransaction
- chore: changing closure Fn requirements on integration test utils
- test(mempool_p2p): test received rpc transactions are being broadcasted
- test(mempool_p2p): test tx received from p2p reach mempool
7dfcba9
to
04ec373
Compare
|
||
let _tx_hashes = run_integration_test_scenario(tx_generator, &mut |tx: RpcTransaction| { | ||
expected_txs.insert(tx.clone()); | ||
ready(TransactionHash::default()) // using the default value becuase we don't use the hash anyways. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: becuase
-> because
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
04ec373
to
3a1bdb9
Compare
Artifacts upload triggered. View details here |
Artifacts upload triggered. View details here |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1990 +/- ##
==========================================
- Coverage 40.10% 34.08% -6.03%
==========================================
Files 26 267 +241
Lines 1895 30966 +29071
Branches 1895 30966 +29071
==========================================
+ Hits 760 10554 +9794
- Misses 1100 19440 +18340
- Partials 35 972 +937 ☔ View full report in Codecov by Sentry. |
3a1bdb9
to
bfe4647
Compare
Artifacts upload triggered. View details here |
bfe4647
to
37ef08e
Compare
Artifacts upload triggered. View details here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 12 files reviewed, 1 unresolved discussion (waiting on @graphite-app[bot] and @ShahakShama)
|
||
let _tx_hashes = run_integration_test_scenario(tx_generator, &mut |tx: RpcTransaction| { | ||
expected_txs.insert(tx.clone()); | ||
ready(TransactionHash::default()) // using the default value becuase we don't use the hash anyways. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
37ef08e
to
d5d30ea
Compare
Artifacts upload triggered. View details here |
d5d30ea
to
dbccbea
Compare
Artifacts upload triggered. View details here |
dbccbea
to
0b0ec55
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 8 files at r4, 4 of 5 files at r6, 1 of 7 files at r8, 6 of 6 files at r9, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @AlonLStarkWare)
crates/starknet_integration_tests/tests/mempool_p2p_flow_test.rs
line 136 at r9 (raw file):
#[tokio::test] async fn test_mempool_receives_tx_from_other_peer(tx_generator: MultiAccountTransactionGenerator) { let handle = Handle::current();
Remove code dup with other test by defining a setup function in this module (it can be private)
crates/starknet_integration_tests/tests/mempool_p2p_flow_test.rs
line 205 at r9 (raw file):
} // waiting for the tx to be received (TODO: figure out a better solution)
You can iteratively call get_txs with a sleep of much lower time until you get expected_txs.len() txs
Fail if you passed x iterations (calculate x so that the time it will take is 2 seconds)
crates/starknet_integration_tests/tests/mempool_p2p_flow_test.rs
line 208 at r9 (raw file):
tokio::time::sleep(std::time::Duration::from_millis(2000)).await; let received_tx = mempool_client.get_txs(expected_txs.len()).await.unwrap();
received_txs
crates/starknet_integration_tests/tests/mempool_p2p_flow_test.rs
line 213 at r9 (raw file):
assert_eq!(received_tx.len(), expected_txs.len()); for tx in received_tx.iter() {
No need to write iter here. Surprised clippy and graphite-bot didn't say anything
crates/starknet_integration_tests/tests/mempool_p2p_flow_test.rs
line 214 at r9 (raw file):
for tx in received_tx.iter() { let converted_tx: RpcTransaction = match tx {
Add TODO to change mempool to store RpcTransaction