Make developer-specific test-harness paths configurable via environment variables - #29
Make developer-specific test-harness paths configurable via environment variables#29kaijunli-infr wants to merge 2 commits into
Conversation
9c0b747 to
f7b3d88
Compare
The wall-time scaling sweep, the MoE-EP baseline replay, and the MoE-EP non-dummy matrix all hard-coded /data/ycfeng/tmp as the root for their large intermediate outputs. On any other machine that path does not exist (or is not writable), so the harnesses cannot run and 13 unit tests fail out of the box. Add tests/scratch_root.py, which resolves the root from the new FRONTIER_TMP_ROOT environment variable and falls back to the historical path when unset, so existing deployments are unchanged. The three harness modules now read the root through this helper on every call instead of freezing it at import time. FRONTIER_WALLTIME_TMPDIR keeps its existing semantics and must still resolve to a descendant of the (now relocatable) root; the rejection messages mention FRONTIER_TMP_ROOT as the remedy. Unit tests for the two affected harnesses point the root at pytest's tmp_path via an autouse fixture, and a new test module covers the helper itself, including the fallback and rejection of non-absolute values. AGENTS.md documents the variable.
f7b3d88 to
0f56dd7
Compare
…DAF_REFERENCE_REPO_ROOT The PD-AF parity harness, its observer bootstrap, and the reference lifecycle integration test hard-coded the location of the pinned read-only Reference checkout under /data/ycfeng/stepfun-performance-optimization. Anyone who holds that checkout elsewhere cannot run the parity tooling, and the unit tests fail at path resolution before reaching the identity checks. Add tests/e2e/pd_af_parity/reference_repo_root.py, which resolves the root from the new FRONTIER_PDAF_REFERENCE_REPO_ROOT environment variable and falls back to the historical path when unset. The harness, bootstrap, and integration test resolve REFERENCE_REPO_ROOT through it once at import, so the many call sites that treat it as a stable pin keep working. Only the location is configurable: the pinned git HEAD and source SHA-256 checks are unchanged. The bootstrap's mismatch error now names the variable as the remedy. The wave1 harness test builds its producer payload from the resolved root instead of a literal so it stays correct under an override, and a new test module covers the helper. AGENTS.md documents the variable next to FRONTIER_TMP_ROOT.
|
Hi Kevin, thanks for your PR. Here are some review comments I put together with the help of AI—feel free to take a look. If you have any follow‑up updates, that would be even better. Appreciate it! Comment 1Severity: BLOCK This import breaks the PD-AF reference integration path. The integration test sets the child process
Please make the bootstrap self-contained under a Reference-only Please add a subprocess regression test that runs the observer-enabled Reference path with the same Comment 2Severity: MEDIUM The Reference root is represented inconsistently across processes. This resolver returns the raw environment-variable path. The bootstrap and integration driver call A valid path containing Please use one canonical representation everywhere. Either canonicalize the path in the shared resolver or call Please add tests for:
Comment 3Severity: MEDIUM This wrapper bypasses the new The Python entry point uses Please pass Please verify both cases:
Merge Requirements
bash -n tests/e2e/run_moe_ep_non_dummy_matrix.sh |
Problem
Several test harnesses hard-code paths that only exist on the original author's machine, so on a fresh clone they cannot run and their unit tests fail before exercising any logic.
Scratch root
/data/ycfeng/tmp, used for large intermediate outputs by:tests/performance/sim_walltime_scaling/sweep.py(DEFAULT_TEMP_ROOT, which also boundsFRONTIER_WALLTIME_TMPDIR)tests/e2e/moe_ep_baseline_replay.py(TMP_ROOT, used for path validation, childTMPDIR, and CLI defaults)tests/e2e/moe_ep_non_dummy_matrix.py(default--output-rootfor both matrix kinds)Pinned Reference checkout
/data/ycfeng/stepfun-performance-optimization/Frontier/worktrees/ref-afd-readonly, used by the PD-AF parity harness (tests/e2e/pd_af_parity/harness.py,reference_observer_bootstrap.py) andtests/integration/test_pdaf_reference_lifecycle_observer.py.Change
Two commits, one per path family. Both keep the historical path as the fallback when the variable is unset, so existing deployments are unchanged.
FRONTIER_TMP_ROOT(commit 1)tests/scratch_root.pywithresolve_scratch_root(). The value must be absolute and is re-read on every call.FRONTIER_WALLTIME_TMPDIRkeeps its semantics and must still resolve to a descendant of the (now relocatable) root. Rejection messages point atFRONTIER_TMP_ROOTas the remedy.tmp_pathvia an autouse fixture. Newtests/unit/test_scratch_root.pycovers the helper.FRONTIER_PDAF_REFERENCE_REPO_ROOT(commit 2)tests/e2e/pd_af_parity/reference_repo_root.pywithresolve_reference_repo_root().harness.py,reference_observer_bootstrap.py, and the integration test resolveREFERENCE_REPO_ROOTthrough it once at import, so the many call sites that treat it as a stable pin keep working. Only the location is configurable; the pinned git HEAD and source SHA-256 checks are unchanged. The bootstrap's mismatch error names the variable.test_pdaf_parity_harness_wave1.pybuilds its producer payload from the resolved root instead of a literal. Newtests/unit/test_pdaf_reference_repo_root.pycovers the helper.AGENTS.mddocuments both variables under Tests and gets a Modification History row.Verification
Fresh clone on a machine without
/data/ycfeng(environment.yml+pip install -e ".[test]", plus torch/matplotlib), runningtests/unit tests/integration:test_sim_walltime_scaling_sweep.py+test_moe_ep_baseline_replay.pyWith
FRONTIER_PDAF_REFERENCE_REPO_ROOTpointed at an existing plain directory, 17 of the 51 bootstrap unit tests additionally pass; the remaining 34 (and the 5 integration errors) need a real checkout at the pinned commitdcb1cc8e, which is not in this repository's public history, so they cannot pass anywhere without that private worktree. That is inherent to the identity checks and unchanged by this PR.Other remaining failures pre-exist on
mainand are unrelated: docs-contract tests referencing a missingtests/debugfixture directory, and a few tests needing the profiling env.Also verified manually: with
FRONTIER_TMP_ROOTset,sweep._resolve_temp_root()returns the override and rejectsFRONTIER_WALLTIME_TMPDIR=/tmp; with both variables unset, both helpers return their historical paths.Out of scope
The
/local/ycfeng/...interpreter defaults in the GPU profiling shell scripts undertests/analysis,tests/e2e/operator_parity, andtests/performanceare not touched here.