From a1c9babf0783ee3138b84ae1962f470e44eb8dea Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 3 Oct 2026 16:34:02 -0400 Subject: [PATCH 1/2] Skip Quick Feedback coverage when the test scope is narrowed When a change maps to too many test targets, limit_test_paths narrows the run to the directly changed tests. run_tests still passed every changed .py file to coverage --include, so the uploaded report counted lines in those files that the narrowed tests never exercise. On the reference-only PRs #9764 and #9766 that sent codecov/project to 54% and 69% while codecov/patch was 100%. A narrowed run now skips coverage, like the existing path for changes with no Python source. Codecov then gets no partial report and posts no project status, matching PRs that Quick Feedback defers entirely. Co-Authored-By: Claude Opus 5.5 --- .../selective-coverage-narrowed.fixed.md | 1 + .../tests/core/test_run_selective_tests.py | 84 +++++++++++++++++++ policyengine_us/tests/run_selective_tests.py | 16 ++++ 3 files changed, 101 insertions(+) create mode 100644 changelog.d/selective-coverage-narrowed.fixed.md diff --git a/changelog.d/selective-coverage-narrowed.fixed.md b/changelog.d/selective-coverage-narrowed.fixed.md new file mode 100644 index 00000000000..f437307f76d --- /dev/null +++ b/changelog.d/selective-coverage-narrowed.fixed.md @@ -0,0 +1 @@ +Skip coverage in Quick Feedback when a broad change is narrowed to its directly changed tests, so codecov/project no longer reports the changed source files' untested lines as a coverage drop. diff --git a/policyengine_us/tests/core/test_run_selective_tests.py b/policyengine_us/tests/core/test_run_selective_tests.py index 34a7ab6eb38..e4a3963ed01 100644 --- a/policyengine_us/tests/core/test_run_selective_tests.py +++ b/policyengine_us/tests/core/test_run_selective_tests.py @@ -242,3 +242,87 @@ def test_real_failures_still_fail_beside_an_empty_path(monkeypatch): _fake_pytest({"tests/a_test.py": 1, "tests/populace_fixture.py": 5}), ) assert runner.run_tests({"tests/a_test.py", "tests/populace_fixture.py"}) == 1 + + +def _recording_pytest(commands): + def run(cmd, *args, **kwargs): + commands.append(cmd) + return SimpleNamespace(returncode=0) + + return run + + +BROAD_CHANGE = { + "policyengine_us/tests/policy/baseline/gov/irs/payroll/employer_payroll_tax.yaml", + "policyengine_us/variables/gov/irs/tax/payroll/employer_payroll_tax.py", + "policyengine_us/variables/gov/states/ca/tax/payroll/unemployment/ca_employer_state_unemployment_tax.py", + "policyengine_us/variables/gov/states/tx/tax/payroll/unemployment/tx_employer_state_unemployment_tax.py", +} + + +def test_narrowed_scope_runs_without_coverage(monkeypatch): + """A narrowed run must not upload a partial report on the changed sources. + + Regression for PRs #9764 and #9766: a broad reference-only change ran + only its directly changed tests, yet coverage still measured every + changed variable file, so codecov/project fell to 54% and 69%. + """ + runner = SelectiveTestRunner() + runner.max_test_targets = 1 + runner.max_test_files = 1 + commands = [] + monkeypatch.setattr( + run_selective_tests.subprocess, "run", _recording_pytest(commands) + ) + + limited_paths = runner.limit_test_paths( + runner.map_files_to_tests(BROAD_CHANGE), BROAD_CHANGE + ) + + assert runner.scope_narrowed + assert ( + runner.run_tests(limited_paths, with_coverage=True, changed_files=BROAD_CHANGE) + == 0 + ) + assert commands + assert all("coverage" not in cmd for cmd in commands) + + +def test_unnarrowed_scope_keeps_coverage_of_changed_sources(monkeypatch): + runner = SelectiveTestRunner() + commands = [] + monkeypatch.setattr( + run_selective_tests.subprocess, "run", _recording_pytest(commands) + ) + changed_files = { + "policyengine_us/tests/policy/baseline/gov/irs/payroll/employer_payroll_tax.yaml", + "policyengine_us/variables/gov/irs/tax/payroll/employer_payroll_tax.py", + } + + limited_paths = runner.limit_test_paths( + runner.map_files_to_tests(changed_files), changed_files + ) + + assert not runner.scope_narrowed + runner.run_tests(limited_paths, with_coverage=True, changed_files=changed_files) + assert commands + for cmd in commands: + assert cmd[1:4] == ["-m", "coverage", "run"] + include = cmd[cmd.index("--include") + 1] + assert include == ( + "policyengine_us/variables/gov/irs/tax/payroll/employer_payroll_tax.py" + ) + + +def test_scope_narrowed_resets_on_each_limit_call(): + runner = SelectiveTestRunner() + runner.max_test_targets = 1 + runner.max_test_files = 1 + runner.limit_test_paths(runner.map_files_to_tests(BROAD_CHANGE), BROAD_CHANGE) + assert runner.scope_narrowed + + runner.max_test_targets = 25 + runner.max_test_files = 100 + test_file = "policyengine_us/tests/policy/baseline/gov/irs/payroll/employer_payroll_tax.yaml" + runner.limit_test_paths({test_file}, {test_file}) + assert not runner.scope_narrowed diff --git a/policyengine_us/tests/run_selective_tests.py b/policyengine_us/tests/run_selective_tests.py index d293a564429..052ad06c2b1 100644 --- a/policyengine_us/tests/run_selective_tests.py +++ b/policyengine_us/tests/run_selective_tests.py @@ -71,6 +71,10 @@ def __init__(self, base_branch: str = "master"): self.max_direct_test_files = int( os.environ.get("SELECTIVE_TEST_MAX_DIRECT_FILES", "8") ) + # Set by limit_test_paths when a broad change is narrowed to the + # directly changed tests. Those tests need not exercise the changed + # source files, so run_tests then skips coverage. + self.scope_narrowed = False # Define regex patterns for matching files to tests # Paths that contain only aggregation lists and should not @@ -383,6 +387,7 @@ def get_changed_python_coverage_patterns(changed_files: Set[str]) -> List[str]: def limit_test_paths( self, test_paths: Set[str], changed_files: Set[str] ) -> Set[str]: + self.scope_narrowed = False deferred_paths = { path for path in test_paths if is_quick_feedback_deferred(path) } @@ -423,6 +428,7 @@ def limit_test_paths( "Running only directly changed tests and explicit file targets; " "the full suite jobs provide exhaustive coverage." ) + self.scope_narrowed = True return bounded_test_paths print( @@ -448,6 +454,16 @@ def run_tests( print(f" - {path}") # Construct pytest command + if with_coverage and self.scope_narrowed: + # The narrowed run measures the changed source files only through + # whichever tests happened to change, so its report would show + # their untouched lines as uncovered and fail codecov/project. + print( + "Selective scope was narrowed to directly changed tests; running " + "tests without coverage." + ) + with_coverage = False + if with_coverage: # Only track coverage for the specific files that changed in the PR include_patterns = [] From e905bdb4db95f8de85903c33fd0e2d7270419228 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 3 Oct 2026 17:21:11 -0400 Subject: [PATCH 2/2] Leave only the untested changed sources out of coverage Narrowing is one of two ways Quick Feedback drops a changed file's tests; deferring slow directories (gov/ssa, household, reform, ...) is the other, and it produced the same partial report. Replace the narrowed-scope flag with a per-file rule: a changed source is measured unless it maps to tests and none of them are in this run. Files with no mapped tests keep today's behaviour. Co-Authored-By: Claude Opus 5.5 --- .../selective-coverage-narrowed.fixed.md | 2 +- .../tests/core/test_run_selective_tests.py | 111 +++++++++++------- policyengine_us/tests/run_selective_tests.py | 53 ++++++--- 3 files changed, 107 insertions(+), 59 deletions(-) diff --git a/changelog.d/selective-coverage-narrowed.fixed.md b/changelog.d/selective-coverage-narrowed.fixed.md index f437307f76d..545c51e7923 100644 --- a/changelog.d/selective-coverage-narrowed.fixed.md +++ b/changelog.d/selective-coverage-narrowed.fixed.md @@ -1 +1 @@ -Skip coverage in Quick Feedback when a broad change is narrowed to its directly changed tests, so codecov/project no longer reports the changed source files' untested lines as a coverage drop. +Leave changed source files out of Quick Feedback coverage when none of their mapped tests run there (deferred slow directories or a broad change narrowed to its changed tests), so codecov/project no longer counts their untouched lines as a coverage drop. diff --git a/policyengine_us/tests/core/test_run_selective_tests.py b/policyengine_us/tests/core/test_run_selective_tests.py index e4a3963ed01..1c341f8d5e0 100644 --- a/policyengine_us/tests/core/test_run_selective_tests.py +++ b/policyengine_us/tests/core/test_run_selective_tests.py @@ -252,20 +252,37 @@ def run(cmd, *args, **kwargs): return run -BROAD_CHANGE = { - "policyengine_us/tests/policy/baseline/gov/irs/payroll/employer_payroll_tax.yaml", - "policyengine_us/variables/gov/irs/tax/payroll/employer_payroll_tax.py", - "policyengine_us/variables/gov/states/ca/tax/payroll/unemployment/ca_employer_state_unemployment_tax.py", - "policyengine_us/variables/gov/states/tx/tax/payroll/unemployment/tx_employer_state_unemployment_tax.py", -} +def _coverage_includes(commands): + """The --include list of each coverage command, or None without coverage.""" + return [ + cmd[cmd.index("--include") + 1].split(",") if "coverage" in cmd else None + for cmd in commands + ] + + +PAYROLL_TEST = ( + "policyengine_us/tests/policy/baseline/gov/irs/payroll/employer_payroll_tax.yaml" +) +PAYROLL_SOURCE = "policyengine_us/variables/gov/irs/tax/payroll/employer_payroll_tax.py" +SSA_SOURCE = "policyengine_us/variables/gov/ssa/ss/social_security_retirement.py" +UNMAPPED_SOURCE = ( + "policyengine_us/tools/geography/download_50_state_census_block_data.py" +) -def test_narrowed_scope_runs_without_coverage(monkeypatch): - """A narrowed run must not upload a partial report on the changed sources. +def _unbounded_runner(): + runner = SelectiveTestRunner() + runner.max_test_targets = 10**6 + runner.max_test_files = 10**6 + return runner + - Regression for PRs #9764 and #9766: a broad reference-only change ran - only its directly changed tests, yet coverage still measured every - changed variable file, so codecov/project fell to 54% and 69%. +def test_narrowed_scope_leaves_untested_sources_out_of_coverage(monkeypatch): + """A narrowed run must not report the changed sources as uncovered. + + Regression for PRs #9764 and #9766: broad reference-only changes ran + only their directly changed tests, yet coverage measured every changed + variable file, so codecov/project fell to 54% and 69%. """ runner = SelectiveTestRunner() runner.max_test_targets = 1 @@ -274,55 +291,69 @@ def test_narrowed_scope_runs_without_coverage(monkeypatch): monkeypatch.setattr( run_selective_tests.subprocess, "run", _recording_pytest(commands) ) + changed_files = { + PAYROLL_TEST, + PAYROLL_SOURCE, + "policyengine_us/variables/gov/states/ca/tax/payroll/unemployment/ca_employer_state_unemployment_tax.py", + "policyengine_us/variables/gov/states/tx/tax/payroll/unemployment/tx_employer_state_unemployment_tax.py", + } limited_paths = runner.limit_test_paths( - runner.map_files_to_tests(BROAD_CHANGE), BROAD_CHANGE + runner.map_files_to_tests(changed_files), changed_files ) - assert runner.scope_narrowed + assert limited_paths == {PAYROLL_TEST} assert ( - runner.run_tests(limited_paths, with_coverage=True, changed_files=BROAD_CHANGE) + runner.run_tests(limited_paths, with_coverage=True, changed_files=changed_files) == 0 ) - assert commands - assert all("coverage" not in cmd for cmd in commands) + assert _coverage_includes(commands) == [None] -def test_unnarrowed_scope_keeps_coverage_of_changed_sources(monkeypatch): - runner = SelectiveTestRunner() +def test_deferred_directory_sources_are_left_out_of_coverage(monkeypatch): + runner = _unbounded_runner() commands = [] monkeypatch.setattr( run_selective_tests.subprocess, "run", _recording_pytest(commands) ) - changed_files = { - "policyengine_us/tests/policy/baseline/gov/irs/payroll/employer_payroll_tax.yaml", - "policyengine_us/variables/gov/irs/tax/payroll/employer_payroll_tax.py", - } + changed_files = {PAYROLL_TEST, PAYROLL_SOURCE, SSA_SOURCE} limited_paths = runner.limit_test_paths( runner.map_files_to_tests(changed_files), changed_files ) + runner.run_tests(limited_paths, with_coverage=True, changed_files=changed_files) + + assert "policyengine_us/tests/policy/baseline/gov/ssa" not in limited_paths + assert commands + assert all(include == [PAYROLL_SOURCE] for include in _coverage_includes(commands)) + + +def test_sources_with_running_or_no_mapped_tests_keep_coverage(monkeypatch): + runner = _unbounded_runner() + commands = [] + monkeypatch.setattr( + run_selective_tests.subprocess, "run", _recording_pytest(commands) + ) + changed_files = {PAYROLL_TEST, PAYROLL_SOURCE, UNMAPPED_SOURCE} - assert not runner.scope_narrowed + limited_paths = runner.limit_test_paths( + runner.map_files_to_tests(changed_files), changed_files + ) runner.run_tests(limited_paths, with_coverage=True, changed_files=changed_files) + assert commands - for cmd in commands: - assert cmd[1:4] == ["-m", "coverage", "run"] - include = cmd[cmd.index("--include") + 1] - assert include == ( - "policyengine_us/variables/gov/irs/tax/payroll/employer_payroll_tax.py" - ) + for include in _coverage_includes(commands): + assert sorted(include) == sorted([PAYROLL_SOURCE, UNMAPPED_SOURCE]) -def test_scope_narrowed_resets_on_each_limit_call(): +def test_get_untested_sources_requires_some_mapped_test_to_run(): runner = SelectiveTestRunner() - runner.max_test_targets = 1 - runner.max_test_files = 1 - runner.limit_test_paths(runner.map_files_to_tests(BROAD_CHANGE), BROAD_CHANGE) - assert runner.scope_narrowed - - runner.max_test_targets = 25 - runner.max_test_files = 100 - test_file = "policyengine_us/tests/policy/baseline/gov/irs/payroll/employer_payroll_tax.yaml" - runner.limit_test_paths({test_file}, {test_file}) - assert not runner.scope_narrowed + sources = [PAYROLL_SOURCE, SSA_SOURCE, UNMAPPED_SOURCE] + + assert runner.get_untested_sources(sources, set()) == [ + PAYROLL_SOURCE, + SSA_SOURCE, + ] + assert runner.get_untested_sources( + sources, {"policyengine_us/tests/policy/baseline/gov/irs"} + ) == [SSA_SOURCE] diff --git a/policyengine_us/tests/run_selective_tests.py b/policyengine_us/tests/run_selective_tests.py index 052ad06c2b1..21308c04d0d 100644 --- a/policyengine_us/tests/run_selective_tests.py +++ b/policyengine_us/tests/run_selective_tests.py @@ -71,10 +71,6 @@ def __init__(self, base_branch: str = "master"): self.max_direct_test_files = int( os.environ.get("SELECTIVE_TEST_MAX_DIRECT_FILES", "8") ) - # Set by limit_test_paths when a broad change is narrowed to the - # directly changed tests. Those tests need not exercise the changed - # source files, so run_tests then skips coverage. - self.scope_narrowed = False # Define regex patterns for matching files to tests # Paths that contain only aggregation lists and should not @@ -384,10 +380,25 @@ def get_changed_python_coverage_patterns(changed_files: Set[str]) -> List[str]: if file.endswith(".py") and "/tests/" not in file ] + def get_untested_sources( + self, source_files: List[str], test_paths: Set[str] + ) -> List[str]: + """Source files that map to tests, none of which are in this run. + + limit_test_paths defers slow directories and narrows broad changes + to their directly changed tests, so some files' tests run only in + the full suite. Coverage of those files here would count their + untouched lines as uncovered. + """ + return [ + file + for file in source_files + if (mapped := self.map_files_to_tests({file})) and not mapped & test_paths + ] + def limit_test_paths( self, test_paths: Set[str], changed_files: Set[str] ) -> Set[str]: - self.scope_narrowed = False deferred_paths = { path for path in test_paths if is_quick_feedback_deferred(path) } @@ -428,7 +439,6 @@ def limit_test_paths( "Running only directly changed tests and explicit file targets; " "the full suite jobs provide exhaustive coverage." ) - self.scope_narrowed = True return bounded_test_paths print( @@ -454,16 +464,6 @@ def run_tests( print(f" - {path}") # Construct pytest command - if with_coverage and self.scope_narrowed: - # The narrowed run measures the changed source files only through - # whichever tests happened to change, so its report would show - # their untouched lines as uncovered and fail codecov/project. - print( - "Selective scope was narrowed to directly changed tests; running " - "tests without coverage." - ) - with_coverage = False - if with_coverage: # Only track coverage for the specific files that changed in the PR include_patterns = [] @@ -472,8 +472,25 @@ def run_tests( changed_py_files = self.get_changed_python_coverage_patterns( changed_files ) - if changed_py_files: - include_patterns = changed_py_files + untested = self.get_untested_sources(changed_py_files, test_paths) + if untested: + print( + f"Leaving {len(untested)} changed source file(s) out of " + "coverage; their tests run only in the full suite:" + ) + for file in sorted(untested): + print(f" - {file}") + tested_py_files = [ + file for file in changed_py_files if file not in untested + ] + if tested_py_files: + include_patterns = tested_py_files + elif untested: + print( + "No changed source file is tested in this run; running " + "tests without coverage." + ) + with_coverage = False else: print( "No changed Python source files; running tests without "