diff --git a/changelog.d/selective-coverage-narrowed.fixed.md b/changelog.d/selective-coverage-narrowed.fixed.md new file mode 100644 index 00000000000..545c51e7923 --- /dev/null +++ b/changelog.d/selective-coverage-narrowed.fixed.md @@ -0,0 +1 @@ +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 34a7ab6eb38..1c341f8d5e0 100644 --- a/policyengine_us/tests/core/test_run_selective_tests.py +++ b/policyengine_us/tests/core/test_run_selective_tests.py @@ -242,3 +242,118 @@ 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 + + +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 _unbounded_runner(): + runner = SelectiveTestRunner() + runner.max_test_targets = 10**6 + runner.max_test_files = 10**6 + return runner + + +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 + runner.max_test_files = 1 + commands = [] + 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(changed_files), changed_files + ) + + assert limited_paths == {PAYROLL_TEST} + assert ( + runner.run_tests(limited_paths, with_coverage=True, changed_files=changed_files) + == 0 + ) + assert _coverage_includes(commands) == [None] + + +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 = {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} + + 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 include in _coverage_includes(commands): + assert sorted(include) == sorted([PAYROLL_SOURCE, UNMAPPED_SOURCE]) + + +def test_get_untested_sources_requires_some_mapped_test_to_run(): + runner = SelectiveTestRunner() + 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 d293a564429..21308c04d0d 100644 --- a/policyengine_us/tests/run_selective_tests.py +++ b/policyengine_us/tests/run_selective_tests.py @@ -380,6 +380,22 @@ 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]: @@ -456,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 "