Moved rigetti integration test logic to higher level conftest.py file… #6328
Conversation
… and included slow marker logic.
|
@pavoljuhas Created this pull request for issue #6211 . This is my first pull request, if I have made any mistake in the process kindly guide me. |
Define them in the correct section. Also clean up duplicate marker definition.
|
@nafaynajam - thank you for contributing, I will do the review later today. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6328 +/- ##
=======================================
Coverage 97.89% 97.89%
=======================================
Files 1108 1108
Lines 96210 96204 -6
=======================================
- Hits 94186 94182 -4
+ Misses 2024 2022 -2
☔ View full report in Codecov by Sentry. |
These are already specified in dev_tools/requirements/deps/.
Iterate over test items once rather than 3 times.
Allow checking of our custom test selection with `pytest --co -m skip`. Otherwise let the mark expression fully control what tests are selected.
dev_tools/conftest.py
Outdated
| config.addinivalue_line("markers", "slow: mark tests as slow") | ||
| config.addinivalue_line("markers", "weekly: mark tests as run only by weekly automation") | ||
| config.addinivalue_line("markers", "rigetti_integration: tests that connect to Quil compiler or QVM.") |
There was a problem hiding this comment.
This is unnecessary after adding mark definitions to pyproject.toml.
dev_tools/conftest.py
Outdated
| skip_slow_marker = pytest.mark.skip(reason='slow marker not selected') | ||
| # do not skip slow tests if --enable-slow-tests is passed |
There was a problem hiding this comment.
Typically conftest.py in subdirectories should have configurations relevant for that directory tree only.
The intent here is to have extra pytest options and test selection logic at one place.
As these are global settings it is better to move this to the top-level conftest.py.
dev_tools/conftest.py
Outdated
| if config.getoption('--rigetti-integration'): | ||
| return |
There was a problem hiding this comment.
The early return here will also enable tests with the weekly mark, because their deselection happens later on.
pyproject.toml
Outdated
| @@ -4,6 +4,9 @@ target_version = ['py39', 'py310', 'py311'] | |||
| skip-string-normalization = true | |||
| skip-magic-trailing-comma = true | |||
|
|
|||
| [tool.pytest] | |||
| markers = ["rigetti_integration: tests that connect to Quil compiler or QVM.", "slow: mark tests as slow", "weekly: mark tests as run only by weekly automation"] | |||
There was a problem hiding this comment.
This should be in the [tool.pytest.ini_options] section below.
Use pytest --markers to check if the extra marks are available.
Also the list should be formatted as one item per line for a better readability.
requirements.txt
Outdated
| @@ -0,0 +1,2 @@ | |||
| pytest~=7.4.0 | |||
There was a problem hiding this comment.
This file is unnecessary, dependencies are already specified under dev_tools/requirements.
There was a problem hiding this comment.
Please see inline comments. I have got some ideas for improvement,
which I pushed as extra commits.
Please take a look and let me know if the updated code looks ok.
As a general suggestion for git workflows, it is a good practice to do the
PR work in a new branch with some descriptive name (say unify-test-selection-logic).
The master branch can be then kept in sync with the main repo.
|
BTW, I have verified the default test selection and the |
|
Partial fix of #6211 |
Exclude a few conftest.py lines from coverage enforcement.
… and included slow marker logic.