Merge pull request #739 from janosh/pytest-split

use pytest-split in GHA test workflow

Author: itsduowang

.github/workflows/combine_durations.py

@@ -0,0 +1,21 @@
+import json
+from pathlib import Path
+
+split_prefix = "split-"
+durations_path = Path(".test_durations")
+
+split_paths = Path(".").glob(f"{split_prefix}*/{durations_path.name}")
+
+try:
+    previous_durations = json.loads(durations_path.read_text())
+except FileNotFoundError:
+    previous_durations = {}
+
+new_durations = previous_durations.copy()
+
+for path in split_paths:
+    durations = json.loads(path.read_text())
+    new_durations.update(durations)
+
+durations_path.parent.mkdir(parents=True, exist_ok=True)
+durations_path.write_text(json.dumps(new_durations))

.github/workflows/test.yml

@@ -10,20 +10,28 @@ jobs:
   test:
     if: github.repository_owner == 'hackingmaterials' # don't run on forks
     runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        # pytest-split groups for automatically equally split concurrent test runners
+        group: [1, 2, 3, 4, 5]
     services:
       mongodb:
         image: mongo
         ports:
           - 27017:27017
 
     steps:
-      - name: Checkout repo
-        uses: actions/checkout@v2
+      - name: Check out repo
+        uses: actions/checkout@v3
 
-      - name: Setup Python
-        uses: actions/setup-python@v2
+      - name: Set up python
+        uses: actions/setup-python@v3
         with:
           python-version: 3.8
+          cache: pip
+          cache-dependency-path: |
+            setup.py
+            requirements-ci.txt
 
       - name: Install OpenBabel
         run: |
@@ -32,19 +40,54 @@ jobs:
           # https://github.com/openbabel/openbabel/issues/2408#issuecomment-1014466193
           sudo ln -s /usr/include/openbabel3 /usr/local/include/openbabel3
 
-      - name: Cache pip
-        uses: actions/cache@v2
-        with:
-          path: ~/.cache/pip
-          key: ${{ runner.os }}-pip-${{ hashFiles('requirements-ci.txt', 'setup.py') }}
-          restore-keys: |
-            ${{ runner.os }}-pip-
-
       - name: Install dependencies
         run: |
           pip install -r requirements-ci.txt
           pip install .[complete]
 
-      - name: pytest
+      - name: Get durations from cache
+        uses: actions/cache@v2
+        with:
+          path: test_durations
+          # the key must never match, even when restarting workflows, as that
+          # will cause durations to get out of sync between groups, the
+          # combined durations will be loaded if available
+          key: test-durations-split-${{ github.run_id }}-${{ github.run_number}}-${{ matrix.group }}
+          restore-keys: |
+            test-durations-combined-${{ github.sha }}
+            test-durations-combined
+
+      - name: Run tests
         run: |
-          pytest --ignore=atomate/qchem/test_files --cov=atomate --cov-report html:coverage_reports atomate
+          pytest --splits 5 --group ${{ matrix.group }} --store-durations \
+            --cov=atomate --cov-report html:coverage_reports atomate
+
+      - name: Upload partial durations
+        uses: actions/upload-artifact@v2
+        with:
+          name: split-${{ matrix.group }}
+          path: .test_durations
+
+  update_durations:
+    name: Combine and update test durations
+    runs-on: ubuntu-latest
+    needs: test
+    steps:
+      - name: Check out repo
+        uses: actions/checkout@v2
+
+      - name: Get durations from cache
+        uses: actions/cache@v2
+        with:
+          path: .test_durations
+          # key won't match during the first run for the given commit, but
+          # restore-key will if there's a previous stored durations file,
+          # so cache will both be loaded and stored
+          key: test-durations-combined-${{ github.sha }}
+          restore-keys: test-durations-combined
+
+      - name: Download artifacts
+        uses: actions/download-artifact@v2
+
+      - name: Combine test durations
+        run: python3 .github/workflows/combine_durations.py

.gitignore

@@ -31,7 +31,7 @@ pip-log.txt
 # Unit test / coverage reports
 .coverage
 .tox
-nosetests.xml
+.test_durations
 *.mypy_cache/*
 
 # Translations

atomate/common/firetasks/tests/test_parse_outputs.py

@@ -21,9 +21,6 @@ def __init__(self):
     def assimilate(self, path):
         return {"drone": "Test Drone", "dir_name": "/test", "state": "successful"}
 
-    def get_valid_paths(self, path):
-        return path
-
 
 class TestToDbTask(AtomateTest):
     def test_ToDbTask(self):

atomate/lammps/drones.py

@@ -83,7 +83,7 @@ def assimilate(
         dump_files = [dump_files] if isinstance(dump_files, str) else dump_files
 
         # input set
-        lmps_input = LammpsInputSet.from_file(
+        lmps_input = LammpsInputSet.from_file(  # noqa: F821
             "lammps", input_file, {}, data_file, data_filename
         )
 
@@ -94,7 +94,7 @@ def assimilate(
                 dumps.append((df, LammpsDump.from_file(os.path.join(path, df))))
 
         # log
-        log = LammpsLog(log_file=log_file)
+        log = LammpsLog(log_file=log_file)  # noqa: F821
 
         logger.info(f"Getting task doc for base dir :{path}")
         d = self.generate_doc(path, lmps_input, log, dumps)
@@ -163,9 +163,6 @@ def generate_doc(self, dir_name, lmps_input, log, dumps):
             )
             return None
 
-    def get_valid_paths(self, path):
-        return [path]
-
     def as_dict(self):
         init_args = {
             "additional_fields": self.additional_fields,

atomate/qchem/drones.py

@@ -503,7 +503,3 @@ def validate_doc(self, d):
             diff = v.difference(set(d.get(k, d).keys()))
             if diff:
                 logger.warning(f"The keys {diff} in {k} not set")
-
-    @staticmethod
-    def get_valid_paths(self, path):
-        return [path]

atomate/vasp/drones.py

@@ -420,7 +420,7 @@ def generate_doc(self, dir_name, vasprun_files, outcar_files):
                     d["output"][k] = d_calc_final["output"][k]
 
             # store optical data, overwrites the LOPTICS data
-            if d["input"]["incar"].get("ALGO") == 'CHI':
+            if d["input"]["incar"].get("ALGO") == "CHI":
                 for k in ["optical_absorption_coeff", "dielectric"]:
                     d["output"][k] = d_calc_final["output"][k]
 
@@ -486,12 +486,12 @@ def process_vasprun(self, dir_name, taskname, filename):
             d["output"][k] = d["output"].pop(v)
 
         # Process bandstructure and DOS
-        if self.bandstructure_mode is not False:  # noqa
+        if self.bandstructure_mode is not False:
             bs = self.process_bandstructure(vrun)
             if bs:
                 d["bandstructure"] = bs
 
-        if self.parse_dos is not False:  # noqa
+        if self.parse_dos is not False:
             dos = self.process_dos(vrun)
             if dos:
                 d["dos"] = dos
@@ -588,7 +588,7 @@ def process_vasprun(self, dir_name, taskname, filename):
             d["output"]["optical_absorption_coeff"] = vrun.optical_absorption_coeff
 
         # parse output from response function
-        if vrun.incar.get("ALGO") == 'CHI':
+        if vrun.incar.get("ALGO") == "CHI":
             dielectric = vrun.dielectric
             d["output"]["dielectric"] = dict(
                 energy=dielectric[0], real=dielectric[1], imag=dielectric[2]

atomate/vasp/firetasks/lobster_tasks.py

@@ -231,7 +231,8 @@ def __init__(self, *args, **kwargs):
                         f"Check that you did not misspell it."
                     )
 
-    def _find_gz_file(self, filename):
+    @staticmethod
+    def _find_gz_file(filename):
         gz_filename = filename + ".gz"
         if os.path.exists(gz_filename):
             return gz_filename

requirements-ci.txt

@@ -3,3 +3,4 @@ coverage==5.5
 moto==2.0.10
 pytest-cov==2.12.1
 pytest==6.2.4
+pytest-split