tests speedup #119
tests speedup #119
Conversation
naik-aakash
commented
Jun 5, 2023
•
naik-aakash
commented
- Test adding adding splits tag to pytest for speedup
Add splits to pytest for speedup
test adding split tag
Combine coverage data from splits
|
Hi @JaGeo , This PR tries to resolve the open #97 issue. At the moment it seems to work with significant reduction in time needed for executing tests. Have attempted to use similar strategy as of pymatgen. There seems to be one small issue, there seems to be some loss of coverage, and it mainly seems to stem from tests that used pytest-plugins. If you have any idea how to solve this let me know. I am looking into this as well. So far no luck 😞 |
naik-aakash
force-pushed
the
tests-speedup
branch
from
d36406c to
bbdd293
Compare
|
@naik-aakash Thanks for trying! No idea why the coverage reduces so drastically ... |
.github/workflows/python-package.yml
Outdated
| @@ -27,7 +28,7 @@ jobs: | |||
| - name: Install dependencies | |||
| run: | | |||
| python -m pip install --upgrade pip | |||
| python -m pip install flake8 pytest pytest-mock | |||
| python -m pip install flake8 pytest pytest-mock pytest-split pytest-cov | |||
| python -m pip install types-setuptools | |||
| python -m pip install coverage | |||
There was a problem hiding this comment.
Do you need the second coverage when you install pytest-cov?
There was a problem hiding this comment.
I think we do not need the second coverage. Was trying different possible solutions I could find, one option stated trying to use pytest-cov. But End result was still the same so, we can get rid of it seems.
| @@ -40,5 +41,30 @@ jobs: | |||
| flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics | |||
| - name: Test with pytest and coverage | |||
| run: | | |||
| coverage run -m pytest | |||
| coverage report | |||
| pytest --cov=lobsterpy --cov-append --splits 10 --group ${{ matrix.split }} --durations-path ./lobsterpy/.pytest-split-durations | |||
There was a problem hiding this comment.
To me it looks like you are doing 10 splits but only run 5 out of the 10? Do I overlook something?
There was a problem hiding this comment.
Ahh I see, I made some blunder. Let me fix it. I make 5 splits at top and forgot to change to 5 here. Probably that is root cause of this reduced coverage
|
I changed it already. Let's test the workflow with the 10 splits. |
… tests-speedup sync remote
|
I will get back to this tomorrow. |
Number of splits was the error. It works perfectly now. |
|
I am merging it. Thanks @naik-aakash. |
