-
Notifications
You must be signed in to change notification settings - Fork 372
chore(execute): fix execute hive mode #1698
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
| minversion = 7.0 | ||
| python_files = *.py | ||
| testpaths = tests/ | ||
| testpaths = ../../tests/ |
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 doesn't look right, if the cwd is ./execution-specs/ I think it should not do relative paths.
What's the error you are seeing that you are trying to fix with this?
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.
Fixed thanks!
|
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1698 +/- ##
============================================
Coverage 86.07% 86.07%
============================================
Files 743 743
Lines 44078 44078
Branches 3894 3894
============================================
Hits 37938 37938
Misses 5659 5659
Partials 481 481
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:
|
13b81ac to
a3754a0
Compare
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.
Hey @spencer-tb! Can you check whether adding the test_case_description fixture is really necessary?
| @pytest.fixture(scope="function") | ||
| def test_case_description(request: pytest.FixtureRequest) -> str: | ||
| """The description of the current test case for hive.""" | ||
| return f"Test: {request.node.name}" |
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 will return the test ID, not the test case description as expected by Hive. Is this necessary? Perhaps you're intentionally overwriting this fixture?
Lines 183 to 205 in 15b5cfd
| @pytest.fixture(scope="function") | |
| def test_case_description(request: pytest.FixtureRequest) -> str: | |
| """ | |
| Fixture to extract and combine docstrings from the test class and the test | |
| function. | |
| """ | |
| description_unavailable = "No description available - add a docstring to the python test class or function." | |
| test_class_doc = "" | |
| test_function_doc = "" | |
| if hasattr(request.node, "cls"): | |
| test_class_doc = ( | |
| f"Test class documentation:\n{request.cls.__doc__}" | |
| if request.cls | |
| else "" | |
| ) | |
| if hasattr(request.node, "function"): | |
| test_function_doc = ( | |
| f"{request.function.__doc__}" if request.function.__doc__ else "" | |
| ) | |
| if not test_class_doc and not test_function_doc: | |
| return description_unavailable | |
| combined_docstring = f"{test_class_doc}\n\n{test_function_doc}".strip() | |
| return combined_docstring |
ποΈ Description
Fixes execute hive mode, specifically for adding the
execute-blobssimulator to the hive CI.π Related Issues or PRs
Blocker for: ethpandaops/fusaka-devnets#115
Requires: ethereum/hive#1365
β Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.Cute Animal Picture