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: Add more complex E2E test #12005

Merged
merged 2 commits into from
Oct 24, 2024
Merged

feat: Add more complex E2E test #12005

merged 2 commits into from
Oct 24, 2024

Conversation

klkvr
Copy link
Collaborator

@klkvr klkvr commented Oct 23, 2024

ref #12003

  • Add e2e_test_utils::setup_engine helper which allows to setup multiple nodes with new engine similarly to e2e_test_utils::setup
  • Adds correct support for Prague to e2e testing utils. It was previously not possible to send Prague payloads as we've never sent the requests field
  • Adds E2E test which generates seed, sends random transactions to node and then asserts that second node is able to sync from scratch after a single FCU.

Comment on lines +64 to +71
let requests = payload
.executed_block()
.unwrap()
.execution_outcome()
.requests
.first()
.unwrap()
.clone();
Copy link
Member

Choose a reason for hiding this comment

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

we should likely track some improvement on this, it's not nice and i think we're doing it in some other place too

let signer = signers.choose(&mut rng).unwrap();
let auth = Authorization {
chain_id: provider.get_chain_id().await?,
address: *call_destinations.choose(&mut rng).unwrap(),
Copy link
Member

Choose a reason for hiding this comment

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

this will never actually delegate as it will try to delegate to an empty account

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for this test this doesn't really matter

@onbjerg onbjerg added C-test A change that impacts how or what we test A-consensus Related to the consensus engine labels Oct 23, 2024
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.

cool, let's use this for incremental improvements

let signer = signers.choose(&mut rng).unwrap();
let auth = Authorization {
chain_id: provider.get_chain_id().await?,
address: *call_destinations.choose(&mut rng).unwrap(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for this test this doesn't really matter

@onbjerg onbjerg added this pull request to the merge queue Oct 24, 2024
Merged via the queue into main with commit 5e4da59 Oct 24, 2024
39 checks passed
@onbjerg onbjerg deleted the klkvr/e2e branch October 24, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine C-test A change that impacts how or what we test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants