Repository navigation
Run the beta integration suite on seven pytest-xdist workers - #983
Merged
Merged
Conversation
…kers Pin the beta pytest invocation in simulation-run-integ-tests.sh: seven pytest-xdist workers with --dist loadgroup and --durations, while the prod branch stays in one process. Check that the settings the integration tests read, including the minted access token, reach the pytest process as exported variables, which pytest-xdist workers inherit. In the integration suite, check that every test without beta_only shares the short_tests xdist group and that each beta_only calculation test is a work unit of its own, that the beta run has one worker per work unit, and that only one test makes a worker download the UK local-area dataset. Refs #980 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The beta deploy's integration step ran 35 tests in one process for about 17 minutes; six beta_only tests submit calculations and take almost all of it, the longest about 4.9 minutes. Run beta with -n 7 --dist loadgroup and --durations=0 --durations-min=1.0. The integration conftest puts every test without beta_only in one xdist_group, short_tests, so the suite is one work unit per calculation test plus one for the short tests. With one worker per unit, xdist gives each worker exactly one unit, so every calculation starts at once and none waits behind another on the same worker. Without groups, --dist load, worksteal and loadgroup each keep a second test queued on a busy worker, and in a scaled local trial they put two calculation tests on one worker in 3, 10 and 0 of 10 runs at eight workers. The prod branch, which no workflow runs, stays in one process. Fixes #980 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #980
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Changelog
changelog.d/<name>.<type>.md, whose type is breaking, added, changed, fixed or removed. (changelog.d/980.changed.md)Summary
The beta deploy's integration step ran the complete suite (35 tests) in one pytest process: 17.4 minutes median over 16 deploys (14.5 to 37.2). Six
beta_onlytests submit calculations to beta and take almost all of that time; the longest,test_calculate_default_model, takes about 4.9 minutes. This PR runs the beta suite on pytest-xdist workers so the six calculations run at the same time..github/scripts/simulation-run-integ-tests.sh, beta branch:uv run --frozen pytest tests/simulation/ -v -n 7 --dist loadgroup --durations=0 --durations-min=1.0. The worker count isBETA_PYTEST_WORKERS=7. pytest-xdist 3.8.0 is already in thetestextra anduv.lockofprojects/policyengine-apis-integ, which the script installs withuv sync --extra test --frozen.projects/policyengine-apis-integ/tests/simulation/conftest.py: apytest_collection_modifyitemshook puts every test in that directory withoutbeta_onlyinto onexdist_group,short_tests. Without--dist loadgroup(PR CI, the prod branch) the group has no effect.sim.contracts.full-integration-beta-only: prod passesrun_full_integration: false), and it runs no calculation tests.Worker count and distribution mode
-n 7 --dist loadgroup, with the 29 short tests in one group and each of the six calculation tests a work unit of its own. That is seven work units, so xdist gives each of the seven workers exactly one unit and then tells each worker to stop after it. Every calculation starts at once on its own worker, and none waits behind another. The 29 short tests run one after another on the seventh worker in seconds.Why not plain
-n 8 --dist load(the starting point),worksteal, orloadgroupwithout groups: xdist keeps a second test queued behind the running one on each worker, and that queued test cannot move to an idle worker (workstealnever takes a worker's running test or the one queued next). So two calculation tests can end up on one worker, one waiting minutes for the other. In a local trial that replaced each test with a sleep of its measured length (in seconds instead of minutes), in the suite's collection order, two calculation tests shared a worker in this many of 10 runs at eight workers:load3,worksteal10,loadgroupwithout groups 0 (and 1 of 4 in an earlier set). At six workers every mode did it in every run (4 of 4 each). With theshort_testsgroup, each calculation ran on its own worker in all 10 runs (5 at seven workers, 5 at eight), and a run took 6.0 to 6.4 seconds at seven workers against 17.8 serially.More than seven workers cannot shorten the step: there are only six calculation tests, and an eighth worker would have no work unit (xdist stops it at once). Fewer workers would queue a calculation behind another. The number of calculations that run at once on beta Modal is the number of calculation tests, six, for any count of seven or more.
test_the_beta_run_has_one_worker_per_work_unitfails if a seventhbeta_onlytest is added without raisingBETA_PYTEST_WORKERS, so that change, and its extra load on beta, is a decision someone makes.Are the tests safe to run in parallel?
tests/simulation/conftest.pyis function-scoped; there are no session- or module-scoped fixtures.settings = Settings()is built at import from environment variables in each worker process and only read.endpoint_calls.job_statusandtime.sleepwithmonkeypatch, which is per test and per process; each xdist worker is a separate process.GETthe fixed job IDsauth-smoke-probe-no-tokenandauth-smoke-probe-does-not-exist; nothing creates them. The calculation tests use job IDs the gateway assigns (Modal function call IDs). The stores they touch (Modal Dictssimulation-api-job-metadata,simulation-api-job-progress, and the budget-window job and seed Dicts) are keyed by those IDs, and Stage 12 shadow evaluation IDs are derived from them.test_calculate_us_state_region_modelandtest_calculate_specific_modelsend identical requests; nothing deduplicates or caches submissions by content, so they run as two independent jobs.Load on beta Modal
Containers per calculation, from the Modal app definitions (
projects/policyengine-simulation-executor/src/modal/app.py):test_calculate_default_model(US national, segmented)run_simulation+ 20run_simulation_segmenttest_calculate_us_state_region_model,test_calculate_specific_model(state/ut)run_simulationeachtest_calculate_uk_model,test_calculate_uk_local_area_breakdown(UK is never segmented)run_simulationeachtest_budget_window_multi_year_batch_completesrun_budget_window_batch+ up to 2run_simulationchildren (max_parallel: 2)At most 7 of
run_simulation'smax_containers=100, 20 ofrun_simulation_segment's 300, and 1 ofrun_budget_window_batch's 100 run at once, so no function limit in the repository would queue them. That is about 28 containers (about 217 vCPU and 870 GiB requested) at the peak, against 21 (168 vCPU) for the default-model test alone in the serial run. Workspace-level Modal limits are not in the repository.policyengine-simulation-gateway,web_app): nomax_containersand no@modal.concurrent, so each container serves one request at a time and Modal adds containers. The tests make at most seven requests at once (one per worker), polling every 5 seconds.OLD_GATEWAY_*_TIMEOUT_SECONDS).STAGE12_ENABLEDturns them on (its value was not read): the dispatch skips requests withbreakdownand those withouttime_period, so at most the twostate/utrequests qualify. Their v2 functions allow 10 containers each. A shadow run never changes the v1 response the tests check.timeout_in_millis = 1_500_000; the HTTP client timeout is the same), and each Modal function's timeout is 3600 seconds. Theintegrationjob sets notimeout-minutes. A test that waits for containers waits within its own 25-minute limit, about five times the longest measured test. A submission is not retried: a 5xx onPOST /simulate/economy/comparisonfails its test, while status polls retry 502, 503 and 504.Hugging Face downloads of
microcosm_uk_2024_25_dense.h5Parallel workers do not make more containers download the file:
policyengine bundle installdownloads only each country's default dataset (microcosm_uk_2024_25for the UK), so the default UK dataset is in the image.ensure_datasetsinto/tmp/policyengine-alternate-data/<hash>/of the container, only for a local-area breakdown or a constituency or local-authority region (the bundle'sregion_datasets).test_calculate_uk_local_area_breakdownsends such a request. It is one request, which runs in onerun_simulationcontainer (UK requests are never segmented).test_calculate_uk_modelis UK-wide and uses the default dataset. Stage 12 shadow dispatch skipsbreakdownrequests. US state datasets for 2025 to 2027 come from the image.So each deploy still downloads the file in one container, as before. What changes is when: at the start of the step, while the other calculations' containers start, instead of about 12 minutes in. The 2026-10-27 429s came from several runs downloading the file at once, and policyengine.py's download calls
raise_for_status()with no retry; this PR does not change either. Because the count stays at one, the PR does not group the UK tests.test_one_test_downloads_the_uk_local_area_datasetkeeps that true: it fails if a second test requests a local-area breakdown or a constituency or local-authority region, and its docstring says to put such tests in onexdist_group.Logs
Locally, with a failing test under seven workers, the output keeps: one
[gwN] [ NN%] PASSED|FAILED <test>entry per test (in completion order, with the worker), the start of each test, aFAILURESsection whose header names the worker, aslowest durationssection listing each test phase of a second or more, and the short test summary. The short tests' node IDs carry an@short_testssuffix.Testing
Red and green, re-run at both commits:
9643de72(tests only):TestSimulationRunIntegTests1 failed, 9 passed (the beta invocation);tests/simulation/test_parallel_beta_run.py2 failed, 1 passed (the group check and the worker-count check fail; the Hugging Face check pins an existing condition and passes).0db4f26f:TestSimulationRunIntegTests10 passed;test_parallel_beta_run.py3 passed.New tests:
projects/policyengine-simulation-executor/tests/test_modal_scripts.py: each environment's exact pytest invocation (beta with workers, prod without), and that the minted access token and everysimulation_integ_test_*setting reach the pytest process as exported variables (fakeuvandcurlfirst onPATH).projects/policyengine-apis-integ/tests/simulation/test_parallel_beta_run.py: every collected test in the directory withoutbeta_only, and only those, carries theshort_testsgroup;BETA_PYTEST_WORKERSequals the number ofbeta_onlytests plus one; only one test requests the UK local-area dataset.Checks run locally:
-m "not beta_only", withouttest_ping): serially 29 passed, 2 skipped (the auth smoke, which needs gateway auth); with-n 7 --dist loadgroupthe same 29 passed, 2 skipped, all on one worker. With nothing listening at the base URL, the full suite under-n 7 --dist loadgroupran each calculation test on its own worker (gw1 to gw6) and every other test on gw0.-n 4: every worker saw all five exportedsimulation_integ_test_*variables.projects/policyengine-simulation-executor: 1907 passed, 22 skipped.projects/policyengine-simulation-entry: 840 passed.pr.ymlpin):ruff format --check projects/*/src libs/*/srcand on the changed test files,ruff check .: passed.python scripts/run_quality_guards.py: every guard passed.python -m scripts.rules guard --base-ref origin/main: passed.python -m unittest discover -s scripts/rules/tests -t .: 263 tests OK.python -m scripts.rules render: no diff.python -m scripts.rules pyright projects/policyengine-apis-integ projects/policyengine-simulation-executor: no errors..github/scripts/check-uv-locks.shwith uv 0.13.6: passed.shellcheckon the changed script: passed.Environment variables
The changed step is "Run complete beta integration tests through all services" in the
integrationjob ofsimulation-deploy.reusable.yml. The job declaresenvironment: ${{ inputs.release_environment }}, whichsimulation-deploy.yml'sbetajob sets tobeta, and that caller passessecrets: inherit. The workflow wiring is unchanged by this PR. Names were confirmed with read-onlygh apicalls; no values were read.SIMULATION_TEST_AUTH_REQUIREDenv:literal"1"env:SIMULATION_TEST_AUTH_ISSUERbetaenvironment secretSIMULATION_ENTRYPOINT_AUTH_ISSUERenv:fromsecrets.*; the job'senvironment: betaenvironments/beta/secrets; not a repository or organization secretSIMULATION_TEST_AUTH_AUDIENCEbetaenvironment secretSIMULATION_ENTRYPOINT_AUTH_AUDIENCESIMULATION_TEST_AUTH_CLIENT_IDbetaenvironment secretSIMULATION_ENTRYPOINT_TEST_AUTH_CLIENT_IDSIMULATION_TEST_AUTH_CLIENT_SECRETbetaenvironment secretSIMULATION_ENTRYPOINT_TEST_AUTH_CLIENT_SECRETinputs.release_environment(beta),needs.deploy_entrypoint.outputs.candidate_url,needs.prepare.outputs.us_versionanduk_versionrun:argumentssimulation_integ_test_base_url,_gateway_auth_required,_access_token,_us_model_version,_uk_model_versionSettings)-n 4simulation_integ_test_timeout_in_millis,_poll_interval_secondsSettingsdefaults applyconftest.pyPATH(uv,python3,curl)setup-pythonandsetup-uvstepspytest-xdist itself sets
PYTEST_XDIST_WORKERand related variables in each worker; the tests do not read them. Nothing the tests read is set only in the controller process (for example by a pytest option or a fixture in the controller), so the workers see every value.Unverified until the first deploy after merge
🤖 Generated with Claude Code