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(esplora): speed up test_finalize_chain_update #1663

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ValuedMammal
Copy link
Contributor

The test test_finalize_chain_update was taking up a lot of time during testing. This PR reduces the time to run the tests to around 6 seconds. I stripped down the test logic to the bare essentials. I think it is adequate but let me know if you have other ideas.

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK, Approach NACK

IMO the old tests were a bit more thorough with the testing, and also easier to extend with more test cases. For performance, I think the main culprit is the starting and stopping of the test env.

I think it would make sense to make the test cases use relative heights.

struct TestCase {
    /// Initial checkpoint heights, relative to the starting tip height of the env.
    initial_cps: &'a [i32],
    /// Number of blocks mined since starting env tip. The new tip should be introduced by the update.
    chain_extends_by: i32,
    /// Anchors passed into `chain_update`: `(height_relative_to_starting_tip, txid)`.
    anchors: &[(i32, Txid)],
}

Another option is to keep how TestCase is currently, reuse the TestEnv between tests, but make sure the a TestCase's initial_env_height is always equal or greater than the last TestCase's final_env_height. I think I like this solution better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants