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

fix(consume,plugins): Fix suite duplication due to xdist, consume cli refactor #712

Merged
merged 8 commits into from
Jul 26, 2024

Conversation

marioevz
Copy link
Member

@marioevz marioevz commented Jul 25, 2024

🗒️ Description

Fixes test_suite fixture from spawning duplicated test suites when xdist is used.

Reason being that xdist spawns a different process per worker to distribute the load, and the session or module scoped fixtures are not really shared between them.

Fix is to use the file system to create global locks per consume session, achieved by creating a temporary folder that is unique to each consume instance and is shared between the xdist processes, but do not disrupt other consume sessions.

The fixture also creates a lock per test-suite, so it is possible to run different test suites (i.e. one for rlp and one for engine) in the same consume instance.

Consume commands were also refactored to reduce code and consume hive command is now available which runs test suites of consume rlp and consume engine simultaneously.

🔗 Related Issues

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
  • Tests: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
  • Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

@marioevz marioevz added type:bug Something isn't working scope:pytest Scope: Changes EEST's pytest plugins type:refactor Type: Refactor type:feat type: Feature scope:consume Scope: Consume command suite labels Jul 25, 2024
@marioevz marioevz requested a review from spencer-tb July 25, 2024 20:34
Copy link
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

Really nice! LGTM.

I'm going create a small PR based on this to split pytest_commands.py to the following within src/cli:

pytest_commands/
├── common.py
├── consume.py
├── fill.py
└── __init__.py
...
├── execute.py
├── fuzzer.py

It should keep things clean when adding execute/fuzzer!

Feel free to merge this or wait for my small refactor :D

@spencer-tb
Copy link
Collaborator

The fixture also creates a lock per test-suite, so it is possible to run different test suites (i.e. one for rlp and one for engine) in the same consume instance.

Just so I understand this essentially means running consume hive with parallelism where only one instance is spawned for both rlp and engine?

@marioevz
Copy link
Member Author

The fixture also creates a lock per test-suite, so it is possible to run different test suites (i.e. one for rlp and one for engine) in the same consume instance.

Just so I understand this essentially means running consume hive with parallelism where only one instance is spawned for both rlp and engine?

You can either launch consume rlp and consume engine separately or do consume hive with any xdist number parameter, and the result in hive view should be exactly the same.

Basically we could reduce the code in the hive cluster script.

@marioevz marioevz changed the title fix(consume,plugins): Hive test suite duplication when using xdist fix(consume,plugins): Hive test suite duplication when using xdist, consume cli refactor Jul 26, 2024
@marioevz marioevz changed the title fix(consume,plugins): Hive test suite duplication when using xdist, consume cli refactor fix(consume,plugins): Fix suite duplication due to xdist, consume cli refactor Jul 26, 2024
@marioevz marioevz merged commit 247c8d3 into main Jul 26, 2024
7 checks passed
@marioevz marioevz deleted the fix-sim-parallelism branch July 26, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:consume Scope: Consume command suite scope:pytest Scope: Changes EEST's pytest plugins type:bug Something isn't working type:feat type: Feature type:refactor Type: Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants