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

feat: improve e2e tests API + feeHistory test #12058

Merged
merged 2 commits into from
Oct 25, 2024
Merged

feat: improve e2e tests API + feeHistory test #12058

merged 2 commits into from
Oct 25, 2024

Conversation

klkvr
Copy link
Collaborator

@klkvr klkvr commented Oct 24, 2024

ref #12003

Simplifies e2e tests API by requiring payload building fn to be passed during node setup and by removing versioned_hashes argument from payload building functions as it can be fetched from BuiltPayload

Also added a test for feeHistory endpoint

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this is awesome,

left two comments for potential followups

Comment on lines +41 to +42
let (mut nodes, _tasks, wallet) =
setup_engine::<EthereumNode>(1, chain_spec.clone(), false, eth_payload_attributes).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just realized that returning the TaskManager here is kinda redundant:

let tasks = TaskManager::current();

and we can remove this from the response

Comment on lines +41 to +42
let (mut nodes, _tasks, wallet) =
setup_engine::<EthereumNode>(1, chain_spec.clone(), false, eth_payload_attributes).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

also, should we wrap this into a helper type

Vec<NodeHelperType<N, N::AddOns, BlockchainProvider2<NodeTypesWithDBAdapter<N, TmpDB>>>>,

I assume eventually we could use some utils that need to operate on all nodes for example

@mattsse mattsse added this pull request to the merge queue Oct 25, 2024
@mattsse mattsse added the C-test A change that impacts how or what we test label Oct 25, 2024
Merged via the queue into main with commit d988978 Oct 25, 2024
39 checks passed
@mattsse mattsse deleted the klkvr/tests branch October 25, 2024 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-test A change that impacts how or what we test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants