Merge pull request #73 from ownik/pytest_fail-under

Fix --fail-under with SystemExit raising from pytest or script

Author: jaltmayerpizzorno

.github/workflows/tests.yml

@@ -45,7 +45,7 @@ jobs:
     - name: install prereqs
       run: |
         python3 -m pip install -U pip  # to avoid warnings
-        python3 -m pip install pytest
+        python3 -m pip install pytest pytest-xdist
 
     - name: install Unix dependencies
       if: matrix.os != 'windows-latest'

src/slipcover/__main__.py

@@ -43,7 +43,7 @@ def wrapper(*pargs, **kwargs):
     return wrapper
 
 
-def get_coverage(sci):
+def merged_coverage(sci):
     """Combines this process' coverage with that of any previously forked children and xdist workers."""
     global input_tmpfiles, output_tmpfile
 
@@ -100,7 +100,7 @@ def wrapper(*pargs, **kwargs):
         global output_tmpfile
 
         if output_tmpfile:
-            json.dump(get_coverage(sci), output_tmpfile)
+            json.dump(merged_coverage(sci), output_tmpfile)
             output_tmpfile.flush()
 
         original_exit(*pargs, **kwargs)
@@ -266,7 +266,7 @@ def printit(coverage, outfile):
                                   missing_width=args.missing_width)
 
         if not args.silent:
-            coverage = get_coverage(sci)
+            coverage = merged_coverage(sci)
             if args.out:
                 with open(args.out, "w") as outfile:
                     printit(coverage, outfile)
@@ -275,6 +275,8 @@ def printit(coverage, outfile):
 
     atexit.register(sci_atexit)
 
+    return_code = 0
+
     if args.script:
         # python 'globals' for the script being executed
         script_globals: Dict[Any, Any] = dict()
@@ -300,20 +302,25 @@ def printit(coverage, outfile):
             code = sci.instrument(code)
 
         with sc.ImportManager(sci, file_matcher):
-            exec(code, script_globals)
-
+            try:
+                exec(code, script_globals)
+            except SystemExit as e:
+                return_code = e.code if e.code is not None else 0
     else:
         import runpy
         sys.argv = [*args.module, *args.script_or_module_args]
         with sc.ImportManager(sci, file_matcher):
-            runpy.run_module(*args.module, run_name='__main__', alter_sys=True)
+            try:
+                runpy.run_module(*args.module, run_name='__main__', alter_sys=True)
+            except SystemExit as e:
+                return_code = e.code if e.code is not None else 0
 
     if args.fail_under:
-        cov = sci.get_coverage()
+        cov = merged_coverage(sci)
         if cov['summary']['percent_covered'] < args.fail_under:
             return 2
 
-    return 0
+    return return_code
 
 
 if __name__ == "__main__":

tests/branch.py

@@ -2,3 +2,5 @@ def foo(x):
     if x == 0:
         x += 1
 foo(0)
+
+raise SystemExit()

tests/test_coverage.py

@@ -5,6 +5,7 @@
 import sys
 import xml.etree.ElementTree as ET
 from pathlib import Path
+from textwrap import dedent
 
 import pytest
 
@@ -694,14 +695,78 @@ def test_summary_in_output_zero_lines(do_branch):
 
 
 @pytest.mark.parametrize("json_flag", ["", "--json"])
-def test_fail_under(json_flag):
+def test_fail_under(tmp_path, json_flag):
     p = subprocess.run(f"{sys.executable} -m slipcover {json_flag} --fail-under 100 tests/branch.py".split(), check=False)
     assert 0 == p.returncode
 
-    p = subprocess.run(f"{sys.executable} -m slipcover {json_flag} --branch --fail-under 83 tests/branch.py".split(), check=False)
+    p = subprocess.run(f"{sys.executable} -m slipcover {json_flag} --branch --fail-under 85 tests/branch.py".split(), check=False)
     assert 0 == p.returncode
 
-    p = subprocess.run(f"{sys.executable} -m slipcover {json_flag} --branch --fail-under 84 tests/branch.py".split(), check=False)
+    p = subprocess.run(f"{sys.executable} -m slipcover {json_flag} --branch --fail-under 86 tests/branch.py".split(), check=False)
+    assert 2 == p.returncode
+
+    p = subprocess.run(f"{sys.executable} -m slipcover --branch --fail-under 93 -m pytest tests/pyt.py".split(), check=False)
+    assert 0 == p.returncode
+
+    p = subprocess.run(f"{sys.executable} -m slipcover --branch --fail-under 94 -m pytest tests/pyt.py".split(), check=False)
+    assert 2 == p.returncode
+
+
+def test_fail_under_precedence_with_nonzero_exit(tmp_path):
+    """When the script/pytest run itself fails (nonzero SystemExit) AND
+    coverage is below the fail-under threshold, coverage failure (RC 2)
+    takes precedence. But when coverage is fine, the run's own nonzero
+    exit code must be preserved, not silently replaced with 0.
+    """
+    script = tmp_path / "script.py"
+    script.write_text(dedent("""\
+        def foo(x):
+            if x:
+                return 1
+            return 2
+        foo(0)
+        raise SystemExit(3)
+    """))
+
+    # coverage is fine (line 3 "return 1" never runs, but threshold is low) --
+    # the script's own exit code (3) must be preserved
+    p = subprocess.run(f"{sys.executable} -m slipcover --fail-under 1 {script}".split(), check=False)
+    assert 3 == p.returncode
+
+    # coverage is below threshold -- fail-under (2) must override the
+    # script's own exit code
+    p = subprocess.run(f"{sys.executable} -m slipcover --fail-under 100 {script}".split(), check=False)
+    assert 2 == p.returncode
+
+
+def test_fail_under_precedence_with_failing_pytest_run(tmp_path):
+    """Same precedence check as test_fail_under_precedence_with_nonzero_exit,
+    but through the `-m pytest` path with a genuinely failing test (pytest's
+    own SystemExit(1)), rather than a script raising SystemExit directly.
+    """
+    test_file = tmp_path / "test_mod.py"
+    test_file.write_text(dedent("""\
+        def foo(x):
+            if x:
+                return 1
+            return 2
+
+        def test_fail():
+            assert foo(0) == 2
+            assert False
+    """))
+
+    # coverage is fine -- pytest's own failure exit code (1) must be preserved
+    p = subprocess.run(
+        [sys.executable, '-m', 'slipcover', '--fail-under', '1', '-m', 'pytest', test_file.name],
+        cwd=str(tmp_path), check=False)
+    assert 1 == p.returncode
+
+    # coverage is below threshold -- fail-under (2) must override pytest's
+    # own exit code
+    p = subprocess.run(
+        [sys.executable, '-m', 'slipcover', '--fail-under', '100', '-m', 'pytest', test_file.name],
+        cwd=str(tmp_path), check=False)
     assert 2 == p.returncode
 
 

tests/test_xdist.py

@@ -4,6 +4,7 @@
 import sys
 import json
 from pathlib import Path
+from textwrap import dedent
 
 import pytest
 
@@ -216,3 +217,46 @@ def test_xdist_four_workers(tmp_path):
     # All lines should still be covered with more workers
     assert [1, 2, 3, 4, 5, 6, 8, 9, 10, 11, 13, 14] == file_cov['executed_lines']
     assert [] == file_cov['missing_lines']
+
+
+def test_xdist_fail_under_uses_merged_coverage(tmp_path):
+    """--fail-under must be checked against the merged (all-workers) coverage,
+    not just the coordinator process' own view. Under xdist, actual test code
+    runs only in worker subprocesses -- the coordinator's own local coverage
+    has no files at all, which trivially (and silently) reports as 100%,
+    defeating --fail-under entirely if the local, unmerged view is used.
+    """
+    test_file = tmp_path / "test_partial.py"
+    test_file.write_text(dedent("""\
+        def foo(x):
+            if x:
+                return 1
+            return 2
+
+        def bar(y):
+            if y:
+                return 3
+            return 4
+
+        def test_foo():
+            assert foo(0) == 2
+
+        def test_bar():
+            assert bar(0) == 4
+    """))
+
+    # real merged coverage is 10/12 = 83.3% (the "return 1"/"return 3"
+    # branches are never taken) -- comfortably above 50, so this must pass.
+    # The coordinator's own local, unmerged view would incorrectly see 0%
+    # (it discovers the file via cwd-matching but never executes it itself,
+    # since actual test execution happens only in the xdist workers), which
+    # would incorrectly fail this same check.
+    result = subprocess.run(
+        [sys.executable, '-m', 'slipcover', '--fail-under', '50',
+         '-m', 'pytest', '-n', '2', test_file.name],
+        cwd=str(tmp_path),
+        capture_output=True,
+        text=True
+    )
+
+    assert result.returncode == 0, f"stdout: {result.stdout}\nstderr: {result.stderr}"