feat(clis,pytest-plugins): allow pytest commands from package, addexecute eth-config command#1863
Conversation
danceratopz
left a comment
There was a problem hiding this comment.
Great idea to put this in execute!
Love the integration with src/ethereum_test_forks/forks/forks.py.
Can we add a way to test this via hive? Does this need another sub-command,
execute hive-eth-config? This would allow us to verify before clients hit the devnet and help minimize disruption.
9121084 to
080089e
Compare
|
@danceratopz addressed all your comments and implemented this PR: ethereum/EIPs#9989 Let me know what you think and thanks for the first review :) |
danceratopz
left a comment
There was a problem hiding this comment.
@danceratopz addressed all your comments and implemented this PR: ethereum/EIPs#9989
Nice idea on the fork hash!
The labels are🤌
LGTM, but the question still stands from the first review:
Can we add a way to test this via hive? Does this need another sub-command,
execute hive-eth-config? This would allow us to verify before clients hit the devnet and help minimize disruption.
Thanks for explaining offline :) This doesn't make much sense to test in a staging environment like hive as we need to test the client with the network's genesis. In the EEST meeting today, we decided we can think of this as more of a health check that should be ran regularly on networks to ensure the clients running there meet the fork, and perhaps more relevantly, the next fork's spec. |
danceratopz
left a comment
There was a problem hiding this comment.
@marioevz could you please add something to the docs for this new sub-command under:
- docs/running_tests/running.md
docs/running_tests/execute/
Tnx!
execute eth-config commandexecute eth-config command, enable commands running from package
708d816 to
c633d57
Compare
There was a problem hiding this comment.
Amazing! I was contemplating this yesterday as well, was perfect prep for this review 😄
I also found --rootdir but hadn't put it all together - you've added a really nice solution!
I was wondering about putting the ini files next to the "entry" pytest plugin for the command, but they're clearly better in one place. Easier to diff; no searching required.
c633d57 to
9e13891
Compare
execute eth-config command, enable commands running from packageexecute eth-config command
danceratopz
left a comment
There was a problem hiding this comment.
Hey @marioevz, thanks for adding docs and fill too! LGTM! I didn't get to test the final product as I wanted due to lack of time, but don't want to block a merge here!
7d6309e to
cb72139
Compare
🗒️ Description
Allow all pytest-plugin based commands to run outside of the repo
Allows all commands to run without having to clone the full repository. This was done by adding the pytest-*.ini config files to the pyproject.toml and also changing the working directory, root directory, and adding a new "working-directory" plugin to fix the working directory after pytest has been loaded, before the command runs.
execute eth-configAdd
execute eth-configcommand to test a running eth RPC client for the correct configuration given the specified network.pytest.ini->pytest-fill.iniThe default pytest configuration file for
fillhas been renamed and moved to the appropriate folder, to include in the project build.🔗 Related Issues or PRs
Closes #1885
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlinttype(scope):.