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

test(starknet_integration_tests): add revert flow integration test #4076

Open
wants to merge 2 commits into
base: noam.s/feat_starknet_integration_tests_add_snapshot_tx_generator_fn
Choose a base branch
from

Conversation

noamsp-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_integration_tests_add_snapshot_tx_generator_fn branch from 2489e4f to 1788552 Compare February 11, 2025 09:51
@noamsp-starkware noamsp-starkware force-pushed the noam.s/test_starknet_integration_tests_add_revert_flow_integration_test branch from d054428 to a1d391a Compare February 11, 2025 09:51
@noamsp-starkware noamsp-starkware marked this pull request as ready for review February 11, 2025 09:52
Copy link
Contributor

@ShahakShama ShahakShama left a 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, 6 unresolved discussions (waiting on @noamsp-starkware)


crates/starknet_integration_tests/src/bin/sequencer_node_end_to_end_revert_flow_integration_test.rs line 16 at r1 (raw file):

#[tokio::main]
async fn main() {
    configure_tracing().await;

Call the setup instead


crates/starknet_integration_tests/src/bin/sequencer_node_end_to_end_revert_flow_integration_test.rs line 53 at r1 (raw file):

    // Run the first block scenario to bootstrap the accounts.
    integration_test_manager.send_bootstrap_txs_and_verify().await;
    integration_test_manager.send_invoke_txs_and_verify(N_TXS, BLOCK_TO_REVERT_TO).await;

You can erase this send and have the blocks that we revert to just contain the bootstrap txs


crates/starknet_integration_tests/src/bin/sequencer_node_end_to_end_revert_flow_integration_test.rs line 57 at r1 (raw file):

    let tx_generator_snapshot = integration_test_manager.snapshot_tx_generator();
    integration_test_manager.send_invoke_txs_and_verify(N_TXS, BLOCK_TO_REVERT_FROM).await;
    integration_test_manager.shutdown_nodes(node_indices.clone());

Add log before this line


crates/starknet_integration_tests/src/bin/sequencer_node_end_to_end_revert_flow_integration_test.rs line 61 at r1 (raw file):

    integration_test_manager.run_nodes(node_indices.clone()).await;
    // allow the nodes to revert the blocks.
    sleep(Duration::from_secs(5));

Move this Duration to a constant


crates/starknet_integration_tests/src/bin/sequencer_node_end_to_end_revert_flow_integration_test.rs line 61 at r1 (raw file):

    integration_test_manager.run_nodes(node_indices.clone()).await;
    // allow the nodes to revert the blocks.
    sleep(Duration::from_secs(5));

Add logs before and after this sleep. Something like: "Waiting X seconds for revert to finish" and "Finished waiting for revert to finish. Checking it succeeded and turning the config off


crates/starknet_integration_tests/src/bin/sequencer_node_end_to_end_revert_flow_integration_test.rs line 61 at r1 (raw file):

    integration_test_manager.run_nodes(node_indices.clone()).await;
    // allow the nodes to revert the blocks.
    sleep(Duration::from_secs(5));

Poll the marker with small sleeps between each poll instead of sleeping and checking it after 5 seconds (The comments above are still relevant, you should log what you're doing in the test and put the polling durations in constants)

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_integration_tests_add_snapshot_tx_generator_fn branch from 1788552 to d11d2af Compare February 11, 2025 11:56
@noamsp-starkware noamsp-starkware force-pushed the noam.s/test_starknet_integration_tests_add_revert_flow_integration_test branch from a1d391a to 33c530a Compare February 11, 2025 11:56
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_integration_tests_add_snapshot_tx_generator_fn branch from d11d2af to 7098149 Compare February 11, 2025 14:04
@noamsp-starkware noamsp-starkware force-pushed the noam.s/test_starknet_integration_tests_add_revert_flow_integration_test branch 2 times, most recently from 5d93986 to 314c460 Compare February 11, 2025 14:15
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_integration_tests_add_snapshot_tx_generator_fn branch from 7098149 to c495fd7 Compare February 11, 2025 14:59
@noamsp-starkware noamsp-starkware force-pushed the noam.s/test_starknet_integration_tests_add_revert_flow_integration_test branch from 314c460 to 3223f6a Compare February 11, 2025 20:29
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_integration_tests_add_snapshot_tx_generator_fn branch from c495fd7 to 8f46461 Compare February 12, 2025 10:55
@noamsp-starkware noamsp-starkware force-pushed the noam.s/test_starknet_integration_tests_add_revert_flow_integration_test branch from 3223f6a to b6e2103 Compare February 12, 2025 10:55
Copy link
Contributor

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a 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 4 files reviewed, 7 unresolved discussions (waiting on @noamsp-starkware and @ShahakShama)


-- commits line 2 at r2:
There should be a different value for each integration test, so please:

  1. In a preceding pr, please move this value to be defined in the binary, and not interally in this function. Pass this value as an arg up to the current definition location.
  2. For this pr, create a new enum variant (and also rename the current one), and use it in this binary

let test_unique_id = TestIdentifier::EndToEndIntegrationTest;

Code quote:

- b6e2103: test(starknet_integration_tests): add revert flow integration test

@ShahakShama ShahakShama force-pushed the noam.s/feat_starknet_integration_tests_add_snapshot_tx_generator_fn branch from 8f46461 to bec9bef Compare February 13, 2025 08:55
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.

4 participants