-
Notifications
You must be signed in to change notification settings - Fork 64
apollo_l1_provider: add anvil to flow test #9871
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
base: guyn/l1provider/flow_test_infrastructure
Are you sure you want to change the base?
apollo_l1_provider: add anvil to flow test #9871
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
752d05e to
7d8c090
Compare
7d8c090 to
14b0114
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.
@ShahakShama reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @guy-starkware)
crates/apollo_l1_provider/tests/flow_tests.rs line 53 at r2 (raw file):
// Send message from L1 to L2. let l2_contract_address = "0x12";
Extract this to a function
crates/apollo_l1_provider/tests/flow_tests.rs line 53 at r2 (raw file):
// Send message from L1 to L2. let l2_contract_address = "0x12";
Move this below the setup
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, 3 unresolved discussions (waiting on @guy-starkware)
crates/apollo_l1_provider/tests/flow_tests.rs line 70 at r2 (raw file):
assert!(message_timestamp > BlockTimestamp(0)); // Find the L1 event that was posted to Anvil.
Change to // Make sure the L1 event was posted to Anvil
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, 3 unresolved discussions (waiting on @guy-starkware)
crates/apollo_l1_provider/tests/flow_tests.rs line 53 at r2 (raw file):
Previously, ShahakShama wrote…
Extract this to a function
Consider putting into that function the "Find the L1 event that was posted to Anvil"
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, 3 unresolved discussions (waiting on @guy-starkware)
crates/apollo_l1_provider/tests/flow_tests.rs line 53 at r2 (raw file):
Previously, ShahakShama wrote…
Consider putting into that function the "Find the L1 event that was posted to Anvil"
Looks like great minds think alike (#9909)
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, 3 unresolved discussions (waiting on @ShahakShama)
crates/apollo_l1_provider/tests/flow_tests.rs line 53 at r2 (raw file):
Previously, ShahakShama wrote…
Looks like great minds think alike (#9909)
yeah, slowly building it up.
crates/apollo_l1_provider/tests/flow_tests.rs line 53 at r2 (raw file):
Previously, ShahakShama wrote…
Move this below the setup
Seems like beautifying something that is going to go away on the next PR is a bit of a waste of time (and unnecessary merge conflicts).
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:
complete! all files reviewed, all discussions resolved (waiting on @guy-starkware)
14b0114 to
a78c5bc
Compare

No description provided.