diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index a05fc24f19b..4917f0024b3 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -310,20 +310,23 @@ jobs: - name: Run Contrib Python tests if: ${{ matrix.group == 'other-shard-1' }} run: uv run --no-sync make test-policy-contrib-python - Rest: + RestPython: needs: ReleaseLock - name: Full Suite - Rest (Python + variables) + name: Full Suite - Rest (Python, shard-1 + variables) runs-on: ubuntu-latest - # Matches the PR Rest job (python-tests.yaml, raised to 90 in #9785). - # Oct 6-8 on push: p50 51 min, max success 58.8 min, and one run hit the - # old 60-minute limit (37719726248), which skipped that version's Publish. + # Historical group times, including variables: 33-46 minutes plus setup. timeout-minutes: 90 if: | (github.repository == 'PolicyEngine/policyengine-us') && (github.event.head_commit.message == 'Update PolicyEngine US') env: PYTHONUNBUFFERED: 1 - PYTEST_ADDOPTS: "--durations=25 -p no:unraisableexception -p no:threadexception" + OMP_NUM_THREADS: 4 + OPENBLAS_NUM_THREADS: 4 + MKL_NUM_THREADS: 4 + VECLIB_MAXIMUM_THREADS: 4 + NUMEXPR_NUM_THREADS: 4 + OMP_WAIT_POLICY: PASSIVE steps: - name: Checkout repo uses: actions/checkout@v6 @@ -342,30 +345,98 @@ jobs: - name: Turn off default branching shell: bash run: bash ./update_itemization.sh - # Each Rest group runs in a separate process to release its heap. - # SPM simulation/API contracts run in the Microsimulation job instead. + # Each group remains a separate, sequential process on its runner. + # SPM simulation/API contracts stay in the Microsimulation job. - name: Run remaining Python tests - # Continue after failures, but stop when the workflow is cancelled. if: ${{ !cancelled() }} env: PYTEST_ADDOPTS: "--durations=0 -v -p no:unraisableexception -p no:threadexception" - run: uv run --no-sync make test-other-python-rest REST_REPORT_DIR=. + run: uv run --no-sync make test-other-python-rest REST_PYTHON_SHARD=1 REST_REPORT_DIR=. + - name: Upload Rest Python timing and memory reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: rest-python-test-timings-shard-1 + path: | + rest-python-*.xml + rest-python-*-resources.txt - name: Run tests/variables YAML tests - # Continue after failures, but stop when the workflow is cancelled. if: ${{ !cancelled() }} env: PYTEST_ADDOPTS: "--durations=0 --junitxml=rest-variables.xml -p no:unraisableexception -p no:threadexception" run: /usr/bin/time -v -o rest-variables-resources.txt uv run --no-sync make test-yaml-variables - - name: Upload Rest timing and memory reports + - name: Upload Rest variables timing and memory reports if: always() uses: actions/upload-artifact@v4 with: - name: rest-test-timings + name: rest-variables-test-timings path: | - rest-python-*.xml rest-variables.xml - rest-python-*-resources.txt rest-variables-resources.txt + Rest: + needs: [RestPython] + name: Full Suite - Rest (Python + variables) + runs-on: ubuntu-latest + # Historical group times: 32-46 minutes plus setup. + # This runner also retains the legacy summary check, keeping PRs at 27 runners. + timeout-minutes: 90 + if: | + always() + && (github.repository == 'PolicyEngine/policyengine-us') + && (github.event.head_commit.message == 'Update PolicyEngine US') + env: + PYTHONUNBUFFERED: 1 + OMP_NUM_THREADS: 4 + OPENBLAS_NUM_THREADS: 4 + MKL_NUM_THREADS: 4 + VECLIB_MAXIMUM_THREADS: 4 + NUMEXPR_NUM_THREADS: 4 + OMP_WAIT_POLICY: PASSIVE + steps: + - name: Checkout repo + uses: actions/checkout@v6 + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: 3.14 + - name: Install uv + uses: astral-sh/setup-uv@v8.1.0 + with: + version: "0.12.13" + - name: Resolve latest compatible PolicyEngine Core + run: uv lock --upgrade-package policyengine-core + - name: Install dependencies + run: uv sync --locked --extra dev + - name: Turn off default branching + shell: bash + run: bash ./update_itemization.sh + # Each group remains a separate, sequential process on its runner. + # SPM simulation/API contracts stay in the Microsimulation job. + - name: Run remaining Python tests + id: python_tests + if: ${{ !cancelled() }} + env: + PYTEST_ADDOPTS: "--durations=0 -v -p no:unraisableexception -p no:threadexception" + run: uv run --no-sync make test-other-python-rest REST_PYTHON_SHARD=2 REST_REPORT_DIR=. + - name: Upload Rest Python timing and memory reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: rest-python-test-timings-shard-2 + path: | + rest-python-*.xml + rest-python-*-resources.txt + - name: Check all Rest jobs succeeded + if: always() + shell: bash + env: + PYTHON_RESULT: ${{ needs.RestPython.result }} + SHARD_RESULT: ${{ steps.python_tests.outcome }} + run: | + if [[ "$PYTHON_RESULT" != "success" || "$SHARD_RESULT" != "success" ]]; then + echo "::error::Rest shards did not all succeed: shard-1=$PYTHON_RESULT, shard-2=$SHARD_RESULT" + exit 1 + fi Microsimulation: needs: ReleaseLock name: Full Suite - Microsimulation @@ -423,7 +494,7 @@ jobs: if: | (github.repository == 'PolicyEngine/policyengine-us') && (github.event.head_commit.message == 'Update PolicyEngine US') - needs: [ReleaseLock, ReleaseLockTests, Baseline, HouseholdAPIPartners, Contrib, Rest, Microsimulation] + needs: [ReleaseLock, ReleaseLockTests, Baseline, HouseholdAPIPartners, Contrib, RestPython, Rest, Microsimulation] # PyPI trusted publishing: PyPI accepts uploads only from this workflow in # the pypi environment, which is limited to main. No stored PyPI token. environment: pypi diff --git a/.github/workflows/python-tests.yaml b/.github/workflows/python-tests.yaml index 034aa44b16d..7d22abd7b6d 100644 --- a/.github/workflows/python-tests.yaml +++ b/.github/workflows/python-tests.yaml @@ -1,19 +1,23 @@ name: Python tests on: workflow_call: -# Group these jobs in the Actions view while retaining two independent runners. +# Two sequential Rest runners keep each shard below its timeout. +# Shard 1 includes variables; shard 2 also supplies the legacy summary check. # Only the guarded PR workflow calls this file; it has no automatic triggers. jobs: - Rest: - name: Full Suite - Rest (Python + variables) + RestPython: + name: Full Suite - Rest (Python, shard-1 + variables) runs-on: ubuntu-latest - # Passing runs on 2026-10-04 took 42-55 minutes (#9653, #9766, #9769, - # #9777, #9784), so a 60-minute limit cancels PRs that add a few - # minutes of tests (#9695 was cancelled at 60 minutes twice). + # Historical group times, including variables: 33-46 minutes plus setup. timeout-minutes: 90 env: PYTHONUNBUFFERED: 1 - PYTEST_ADDOPTS: "--durations=25 -p no:unraisableexception -p no:threadexception" + OMP_NUM_THREADS: 4 + OPENBLAS_NUM_THREADS: 4 + MKL_NUM_THREADS: 4 + VECLIB_MAXIMUM_THREADS: 4 + NUMEXPR_NUM_THREADS: 4 + OMP_WAIT_POLICY: PASSIVE steps: - name: Checkout repo uses: actions/checkout@v6 @@ -32,30 +36,95 @@ jobs: - name: Turn off default branching shell: bash run: bash ./update_itemization.sh - # Each Rest group runs in a separate process to release its heap. - # SPM simulation/API contracts run in the Microsimulation job instead. + # Each group remains a separate, sequential process on its runner. + # SPM simulation/API contracts stay in the Microsimulation job. - name: Run remaining Python tests - # Continue after failures, but stop when the workflow is cancelled. if: ${{ !cancelled() }} env: PYTEST_ADDOPTS: "--durations=0 -v -p no:unraisableexception -p no:threadexception" - run: uv run --no-sync make test-other-python-rest REST_REPORT_DIR=. + run: uv run --no-sync make test-other-python-rest REST_PYTHON_SHARD=1 REST_REPORT_DIR=. + - name: Upload Rest Python timing and memory reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: rest-python-test-timings-shard-1 + path: | + rest-python-*.xml + rest-python-*-resources.txt - name: Run tests/variables YAML tests - # Continue after failures, but stop when the workflow is cancelled. if: ${{ !cancelled() }} env: PYTEST_ADDOPTS: "--durations=0 --junitxml=rest-variables.xml -p no:unraisableexception -p no:threadexception" run: /usr/bin/time -v -o rest-variables-resources.txt uv run --no-sync make test-yaml-variables - - name: Upload Rest timing and memory reports + - name: Upload Rest variables timing and memory reports if: always() uses: actions/upload-artifact@v4 with: - name: rest-test-timings + name: rest-variables-test-timings path: | - rest-python-*.xml rest-variables.xml - rest-python-*-resources.txt rest-variables-resources.txt + Rest: + needs: [RestPython] + if: ${{ always() }} + name: Full Suite - Rest (Python + variables) + runs-on: ubuntu-latest + # Historical group times: 32-46 minutes plus setup. + # This runner also retains the legacy summary check, keeping PRs at 27 runners. + timeout-minutes: 90 + env: + PYTHONUNBUFFERED: 1 + OMP_NUM_THREADS: 4 + OPENBLAS_NUM_THREADS: 4 + MKL_NUM_THREADS: 4 + VECLIB_MAXIMUM_THREADS: 4 + NUMEXPR_NUM_THREADS: 4 + OMP_WAIT_POLICY: PASSIVE + steps: + - name: Checkout repo + uses: actions/checkout@v6 + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: 3.14 + - name: Install uv + uses: astral-sh/setup-uv@v8.1.0 + with: + version: "0.12.13" + - name: Resolve latest compatible PolicyEngine Core + run: uv lock --upgrade-package policyengine-core + - name: Install dependencies + run: uv sync --locked --extra dev + - name: Turn off default branching + shell: bash + run: bash ./update_itemization.sh + # Each group remains a separate, sequential process on its runner. + # SPM simulation/API contracts stay in the Microsimulation job. + - name: Run remaining Python tests + id: python_tests + if: ${{ !cancelled() }} + env: + PYTEST_ADDOPTS: "--durations=0 -v -p no:unraisableexception -p no:threadexception" + run: uv run --no-sync make test-other-python-rest REST_PYTHON_SHARD=2 REST_REPORT_DIR=. + - name: Upload Rest Python timing and memory reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: rest-python-test-timings-shard-2 + path: | + rest-python-*.xml + rest-python-*-resources.txt + - name: Check all Rest jobs succeeded + if: always() + shell: bash + env: + PYTHON_RESULT: ${{ needs.RestPython.result }} + SHARD_RESULT: ${{ steps.python_tests.outcome }} + run: | + if [[ "$PYTHON_RESULT" != "success" || "$SHARD_RESULT" != "success" ]]; then + echo "::error::Rest shards did not all succeed: shard-1=$PYTHON_RESULT, shard-2=$SHARD_RESULT" + exit 1 + fi Microsimulation: name: Full Suite - Microsimulation runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index 481a6cdb1ff..842b3d70fb2 100644 --- a/Makefile +++ b/Makefile @@ -43,8 +43,25 @@ REST_SPM_TESTS := $(TESTS)/core/test_spm_policy_family.py \ # in its grid fixture. Core reached 13.3 GB in CI run 38141803955 after this # and three smaller modules arrived; isolate SSI rather than filling heavy-a # (8.1 GB including the pending capital-gains MTR change). These groups remain -# sequential subprocesses on the existing Rest runner. +# sequential subprocesses within each Rest Python shard. REST_PYTHON_GROUPS := cached-arrays heavy-a heavy-b core-heavy core policy-parameters policy remaining-heavy remaining +# CI runs two shards of these existing isolated processes. Across seven +# recent runs (including 38326922385 and 38289790999), their summed group +# times were 32-45 and 32-46 minutes; variables adds about a minute to shard 1. +# Both stay near 45 minutes, while the old serial job hit its 90-minute limit. +# No shard runs more than one Python group at a time. Locally, leaving +# REST_PYTHON_SHARD unset still runs every group in the original order. +REST_PYTHON_SHARD_1_GROUPS := cached-arrays core-heavy policy-parameters remaining-heavy remaining +REST_PYTHON_SHARD_2_GROUPS := heavy-a heavy-b core policy +REST_PYTHON_SHARD ?= +ifeq ($(REST_PYTHON_SHARD),) +REST_PYTHON_SELECTED_GROUPS := $(REST_PYTHON_GROUPS) +else +REST_PYTHON_SELECTED_GROUPS := $(REST_PYTHON_SHARD_$(REST_PYTHON_SHARD)_GROUPS) +ifeq ($(strip $(REST_PYTHON_SELECTED_GROUPS)),) +$(error REST_PYTHON_SHARD must be 1 or 2) +endif +endif REST_CACHED_ARRAY_TESTS := $(TESTS)/test_formulas_do_not_write_into_cached_arrays.py REST_HEAVY_A_TESTS := $(TESTS)/test_md_poverty_line_credit_invariants.py \ $(TESTS)/core/test_ald_determinism.py \ @@ -281,7 +298,7 @@ test-other-python-spm: pytest $(REST_SPM_TESTS) --maxfail=0 # Every group runs even after one fails; the target fails if any group did. test-other-python-rest: - @failed=""; for group in $(REST_PYTHON_GROUPS); do \ + @failed=""; for group in $(REST_PYTHON_SELECTED_GROUPS); do \ echo "=== Rest Python group: $$group ==="; \ $(MAKE) --no-print-directory test-other-python-rest-$$group || failed="$$failed $$group"; \ done; \ diff --git a/changelog.d/ci-rest-parallel-headroom.fixed.md b/changelog.d/ci-rest-parallel-headroom.fixed.md new file mode 100644 index 00000000000..b57fee959de --- /dev/null +++ b/changelog.d/ci-rest-parallel-headroom.fixed.md @@ -0,0 +1 @@ +Split the Rest CI suite into two sequential Python shards, folding variables YAML and the legacy aggregate check into those runners to avoid the 90-minute timeout while increasing the PR runner budget from 26 to 27. Retain isolated test processes and timing reports. diff --git a/policyengine_us/tests/code_health/test_pr_runner_budget.py b/policyengine_us/tests/code_health/test_pr_runner_budget.py index ef6f6197167..48fcb490cbb 100644 --- a/policyengine_us/tests/code_health/test_pr_runner_budget.py +++ b/policyengine_us/tests/code_health/test_pr_runner_budget.py @@ -43,13 +43,17 @@ def test_pr_runner_budget_keeps_the_heavy_suites_separate(): counts[name] = len(matrix["include"]) else: counts[name] = math.prod(len(values) for values in matrix.values()) - assert sum(counts.values()) == 26 + # Rest hit the 90-minute timeout on #10024, #9913, #10139 and #10029 + # in October 2026. One extra runner buys about 45 minutes of headroom: + # two tested shards share variables and the legacy summary check. + assert sum(counts.values()) == 27 assert { name: counts[name] - for name in ("Baseline", "Contrib", "Rest", "Microsimulation") + for name in ("Baseline", "Contrib", "RestPython", "Rest", "Microsimulation") } == { "Baseline": 11, "Contrib": 9, + "RestPython": 1, "Rest": 1, "Microsimulation": 1, } @@ -71,16 +75,21 @@ def test_suite_groups_have_no_extra_runners_or_triggers(): # PyYAML parses the bare `on` key as True. No push/PR trigger here: # the draft guard must control the only route to these runners. assert GROUP_WORKFLOWS[name][True] == {"workflow_call": None} - assert set(PYTHON_JOBS) == {"Rest", "Microsimulation"} + assert set(PYTHON_JOBS) == {"RestPython", "Rest", "Microsimulation"} assert {name: job["timeout-minutes"] for name, job in PYTHON_JOBS.items()} == { + "RestPython": 90, "Rest": 90, "Microsimulation": 60, } - for job in PYTHON_JOBS.values(): + for name, job in PYTHON_JOBS.items(): assert job["runs-on"] == "ubuntu-latest" - assert not job.get("needs") + assert job.get("needs", []) == (["RestPython"] if name == "Rest" else []) assert not job.get("strategy") assert not job.get("continue-on-error") + # The second tested runner also checks the first runner's result. There + # is no third runner just for the summary, or a variables-only runner. + assert PYTHON_JOBS["Rest"]["if"] == "${{ always() }}" + assert PYTHON_JOBS["Rest"]["name"] == "Full Suite - Rest (Python + variables)" def test_sequential_variants_preserve_failures_and_allow_later_results(): diff --git a/policyengine_us/tests/code_health/test_rest_python_groups.py b/policyengine_us/tests/code_health/test_rest_python_groups.py index da2a2045a57..f76b4e2818b 100644 --- a/policyengine_us/tests/code_health/test_rest_python_groups.py +++ b/policyengine_us/tests/code_health/test_rest_python_groups.py @@ -1,18 +1,23 @@ -"""Each remaining Python test file runs in exactly one Rest group. +"""Each remaining Python test file runs in exactly one Rest group and shard. The Rest CI job used to run every Python test outside contrib, microsimulation and the SPM files in one pytest process. That process peaked at 15.7 GB RSS on the 16 GB runner (CI run 37198184683), and later runs lost the runner partway through the step. `make test-other-python-rest` now runs the same files as one -pytest process per group in REST_PYTHON_GROUPS. +pytest process per group in REST_PYTHON_GROUPS. CI assigns those groups to +two Python shards on separate runners, with the variables YAML in the first +shard. The second shard retains the original Rest check and waits for the +first shard so it can fail if either shard fails. Paths and --ignore flags can drop a file, or run it twice, while every test -still passes. So this guard takes each command from `make -n`, asks pytest -which files it collects, and checks that the groups together collect each file -of the old single command exactly once. A new test file lands in core or -policy when it sits under those folders and in remaining otherwise; the check -fails if a change to the groups leaves one out. Running the target with a stub -pytest checks that every group runs and that any failure fails the target. +still passes. So this guard expands the workflows' shard commands through +`make -n`, asks pytest which files each group collects, and checks that each +file of the old single command runs exactly once. A new test file lands in +core or policy when it sits under those folders and in remaining otherwise; +the check fails if a change to the groups or shards leaves one out. Running +the target with a stub pytest checks that every group runs and that any +failure fails the target. The original Rest check aggregates all shard +results, and must fail on failed, cancelled or skipped jobs. pytest lists the files here through its own discovery (paths, --ignore, python_files), but nothing is imported: running this file as a script swaps @@ -42,14 +47,20 @@ WORKFLOWS = [ REPO / ".github" / "workflows" / name for name in ("python-tests.yaml", "push.yaml") ] -# CI runs this file under `make test-other-python-rest REST_REPORT_DIR=.`, and -# that make exports these. Passed on, they would carry its flags and report -# directory into the make runs below, and the stub run would overwrite the -# CI groups' GNU time reports. -PARENT_MAKE_VARIABLES = ("MAKEFLAGS", "MFLAGS", "MAKELEVEL", "REST_REPORT_DIR") - - -def dry_run(target, *overrides): +# CI runs this file under a shard of `make test-other-python-rest`, and that +# make exports these. Passed on, they would restrict the guard's reference to +# one shard, or carry its report directory into the stub run and overwrite +# the CI groups' GNU time reports. +PARENT_MAKE_VARIABLES = ( + "MAKEFLAGS", + "MFLAGS", + "MAKELEVEL", + "REST_REPORT_DIR", + "REST_PYTHON_SHARD", +) + + +def dry_run(target, *overrides, prefixes=("pytest ", "/usr/bin/time ")): """The commands `make ` runs, each split into its arguments.""" if shutil.which("make") is None: pytest.skip("make is not installed") @@ -65,7 +76,7 @@ def dry_run(target, *overrides): return [ shlex.split(line) for line in result.stdout.splitlines() - if line.startswith(("pytest ", "/usr/bin/time ")) + if line.startswith(prefixes) ] @@ -113,41 +124,80 @@ def partition_problems(reference, groups): return problems -def rest_steps(workflow): - return yaml.safe_load(workflow.read_text())["jobs"]["Rest"]["steps"] +def workflow_jobs(workflow): + return yaml.safe_load(workflow.read_text())["jobs"] -def ci_report_dir(workflow): - """The REST_REPORT_DIR a workflow's Rest job passes to the groups.""" - runs = [ - step["run"] - for step in rest_steps(workflow) +def rest_steps(workflow, job="RestPython"): + return workflow_jobs(workflow)[job]["steps"] + + +def ci_python_shards(workflow): + """The real Make overrides and artifact configuration of every shard.""" + jobs = workflow_jobs(workflow) + calls = [ + name + for name, config in jobs.items() + for step in config.get("steps", []) if "make test-other-python-rest" in step.get("run", "") ] - assert len(runs) == 1, f"{workflow.name}: {len(runs)} steps run the groups" - match = re.search(r"REST_REPORT_DIR=(\S+)", runs[0]) - assert match, f"{workflow.name} runs the groups without REST_REPORT_DIR" - return match.group(1) + assert sorted(calls) == ["Rest", "RestPython"], (workflow.name, calls) + shards = {} + for job_name, selector in (("RestPython", "1"), ("Rest", "2")): + job = jobs[job_name] + assert "strategy" not in job, f"{workflow.name}: extra {job_name} runners" + (run,) = [ + step + for step in job["steps"] + if "make test-other-python-rest" in step.get("run", "") + ] + (upload,) = [ + step + for step in job["steps"] + if step.get("uses", "").startswith("actions/upload-artifact") + and "rest-python-*.xml" in step["with"]["path"].split() + ] + argv = shlex.split(run["run"]) + assert argv[:5] == ["uv", "run", "--no-sync", "make", "test-other-python-rest"] + selectors = [ + arg.removeprefix("REST_PYTHON_SHARD=") + for arg in argv[5:] + if arg.startswith("REST_PYTHON_SHARD=") + ] + assert selectors == [selector], (workflow.name, job_name, argv) + assert all(arg.startswith("REST_") and "=" in arg for arg in argv[5:]), argv + shards[f"shard-{selector}"] = { + "overrides": argv[5:], + "artifact": upload["with"]["name"], + "patterns": upload["with"]["path"].split(), + } + return shards -def ci_uploaded_patterns(workflow): - """The artifact paths a workflow's Rest job uploads.""" - uploads = [ - step - for step in rest_steps(workflow) - if step.get("uses", "").startswith("actions/upload-artifact") - ] - assert len(uploads) == 1, f"{workflow.name}: {len(uploads)} upload steps" - return uploads[0]["with"]["path"].split() +def ci_report_dir(workflow): + """The REST_REPORT_DIR each workflow shard passes to the groups.""" + dirs = [] + for shard in ci_python_shards(workflow).values(): + values = [ + arg.removeprefix("REST_REPORT_DIR=") + for arg in shard["overrides"] + if arg.startswith("REST_REPORT_DIR=") + ] + assert len(values) == 1, f"{workflow.name} runs without one REST_REPORT_DIR" + dirs.extend(values) + assert len(set(dirs)) == 1, dirs + return dirs[0] -def reported_groups(report_dir): +def reported_groups(report_dir, *overrides): """Each group's pytest arguments and report files, in run order. The group name comes from the JUnit file, rest-python-.xml. """ groups = {} - for argv in dry_run("test-other-python-rest", f"REST_REPORT_DIR={report_dir}"): + for argv in dry_run( + "test-other-python-rest", f"REST_REPORT_DIR={report_dir}", *overrides + ): assert argv[:3] == ["/usr/bin/time", "-v", "-o"], argv assert argv[4] == "pytest", argv junit = [arg for arg in argv[5:] if arg.startswith("--junitxml=")] @@ -195,35 +245,202 @@ def reference(old_command): return collected_files(old_command) -def test_groups_collect_each_file_of_the_old_single_process_once(groups, reference): +@pytest.fixture(scope="module") +def group_files(groups): + return {group: collected_files(spec["args"]) for group, spec in groups.items()} + + +def test_groups_collect_each_file_of_the_old_single_process_once( + group_files, reference +): # The reference is a real collection: it holds this file. assert Path(__file__).relative_to(REPO).as_posix() in reference - collected = {group: collected_files(spec["args"]) for group, spec in groups.items()} - problems = partition_problems(reference, collected) + problems = partition_problems(reference, group_files) assert not problems, "\n".join(problems) -def test_each_group_writes_reports_the_rest_job_uploads(groups, report_dir): - reports = [report for spec in groups.values() for report in spec["reports"]] - assert len(set(reports)) == len(reports), reports - for workflow in WORKFLOWS: - patterns = [os.path.normpath(p) for p in ci_uploaded_patterns(workflow)] +@pytest.mark.parametrize("workflow", WORKFLOWS, ids=lambda path: path.name) +def test_ci_shards_run_each_group_and_each_python_file_once( + workflow, groups, group_files, reference, report_dir +): + selected = {} + for shard, config in ci_python_shards(workflow).items(): + shard_groups = reported_groups(report_dir, *config["overrides"]) + assert shard_groups, f"{workflow.name}: {shard} runs no groups" + for group, spec in shard_groups.items(): + assert group in groups, f"{workflow.name}: unknown group {group}" + assert spec == groups[group], (workflow.name, shard, group, spec) + selected[f"{shard}/{group}"] = group_files[group] + problems = partition_problems(reference, selected) + assert not problems, f"{workflow.name}:\n" + "\n".join(problems) + + +@pytest.mark.parametrize("workflow", WORKFLOWS, ids=lambda path: path.name) +def test_each_group_writes_reports_its_ci_shard_uploads(workflow, report_dir): + artifacts = [] + reports = [] + for config in ci_python_shards(workflow).values(): + artifacts.append(config["artifact"]) + shard_reports = [ + report + for spec in reported_groups(report_dir, *config["overrides"]).values() + for report in spec["reports"] + ] + reports.extend(shard_reports) + patterns = [os.path.normpath(p) for p in config["patterns"]] missing = [ report - for report in reports + for report in shard_reports if not any(fnmatch(os.path.normpath(report), p) for p in patterns) ] assert not missing, f"{workflow.name} does not upload {missing}" - steps = yaml.safe_load(workflow.read_text())["jobs"]["Rest"]["steps"] - for name in ("Run remaining Python tests", "Run tests/variables YAML tests"): - (step,) = [step for step in steps if step.get("name") == name] - assert step["if"] == "${{ !cancelled() }}" + assert len(set(reports)) == len(reports), reports + variable_uploads = [ + step + for step in rest_steps(workflow, "RestPython") + if step.get("uses", "").startswith("actions/upload-artifact") + and "rest-variables.xml" in step["with"]["path"].split() + ] + assert len(variable_uploads) == 1 + artifacts.append(variable_uploads[0]["with"]["name"]) + assert len(set(artifacts)) == len(artifacts), artifacts + for job in ("RestPython", "Rest"): + steps = rest_steps(workflow, job) + (run,) = [ + step + for step in steps + if "make test-other-python-rest" in step.get("run", "") + ] + assert run["if"] == "${{ !cancelled() }}" (upload,) = [ step for step in steps if step.get("uses", "").startswith("actions/upload-artifact") + and "rest-python-*.xml" in step["with"]["path"].split() ] assert upload["if"] == "always()" + assert not any(step.get("continue-on-error", False) for step in steps) + + +@pytest.mark.parametrize("workflow", WORKFLOWS, ids=lambda path: path.name) +def test_variable_yaml_selection_runs_once_in_the_first_python_shard(workflow): + jobs = workflow_jobs(workflow) + calls = [ + (job, step) + for job, config in jobs.items() + for step in config.get("steps", []) + if "make test-yaml-variables" in step.get("run", "") + ] + assert len(calls) == 1, (workflow.name, calls) + job, step = calls[0] + assert job == "RestPython" + assert "strategy" not in jobs[job], "variables YAML must run exactly once" + assert step["if"] == "${{ !cancelled() }}" + assert shlex.split(step["run"]) == [ + "/usr/bin/time", + "-v", + "-o", + "rest-variables-resources.txt", + "uv", + "run", + "--no-sync", + "make", + "test-yaml-variables", + ] + (command,) = dry_run("test-yaml-variables", prefixes=("python ",)) + assert command == [ + "python", + "policyengine_us/tests/test_batched.py", + "policyengine_us/tests/variables", + "--batches", + "1", + ] + assert not any( + "make test-yaml-variables" in candidate.get("run", "") + for candidate in rest_steps(workflow, "Rest") + ) + (upload,) = [ + candidate + for candidate in jobs[job]["steps"] + if candidate.get("uses", "").startswith("actions/upload-artifact") + and "rest-variables.xml" in candidate["with"]["path"].split() + ] + assert upload["if"] == "always()" + junit = [ + arg.removeprefix("--junitxml=") + for arg in shlex.split(step["env"]["PYTEST_ADDOPTS"]) + if arg.startswith("--junitxml=") + ] + assert junit == ["rest-variables.xml"] + patterns = upload["with"]["path"].split() + assert all( + any(fnmatch(report, pattern) for pattern in patterns) + for report in [junit[0], "rest-variables-resources.txt"] + ) + + +@pytest.mark.parametrize("workflow", WORKFLOWS, ids=lambda path: path.name) +def test_original_rest_check_aggregates_all_shards_without_hiding_failure(workflow): + jobs = workflow_jobs(workflow) + summary = jobs["Rest"] + assert summary["name"] == "Full Suite - Rest (Python + variables)" + assert summary["needs"] == ["RestPython"] + assert "always()" in summary["if"] + assert "strategy" not in summary + assert summary["timeout-minutes"] == 90 + assert not summary.get("continue-on-error", False) + assert not jobs["RestPython"].get("continue-on-error", False) + if workflow.name == "push.yaml": + assert " ".join(summary["if"].split()) == ( + "always() && (github.repository == 'PolicyEngine/policyengine-us') " + "&& (github.event.head_commit.message == 'Update PolicyEngine US')" + ) + assert {"RestPython", "Rest"} <= set(jobs["Publish"]["needs"]) + else: + assert summary["if"] in ("always()", "${{ always() }}") + (step,) = [ + candidate + for candidate in summary["steps"] + if "PYTHON_RESULT" in candidate.get("env", {}) + ] + (python_step,) = [ + candidate + for candidate in summary["steps"] + if candidate.get("id") == "python_tests" + ] + assert "make test-other-python-rest" in python_step["run"] + assert "REST_PYTHON_SHARD=2" in shlex.split(python_step["run"]) + assert python_step["if"] == "${{ !cancelled() }}" + # This runner executes shard 2 before checking shard 1. Its own failures + # must remain failures, and cancellation skipping its Python step must + # fail the check even if shard 1 succeeded. + assert summary["steps"][-1] == step + assert step["if"] in ("always()", "${{ always() }}") + assert step["shell"] == "bash" + assert step["env"] == { + "PYTHON_RESULT": "${{ needs.RestPython.result }}", + "SHARD_RESULT": "${{ steps.python_tests.outcome }}", + } + assert not any( + candidate.get("continue-on-error", False) + for candidate in summary["steps"] + jobs["RestPython"]["steps"] + ) + for python in ("success", "failure", "cancelled", "skipped"): + for shard in ("success", "failure", "cancelled", "skipped"): + env = dict(os.environ, PYTHON_RESULT=python, SHARD_RESULT=shard) + result = subprocess.run( + ["bash", "-e", "-c", step["run"]], + cwd=REPO, + env=env, + capture_output=True, + text=True, + ) + assert (result.returncode == 0) == (python == shard == "success"), ( + workflow.name, + python, + shard, + result.stdout + result.stderr, + ) @pytest.mark.parametrize("workflow", WORKFLOWS, ids=lambda path: path.name) @@ -278,6 +495,29 @@ def test_groups_run_as_plain_pytest_without_a_report_dir(groups): assert [argv[1:] for argv in plain] == [spec["args"] for spec in groups.values()] +def test_inherited_ci_shard_does_not_restrict_the_reference(monkeypatch, groups): + monkeypatch.setenv("REST_PYTHON_SHARD", "2") + monkeypatch.setenv("REST_REPORT_DIR", "inherited-reports") + plain = dry_run("test-other-python-rest") + assert [argv[1:] for argv in plain] == [spec["args"] for spec in groups.values()] + + +@pytest.mark.parametrize("shard", ["0", "3", "unknown"]) +def test_invalid_shard_cannot_pass_without_running_any_groups(shard): + if shutil.which("make") is None: + pytest.skip("make is not installed") + env = {k: v for k, v in os.environ.items() if k not in PARENT_MAKE_VARIABLES} + result = subprocess.run( + ["make", "-n", "test-other-python-rest", f"REST_PYTHON_SHARD={shard}"], + cwd=REPO, + env=env, + capture_output=True, + text=True, + ) + assert result.returncode != 0 + assert "REST_PYTHON_SHARD must be 1 or 2" in result.stderr + + def import_name(file): """The module name pytest's default import mode gives a test file: its dotted path within its outermost package, or its stem outside one."""