Support git being installed on windows in tests

.github/workflows/tests.yml

@@ -10,7 +10,7 @@ env:
 
 jobs:
   check:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
 
     steps:
       - uses: actions/checkout@v2
@@ -26,7 +26,7 @@ jobs:
   test-job:
     strategy:
       matrix:
-        os: [ubuntu-latest, windows-2019]
+        os: [ubuntu-20.04, windows-2019]
         # Python 3.8 is what we currently support for running cohortextractor
         # locally, and 3.9 is what we required for databuilder so we need to make
         # sure we can run with those
@@ -49,15 +49,15 @@ jobs:
       - uses: extractions/setup-just@aa5d15c144db4585980a44ebfdd2cf337c4f14cb  # 1.4
 
       - name: Run actual tests on ${{ matrix.os }}
-        if: ${{ matrix.os == 'ubuntu-latest' }}
-        run: just test
+        if: ${{ matrix.os == 'ubuntu-20.04' }}
+        run: just test -vvv
 
       - name: Run actual tests on ${{ matrix.os }}
         if: ${{ matrix.os == 'windows-2019' }}
-        run: just test-no-docker
+        run: just test-no-docker -vvv
 
   test-package-build:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     name: Test we can build PyPI package
     steps:
       - name: Checkout
@@ -84,7 +84,7 @@ jobs:
         run: just package-test sdist
 
   test-docker:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     name: Test docker image
     steps:
       - name: Checkout
@@ -102,7 +102,7 @@ jobs:
         run: just docker/test
 
   test-github-workflow-output:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     name: Inspect test runner output in the context of a Github Workflow
     steps:
       - name: Checkout

jobrunner/config.py

@@ -23,7 +23,7 @@ def _is_valid_backend_name(name):
         VERSION = subprocess.check_output(
             ["git", "describe", "--tags"], text=True
         ).strip()
-    except subprocess.CalledProcessError:
+    except (FileNotFoundError, subprocess.CalledProcessError):
         pass
 
 
@@ -33,7 +33,7 @@ def _is_valid_backend_name(name):
         GIT_SHA = subprocess.check_output(
             ["git", "rev-parse", "--short", "HEAD"], text=True
         ).strip()
-    except subprocess.CalledProcessError:
+    except (FileNotFoundError, subprocess.CalledProcessError):
         pass
 
 

tests/test_config.py

@@ -9,7 +9,7 @@
 
 
 script = """
-from jobrunner import config;
+from jobrunner import config
 cfg = {k: str(v) for k, v in vars(config).items() if k.isupper()}
 print(repr(cfg))
 """
@@ -25,17 +25,20 @@ def import_cfg(env, raises=None):
     if "SYSTEMROOT" in os.environ:
         env["SYSTEMROOT"] = os.environ["SYSTEMROOT"]
 
-    ps = subprocess.run(
-        [sys.executable, "-c", script],
-        env=env,
-        text=True,
-        capture_output=True,
-    )
-    if ps.returncode == 0:
-        print(ps.stdout)
-        return ast.literal_eval(ps.stdout), None
-    else:
-        return None, ps.stderr
+    try:
+        ps = subprocess.run(
+            [sys.executable, "-c", script],
+            env=env,
+            text=True,
+            check=True,
+            capture_output=True,
+        )
+    except subprocess.CalledProcessError as err:
+        print(err.stderr)
+        raise
+
+    print(ps.stdout)
+    return ast.literal_eval(ps.stdout)
 
 
 def test_config_imports_with_clean_env():
@@ -47,22 +50,22 @@ def test_config_presto_paths(tmp_path):
     key.write_text("key")
     cert = tmp_path / "cert"
     cert.write_text("cert")
-    cfg, err = import_cfg(
+    print(key)
+    print(cert)
+    cfg = import_cfg(
         {"PRESTO_TLS_KEY_PATH": str(key), "PRESTO_TLS_CERT_PATH": str(cert)}
     )
-    assert err is None
     assert cfg["PRESTO_TLS_KEY"] == "key"
     assert cfg["PRESTO_TLS_CERT"] == "cert"
 
 
 def test_config_presto_paths_not_exist(tmp_path):
-
     key = tmp_path / "key"
     key.write_text("key")
     cert = tmp_path / "cert"
     cert.write_text("cert")
 
-    cfg, err = import_cfg(
+    cfg = import_cfg(
         {
             "PRESTO_TLS_KEY_PATH": str(key),
             "PRESTO_TLS_CERT_PATH": str(cert),
@@ -71,25 +74,32 @@ def test_config_presto_paths_not_exist(tmp_path):
     assert cfg["PRESTO_TLS_KEY"] == "key"
     assert cfg["PRESTO_TLS_CERT"] == "cert"
 
-    # only one
-    _, err = import_cfg({"PRESTO_TLS_KEY_PATH": "foo"})
-    assert "Both PRESTO_TLS_KEY_PATH and PRESTO_TLS_CERT_PATH must be defined" in err
+    with pytest.raises(subprocess.CalledProcessError) as err:
+        import_cfg({"PRESTO_TLS_KEY_PATH": "foo"})
 
-    cfg, err = import_cfg(
-        {
-            "PRESTO_TLS_KEY_PATH": "key.notexists",
-            "PRESTO_TLS_CERT_PATH": str(cert),
-        }
+    assert "Both PRESTO_TLS_KEY_PATH and PRESTO_TLS_CERT_PATH must be defined" in str(
+        err.value.stderr
     )
-    assert "PRESTO_TLS_KEY_PATH=key.notexists" in err
 
-    cfg, err = import_cfg(
-        {
-            "PRESTO_TLS_KEY_PATH": str(key),
-            "PRESTO_TLS_CERT_PATH": "cert.notexists",
-        }
-    )
-    assert "PRESTO_TLS_CERT_PATH=cert.notexists" in err
+    with pytest.raises(subprocess.CalledProcessError) as err:
+        import_cfg(
+            {
+                "PRESTO_TLS_KEY_PATH": "key.notexists",
+                "PRESTO_TLS_CERT_PATH": str(cert),
+            }
+        )
+
+    assert "PRESTO_TLS_KEY_PATH=key.notexists" in str(err.value.stderr)
+
+    with pytest.raises(subprocess.CalledProcessError) as err:
+        import_cfg(
+            {
+                "PRESTO_TLS_KEY_PATH": str(key),
+                "PRESTO_TLS_CERT_PATH": "cert.notexists",
+            }
+        )
+
+    assert "PRESTO_TLS_CERT_PATH=cert.notexists" in str(err.value.stderr)
 
 
 @pytest.mark.parametrize(

tests/test_local_executor.py

@@ -528,6 +528,9 @@ def test_finalize_failed_oomkilled(docker_cleanup, test_repo, tmp_work_dir, volu
 
     wait_for_state(api, job, ExecutorState.EXECUTED)
 
+    # let the docker service catch up, or else it can sometimes not mark it as OOMKilled fast enough
+    time.sleep(5)
+
     status = api.finalize(job)
     assert status.state == ExecutorState.FINALIZING