Skip to content

Conversation

@greged93
Copy link
Collaborator

@greged93 greged93 commented Nov 3, 2025

PR for the update of the test setup. Provides a set of high level abstractions that allow testers to easily set up initial tests conditions, run the test and directly expect test outcome.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025

CodSpeed Performance Report

Merging #407 will not alter performance

Comparing tests/improve-framework (e7da229) with main (e9cff2d)

Summary

✅ 2 untouched

Comment on lines +95 to +102
pub fn sequencer() -> TestFixtureBuilder {
TestFixtureBuilder::new().with_sequencer()
}

/// Create a new test fixture builder for follower nodes.
pub fn followers(count: usize) -> TestFixtureBuilder {
TestFixtureBuilder::new().with_nodes(count)
}
Copy link
Collaborator

@frisitano frisitano Nov 4, 2025

Choose a reason for hiding this comment

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

Should these methods instead consume self so we can chain them? Something like TestFixture::builder().sequencer().followers(2)

/// The underlying node context.
pub node: NodeHelperType<ScrollRollupNode, TestBlockChainProvider>,
/// Engine instance for this node.
pub engine: Engine<ScrollAuthApiEngineClient<EC>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you erase the type here using:

    pub engine: Engine<dyn ScrollEngineApi>,

let events = &mut self.fixture.nodes[self.node_index].chain_orchestrator_rx;

let result = timeout(self.timeout_duration, async {
while let Some(event) = events.next().await {
Copy link
Contributor

Choose a reason for hiding this comment

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

For debugging purposes we should log all events here (matching and non matching). This makes it much easier to understand which events are being processed/fired instead of the expected one. Similar to what is done in wait_for_event_predicate

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