apollo_integration_tests: split end to end flow test into 5 scenarios

[ArniStarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[ArniStarkware]

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.
Learn more

• apollo_integration_tests: simplify scenario arg in end to end flow #11505
• apollo_integration_tests: split end to end flow test into 5 scenarios #11504 👈 (View in Graphite)
• apollo_integration_tests: use validate_tx_count accross integration tests #11503
• apollo_integration_tests: clean old todos #11502
• apollo_integration_tests: rename reverted_l1_handler_tx_flow_test file #11501
• main-v0.14.1-committer

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 2 comments.
Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware).

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 12 at r1 (raw file):

use crate::common::{end_to_end_flow, validate_tx_count, EndToEndFlowArgs, TestScenario};

mod common;

Why is this needed? What breaks if we just import the relevant stuff from there?

Code quote:

mod common;

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 16 at r1 (raw file):

/// Number of threads is 3 = Num of sequencer + 1 for the test thread.
#[tokio::test(flavor = "multi_thread", worker_threads = 3)]
async fn test_deploy_account_and_invoke_flow() {

No need for this prefix, the annotation indicates this is a test 🙏 (this is the convention)
Relevant to the other tests as well

Code quote:

test_

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @ArniStarkware).

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 18 at r1 (raw file):

async fn test_deploy_account_and_invoke_flow() {
    end_to_end_flow(EndToEndFlowArgs::new(
        TestIdentifier::EndToEndFlowTest,

We'd also need to pass a test index or something of that sort to differentiate the used ports of the 5 different tests

Code quote:

TestIdentifier::EndToEndFlowTest,

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @ArniStarkware).

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 18 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

We'd also need to pass a test index or something of that sort to differentiate the used ports of the 5 different tests

(this currently works as the ci is configured to run these tests sequentially, and the motivation is to remove this configuration)

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @ArniStarkware).

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 18 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

(this currently works as the ci is configured to run these tests sequentially, and the motivation is to remove this configuration)

This can and probably should be in a different pr -- but add a todo please 🙏

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, 4 unresolved discussions (waiting on @ArniStarkware).

---

crates/apollo_integration_tests/tests/l1_to_l2_message_flow_test.rs line 34 at r1 (raw file):

) -> Vec<L1HandlerTransaction> {
    const N_TXS: usize = 1;
    const SHOULD_REVERT: bool = false;

Needed as consts?
wdyt about cancelling this fn entirely and creating a closure |tx_generator| create_l1_to_l2_messages_args(tx_generator, 1, false) at the callsite instead?
(can be in a different pr, but add a todo 🙏 )

Code quote:

    const N_TXS: usize = 1;
    const SHOULD_REVERT: bool = false;

[ArniStarkware]

@ArniStarkware made 4 comments.
Reviewable status: 0 of 7 files reviewed, 4 unresolved discussions (waiting on @Itay-Tsabary-Starkware).

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 12 at r1 (raw file):
From the AI agent:

In Rust integration tests, each file in tests/ is compiled as its own crate. To use a module from tests/common/, you must declare it with mod common; in each test file.

This is the same issue as link

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 16 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

No need for this prefix, the annotation indicates this is a test 🙏 (this is the convention)
Relevant to the other tests as well

Done.

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 18 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

This can and probably should be in a different pr -- but add a todo please 🙏

Done.

---

crates/apollo_integration_tests/tests/l1_to_l2_message_flow_test.rs line 34 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Needed as consts?
wdyt about cancelling this fn entirely and creating a closure |tx_generator| create_l1_to_l2_messages_args(tx_generator, 1, false) at the callsite instead?
(can be in a different pr, but add a todo 🙏 )

Done.

[ArniStarkware]

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 16 at r2 (raw file):

/// Number of threads is 3 = Num of sequencer + 1 for the test thread.
#[tokio::test(flavor = "multi_thread", worker_threads = 3)]
async fn deploy_account_and_invoke_flow() {

@meship-starkware Looks good to you?

Suggestion:

mod common;

// TODO(Meshi): Fail the test if no class have migrated.
/// Number of threads is 3 = Num of sequencer + 1 for the test thread.
#[tokio::test(flavor = "multi_thread", worker_threads = 3)]
async fn deploy_account_and_invoke_flow() {

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 2 comments and resolved 4 discussions.
Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware).

---

crates/apollo_integration_tests/tests/deploy_account_and_invoke_flow_test.rs line 12 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

From the AI agent:

In Rust integration tests, each file in tests/ is compiled as its own crate. To use a module from tests/common/, you must declare it with mod common; in each test file.

This is the same issue as link

Yea, more of a reason to deprecate this atrocity 😅

---

crates/apollo_integration_tests/tests/l1_to_l2_message_flow_test.rs line 15 at r2 (raw file):

async fn l1_to_l2_message_flow() {
    end_to_end_flow(EndToEndFlowArgs::new(
        // TODO(Arni): Change the TestIdentifier to L1ToL2MessageFlow.

Let's talk about this in person, this is somewhat suboptimal 🙏

Code quote:

// TODO(Arni): Change the TestIdentifier to L1ToL2MessageFlow.