-
Notifications
You must be signed in to change notification settings - Fork 340
feat(tox): Implement eest environments #1408
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
base: forks/osaka
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1408 +/- ##
============================================
Coverage 94.94% 94.94%
============================================
Files 583 583
Lines 34665 34665
Branches 3070 3070
============================================
Hits 32914 32914
Misses 1198 1198
Partials 553 553
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see this stuff in EELS 😄 but I'm not sure we need the main fill
tox envs?
Also, not sure we need to speed run lint and typecheck sections as we don't edit any files in the EEST sub-trees right? I.e., we do
and the equivalent for mypy (much easier) and get a unified experience.
Edit (after chatting with Mario): A lot of these changes (mypy stubs, for example) can be made in a PR that can be applied during the Switch (or post-Switch), so it's not intended to merge this now/before the Switch.
ethereum-spec-patch = "ethereum_spec_tools.patch_tool:main" | ||
ethereum-spec-evm = "ethereum_spec_tools.evm_tools:main" | ||
ethereum-spec-fill = "cli.pytest_commands.fill:fill" | ||
ethereum-spec-consume = "cli.pytest_commands.consume:consume" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added EEST's entrypoints here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was mainly to provoke a discussion though, so that everyone knows what we're getting into :)
minversion = 7.0 | ||
python_files = *.py | ||
testpaths = tests/ | ||
testpaths = tests/eest/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and many other of these changes are not required for tox, right?
This is a task to be added to the Switch (similar to the line-length change), to avoid editing these sub-trees in EELS before the Switch?
That means that this command will be broken in EELS until the Switch and standalone uvx commands from EELS won't work, but that's ok, I think. By standalone, I mean things like uvx --from git+https://github.com/ethereum/execution-spec-tests \ execute ...
.
develop = Osaka | ||
eip7692 = EOFv1 | ||
|
||
[testenv:eest-tests-deployed] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we'll need this environment post-Weld. We fill until the fork that the EELS branch points to and this will typically be a develop branch, so we fill until there.
extras = test,fill | ||
commands = ethereum-spec-fill -n auto -m "benchmark" --gas-benchmark-values 5 --output=/tmp/fixtures-tox --clean --evm-bin=evmone-t8n | ||
|
||
[testenv:eest-tests-develop] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This essentially duplicates the existing EELS py3
environment, right?
setenv = | ||
# Use custom EELS_RESOLUTIONS_FILE if it is set via the environment (eg, in CI) | ||
EELS_RESOLUTIONS_FILE = {env:EELS_RESOLUTIONS_FILE:} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope we don't need the resolver here 😄 but perhaps I don't understand your intention of adding this here?
minversion = 7.0 | ||
python_files = *.py | ||
testpaths = tests/ | ||
testpaths = tests/eest/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could avoid this until the Switch by using -o testpaths=tests/eest
on the tox command-line to override this ini file setting.
This needs to be split into pre and post switch PRs. |
What was wrong?
EEST code was missing tox environments.
How was it fixed?
Partially added tox environments, skipping those related to documentation but those can be added on a separate PR.
Current state:
Cute Animal Picture