-
Notifications
You must be signed in to change notification settings - Fork 27
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
chore(tests_integration): return tx sender address #2056
Conversation
36c38f8
to
731b8d3
Compare
7296087
to
bd9ea66
Compare
bd9ea66
to
d4e1db2
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2056 +/- ##
==========================================
- Coverage 40.10% 33.61% -6.50%
==========================================
Files 26 269 +243
Lines 1895 31203 +29308
Branches 1895 31203 +29308
==========================================
+ Hits 760 10488 +9728
- Misses 1100 19743 +18643
- Partials 35 972 +937 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Itay-Tsabary-Starkware)
crates/starknet_integration_tests/src/utils.rs
line 186 at r1 (raw file):
} fn create_txs_for_tx_generator_test_scenario(
Consider passing the account ID as an arg, and then you can get the contract address outside of this function.
Also can you please rename this function? It's not clear what it does.
crates/starknet_integration_tests/src/utils.rs
line 241 at r1 (raw file):
/// Creates and runs the many txs test scenario for the sequencer integration test. Returns /// a list of transaction hashes, in the order they are expected to be in the mempool. pub async fn run_transaction_generator_test_scenario<'a, Fut>(
Also rename to match.
Code quote:
run_transaction_generator_test_scenario
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: all files reviewed, 2 unresolved discussions (waiting on @alonh5)
crates/starknet_integration_tests/src/utils.rs
line 186 at r1 (raw file):
Previously, alonh5 (Alon Haramati) wrote…
Consider passing the account ID as an arg, and then you can get the contract address outside of this function.
Also can you please rename this function? It's not clear what it does.
Added a todo to address that, this will require a few changes elsewhere.
crates/starknet_integration_tests/src/utils.rs
line 241 at r1 (raw file):
Previously, alonh5 (Alon Haramati) wrote…
Also rename to match.
The todo above includes renaming this function as well.
f7eec9b
to
b9f88f3
Compare
b28bae6
to
88b3d4b
Compare
commit-id:945de3b4
88b3d4b
to
3a2382b
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 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)
crates/starknet_integration_tests/src/utils.rs
line 186 at r1 (raw file):
Previously, Itay-Tsabary-Starkware wrote…
Added a todo to address that, this will require a few changes elsewhere.
It's a small change, I opened a PR over your stack to show you what I meant.
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 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)
Stack: