-
Notifications
You must be signed in to change notification settings - Fork 395
chore(tests): read test list from file #1807
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1807 +/- ##
============================================
Coverage 87.31% 87.31%
============================================
Files 541 541
Lines 32832 32832
Branches 3015 3015
============================================
Hits 28668 28668
Misses 3557 3557
Partials 607 607
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:
|
tests/json_infra/conftest.py
Outdated
| return | ||
|
|
||
| with open(tests_path) as f: | ||
| test_ids = set(x[:-1] for x in f.readlines()) |
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.
| test_ids = set(x[:-1] for x in f.readlines()) | |
| test_ids = set(line.strip() for line in f) |
Otherwise, this could skip the last test in the file, if the file does not end with a new line
| ), | ||
| ) | ||
|
|
||
| parser.addoption( |
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.
Some of these options might end up contradicting each other. For example, we might have --fork=Frontier and the tests file might have an entry called tests/json_infra/fixtures/latest_fork_tests/fixtures/state_tests/berlin/eip2930_access_list/test_account_storage_warm_cold_state.json::tests/berlin/eip2930_access_list/test_acl.py::test_account_storage_warm_cold_state[fork_Shanghai-state_test-account_warm_True-storage_key_warm_True]::Shanghai::0 which is from Shanghai.
In this set-up, the Shanghai tests will not run. Is this what is intended? Alternatively, we could just make it invalid to pair the --tests-file option with the others.
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 think this makes sense. If I'm not mistaken, the following command wouldn't run any tests either, so this behaviour is consistent.
pytest --fork=Frontier \
'tests/json_infra/fixtures/latest_fork_tests/fixtures/state_tests/berlin/eip2930_access_list/test_account_storage_warm_cold_state.json::tests/berlin/eip2930_access_list/test_acl.py::test_account_storage_warm_cold_state[fork_Shanghai-state_test-account_warm_True-storage_key_warm_True]::Shanghai::0'
🗒️ Description
Teaches the json_infra tests how to read the set of tests to execute from a file. Useful in conjunction with minimize-tests.
Cute Animal Picture