Skip to content
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

pytest-split in CI #2704

Merged
merged 5 commits into from
Oct 25, 2022
Merged

pytest-split in CI #2704

merged 5 commits into from
Oct 25, 2022

Conversation

janosh
Copy link
Member

@janosh janosh commented Oct 24, 2022

https://github.com/jerry-git/pytest-split is a pytest plugin that splits the test suite into equally sized splits based on test execution time.

@janosh
Copy link
Member Author

janosh commented Oct 24, 2022

Looks like this takes the runtime for the main text-linux workflow from ~18 min down to ~9 min. The number of parallel jobs went from 4 to 10. Setting up the repo and installing deps is a ~3.5 min constant time for every job. So probably not that useful to crank the parallel job count much higher.

@shyuep @mkhorton Wanted to get your opinion on this before merging since it's a bigger-than-usual change to the main CI workflow. We could add the same splitting to the mac and windows workflows. In fact, is there still a strong reason to not run the same test suite on all 3 platforms? Would make maintenance of the workflow files easier since we could get rid of .github/workflows/test-{max,win}.yml and simply add os to matrix strategy in .github/workflows/test-linux.yml.

@shyuep
Copy link
Member

shyuep commented Oct 24, 2022

Sure this is fine.

…moments' on /pymatgen/io/phonopy.py:51

     @requires(Phonopy, "phonopy not installed!")
    def get_pmg_structure(phonopy_structure):
        ...
        lattice = phonopy_structure.cell
        frac_coords = phonopy_structure.scaled_positions
        symbols = phonopy_structure.symbols
        masses = phonopy_structure.masses
>       mms = phonopy_structure.magnetic_moments
@coveralls
Copy link

coveralls commented Oct 24, 2022

Coverage Status

Coverage increased (+18.08%) to 81.711% when pulling 9af323a on pytest-split-in-ci into 302ca2c on master.

@janosh janosh force-pushed the pytest-split-in-ci branch 3 times, most recently from 4b42aaa to 0538c7d Compare October 24, 2022 17:55
@janosh janosh force-pushed the pytest-split-in-ci branch from 3e02c75 to 9af323a Compare October 25, 2022 02:39
@janosh janosh enabled auto-merge (squash) October 25, 2022 02:40
@janosh janosh disabled auto-merge October 25, 2022 02:40
@janosh janosh enabled auto-merge October 25, 2022 02:40
@janosh janosh disabled auto-merge October 25, 2022 03:08
@janosh janosh merged commit aac3784 into master Oct 25, 2022
@janosh janosh deleted the pytest-split-in-ci branch October 25, 2022 03:08
@janosh
Copy link
Member Author

janosh commented Oct 25, 2022

Coverage Status

Coverage increased (+18.08%) to 81.711% when pulling 9af323a on pytest-split-in-ci into 302ca2c on master.

Just noticed the coverage increased by 18% in this PR. Is this because a fifth of the test suite wasn't running before with the old splitting setup?

include:
  - pkg: pymatgen/analysis/chemenv pymatgen/analysis/elasticity pymatgen/analysis/magnetism
    pkg_id: 1
  - pkg: pymatgen/analysis --ignore=pymatgen/analysis/chemenv --ignore=pymatgen/analysis/elasticity --ignore=pymatgen/analysis/magnetism
    pkg_id: 2
  - pkg: pymatgen/electronic_structure pymatgen/symmetry pymatgen/command_line pymatgen/ext
    pkg_id: 3
  - pkg: pymatgen --ignore=pymatgen/analysis --ignore=pymatgen/electronic_structure --ignore=pymatgen/symmetry --ignore=pymatgen/ext --ignore=pymatgen/command_line
    pkg_id: 4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants