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.

Sign up for GitHub

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

Jump to bottom

test(engine): enable state root task in engine unit tests #14482

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

test(engine): enable state root task in engine unit tests #14482

wants to merge 1 commit into from

Conversation

fgimenez
Copy link
Member

@fgimenez fgimenez commented Feb 13, 2025 •
edited
Loading

Closes: #14376

  • Introduces traits StateRootTaskFactory, StateRootTaskRunner and StateRootComputeHandle
  • Changes EngineApiTreeHandler to accept and use a StateRootTaskFactory, StateRootTask is created from it
  • Adds implementations to these traits for prod and test code. In tests, the impl allows to set the expected state root

@fgimenez fgimenez added C-test A change that impacts how or what we test A-engine Related to the engine implementation labels Feb 13, 2025
@fgimenez fgimenez force-pushed the fgimenez/enable-state-root-in-engine-tests branch 4 times, most recently from f430844 to cda2a21 Compare February 19, 2025 15:36
@fgimenez fgimenez marked this pull request as ready for review February 19, 2025 15:36
@fgimenez fgimenez force-pushed the fgimenez/enable-state-root-in-engine-tests branch from cda2a21 to 2930979 Compare February 19, 2025 15:53
@fgimenez fgimenez force-pushed the fgimenez/enable-state-root-in-engine-tests branch 3 times, most recently from 32b0300 to e4e01f8 Compare February 19, 2025 22:13
@fgimenez
Copy link
Member Author

fgimenez commented Feb 20, 2025 •
edited
Loading

this has grown too big, i'll split it to keep the changes scoped:

  • one PR with the changes in the DataBaseProviderFactory impl for MockEthProvider
  • one PR with the changes in how has_enough_parallelism is managed, allowing to override the value read from the system to run tests with state root task enabled on constrained environments (eg. CI)
  • rest of the changes specifically related to enabling state root task in engine unit tests in this PR

This was referenced Feb 20, 2025
Merged
Merged
@fgimenez fgimenez force-pushed the fgimenez/enable-state-root-in-engine-tests branch from e4e01f8 to 5d5db1d Compare February 20, 2025 12:03
@fgimenez fgimenez changed the base branch from main to fgimenez/tree-config-has-enough-parallelism February 20, 2025 16:32
@fgimenez fgimenez requested a review from klkvr as a code owner February 20, 2025 16:32
@fgimenez fgimenez force-pushed the fgimenez/enable-state-root-in-engine-tests branch from 5d5db1d to b6a3f1c Compare February 20, 2025 16:33
@fgimenez fgimenez force-pushed the fgimenez/tree-config-has-enough-parallelism branch from 00c52b5 to 16825a3 Compare February 20, 2025 16:34
@fgimenez fgimenez force-pushed the fgimenez/enable-state-root-in-engine-tests branch from b6a3f1c to fe43a5c Compare February 20, 2025 16:34
Base automatically changed from fgimenez/tree-config-has-enough-parallelism to main February 21, 2025 13:09
@fgimenez fgimenez force-pushed the fgimenez/enable-state-root-in-engine-tests branch from fe43a5c to 4c26dd2 Compare February 21, 2025 13:15
mattsse
mattsse reviewed Feb 24, 2025
View reviewed changes
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 makes sense

@fgimenez I'm blocking this until I've made more progress on #14589 and ideally incorporate this there because I'm changing how the statetask works

@mattsse mattsse added the S-blocked This cannot more forward until something else changes label Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse left review comments

@rakita rakita Awaiting requested review from rakita rakita is a code owner

@joshieDo joshieDo Awaiting requested review from joshieDo joshieDo is a code owner

@shekhirin shekhirin Awaiting requested review from shekhirin shekhirin is a code owner

@rkrasiuk rkrasiuk Awaiting requested review from rkrasiuk rkrasiuk is a code owner

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

@onbjerg onbjerg Awaiting requested review from onbjerg

@gakonst gakonst Awaiting requested review from gakonst

@DaniPopes DaniPopes Awaiting requested review from DaniPopes

@klkvr klkvr Awaiting requested review from klkvr

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
A-engine Related to the engine implementation C-test A change that impacts how or what we test S-blocked This cannot more forward until something else changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix engine tree tests with state root task enabled
2 participants
@fgimenez @mattsse