ci: Narrow Testmon cache compatibility inputs by balasaajay · Pull Request #8001 · NVIDIA/Megatron-LM

balasaajay · 2026-10-09T06:20:30Z

What does this PR do?

Reduce unnecessary full unit-test runs by narrowing Testmon compatibility inputs and selecting conftests per bucket. For example, adding tests/unit_tests/tuning/conftest.py no longer invalidates an unrelated pipeline or model bucket's baseline.

Selective PR runs prioritize fast feedback and may miss failures caused by recipe or native-build changes. This PR deliberately accepts that limitation to avoid repeated full runs during PR iteration. Merge-group validation is the final testing gate: it runs the configured unit-test suites in full mode with Testmon selection disabled, using the merged recipe and built image. Failures missed during PR selection may therefore first appear in the merge queue. Existing workflow tests cover merge-group exclusion with selective testing requested through either the repository default or the enable label.

Keep COMPATIBILITY_FILES as a set containing only tests/test_utils/python_scripts/download_unit_tests_dataset.py. This script prepares the datasets and tokenizers embedded in the test container, so changes can alter test inputs without changing conftest.py or tracked package versions. Remove the other CI/build, package, README, runner/helper inputs (including testmon_mandatory.py), .dockerignore, and the docker/**/* glob, including recipe hashing through the platform registry. Keep COMPATIBILITY_GLOBS as an empty tuple and retain its expansion logic for future compatibility inputs.

Hash ancestor conftests for each bucket. Directory and glob buckets also include descendant conftests; explicit-file buckets include only ancestors. Glob handling conservatively includes descendants below the literal directory prefix, so catch-all buckets still track all unit-test conftests even when markers or ignore rules exclude tests. The selected path-to-hash map detects additions, edits, and deletions. Runtime-version checks, platform/bucket isolation, and database-integrity validation remain in place.

Adjust existing tests in tests/test_utils/test_unit_testmon_cache.py for the reduced compatibility inputs and recipe behavior. Extend the existing isolation test to verify root/nested fixture scope and cache reuse after an unrelated tuning conftest is added. No test functions or parameter cases are added.

Existing baseline manifests will mismatch once because their input maps contain the removed paths. Jobs continue to fall back to full buckets until the main baseline producer publishes refreshed generations.

Validation

Run in the existing Megatron-LM dev CI container with Python 3.12.3 and pytest-testmon 2.2.0:

Existing cache tests: 64 passed.
Black, isort, pylint, Ruff, mypy, and whitespace checks pass.

python -m pytest -q -o addopts= -p no:cacheprovider \
  tests/test_utils/test_unit_testmon_cache.py
BASE_REF=main CHECK_ONLY=true SKIP_DOCS=false bash tools/autoformat.sh
git diff --check

An earlier broader Testmon run also identified eight existing test_pr_label_gate failures in tests/test_utils/test_unit_testmon.py, reproduced against unmodified upstream commit ab1a28486. That fixture does not supply the workflow's UNIT_TESTMON_REQUESTED variable. Those unrelated tests are unchanged in this PR.

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>

copy-pr-bot · 2026-10-09T06:20:33Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>

balasaajay · 2026-10-09T07:05:06Z

/ok to test 2d31e0b

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>

ko3n1g · 2026-10-09T10:16:14Z

/review

svcnvidia-nemo-ci

Two compatibility checks are removed for inputs that Testmon cannot reliably observe. Static review only; I did not execute tests.

[P2] Retain the selected recipe's runtime contract

Failure: Changing only GPUS_PER_NODE=4 to 8 in the H100 GTP recipe now leaves the same bucket/platform compatibility identity. The interpreter and tracked package versions also remain unchanged, so the old generation passes validate_cache. This changes which tests can execute: GTP helpers skip mismatched world sizes, and the padded Muon parity cases require eight ranks. A four-rank baseline does not establish those results. The selection wrapper only collects tests, so it cannot discover this runtime change by executing their bodies. The expected behavior is to reject that baseline and fall back to the full bucket.

Correction: Preserve a compatibility signal for runtime-affecting changes to the selected recipe. The smallest conservative correction is to hash only that platform's recipe, rather than restoring the previous coupling to every platform. At cache_identity's input construction, replace the initial assignment with:

paths = {root / path for path in COMPATIBILITY_FILES}
paths.add(root / PLATFORMS[recipe_platform]["recipe"])

This deliberately also invalidates metadata-only edits to the selected recipe; retaining that optimization would require an explicit semantic runtime-contract fingerprint instead. Verify the proposed correction before adopting it.

Preservation: Keep platform/bucket lookup prefixes stable, keep unrelated platforms' recipe edits independent, and retain bucket-specific conftest isolation. Update test_new_platform_uses_registry_without_hashing_its_recipe to assert that the selected GB300 recipe is included and that editing it changes compatibility, rather than asserting identical identities. Preserve its unsupported-platform rejection.

Acceptance — proposed, not executed: With a finalized generation, edit its selected recipe and assert an unchanged cache_prefix, different compatibility, and a read-only validate_cache rejection matching compatibility. Separately capture an H100 identity, edit only the GB300 recipe, and assert the H100 identity remains identical. Retain the existing root/nested fixture assertions and unrelated tuning-conftest reuse assertion in test_platform_and_bucket_are_isolated. For the concrete GTP change, verify that the resolver selects full before Testmon selection; an eight-rank run should execute the eight-rank parity cases instead of relying on the four-rank generation.

Review generated by codex

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>

balasaajay · 2026-10-10T00:01:51Z

/review

svcnvidia-nemo-ci

Follow-up to review 5468913423. I compared the prior reviewed tree with the current head and checked the subsequent edits and directly affected validation/selection paths. Both prior failures remain unresolved. This was a scoped follow-up, not a fresh whole-PR review. Static review only; tests were not executed.

[P2] Retain the selected recipe's runtime contract

Failure: The recipe failure from the prior review still exists: cache_identity hashes no recipe. Changing only GPUS_PER_NODE=4 to 8 in the H100 GTP recipe leaves its bucket/platform compatibility unchanged. With matching package versions, the old generation passes validate_cache. However, the runner uses this value for torchrun's rank count, GTP helpers skip mismatched world sizes, and the padded parity cases require eight ranks. The four-rank baseline does not establish those results. Selection is collect-only, so it cannot discover this runtime change by executing the test bodies. Expected behavior is rejection of that baseline and a full-bucket fallback.

Correction: At cache_identity's path construction, replace the initial assignment with:

paths = {root / path for path in COMPATIBILITY_FILES}
paths.add(root / PLATFORMS[recipe_platform]["recipe"])

This restores the invariant that changing the selected recipe's runtime contract cannot accept a baseline produced under the old contract. It hashes only the selected platform's recipe, not every registered recipe. It conservatively invalidates metadata-only edits to that recipe too. Verify this suggested code before adopting it.

Preservation: Keep lookup prefixes stable, unrelated platforms' recipe edits independent, and bucket-specific conftest selection unchanged. Pipeline/glob buckets must still include their root and nested conftests; explicit-file buckets must still exclude unrelated descendants; adding the tuning conftest must still preserve all identities captured by test_platform_and_bucket_are_isolated.

Acceptance — proposed, not executed:

Using the existing finalized-generation fixture, edit the selected recipe. Assert unchanged cache_prefix, changed compatibility, and validate_cache raising a compatibility error without changing the restored files. Feed that identity to the existing action-resolver setup and expect mode=full, before Testmon selection.
Capture an H100 identity, edit only the GB200 recipe, and assert the H100 identity remains identical.
Update test_new_platform_uses_registry_without_hashing_its_recipe: the selected GB300 recipe must be present in inputs; editing it must preserve the prefix but change compatibility. Retain the unsupported-platform rejection and the assertion that the cache helper itself is not a compatibility input.
For the concrete GTP change, verify full fallback and then an eight-rank run executing the padded parity cases rather than relying on their four-rank baseline.

Review generated by codex

svcnvidia-nemo-ci · 2026-10-10T00:10:14Z

-)
-COMPATIBILITY_GLOBS = ("docker/**/*", ".dockerignore", "tests/unit_tests/**/conftest.py")
+COMPATIBILITY_FILES: set[str] = {"tests/test_utils/python_scripts/download_unit_tests_dataset.py"}
+COMPATIBILITY_GLOBS: tuple[str, ...] = ()


[P2] Keep native build changes from accepting stale selections

Failure: The native-build failure from prior comment 4229194174 remains unresolved. Edit a CUDA operation in the existing DeepEP NVSHMEM patch and rebuild the dev image without changing the tracked package versions. The Dockerfile applies that patch and builds the extension, but the empty glob list leaves cache_identity unchanged. validate_cache accepts the old generation; image identity is diagnostic, not a rejection guard. DeepEP is also excluded by the dependency override, and Python coverage cannot fingerprint the changed CUDA implementation. Consequently, unchanged DeepEP forward/backward tests can be deselected despite exercising a different binary. Expected behavior is to reject the generation and run the full bucket.

Correction: Restore the native-build compatibility signal with this self-contained replacement:

Suggested change

COMPATIBILITY_GLOBS: tuple[str, ...] = ()

COMPATIBILITY_GLOBS: tuple[str, ...] = ("docker/**/*", ".dockerignore")

The existing expansion already hashes these files and detects additions/deletions through the input map. The invariant is that a changed native build must not be declared compatible solely because tracked Python package versions match. Verify the suggested replacement before adopting it.

Preservation: Retain the dataset-downloader-only COMPATIBILITY_FILES, stable lookup prefixes, ordinary Python-source reuse, optional image diagnostics, and bucket-specific conftest isolation. In particular, do not restore the global conftest glob: an unrelated tuning conftest must still leave pipeline and explicit-file identities unchanged.

Acceptance — proposed, not executed: In source_tree, create .dockerignore and a native patch under docker/patches/ before finalizing the generation. Extend the compatibility rejection test with the Dockerfile, native patch, and .dockerignore, retaining its downloader case. Editing each must preserve cache_prefix, change compatibility, and raise a compatibility error without altering restored bytes or mtimes. Retain ordinary-source identity equality, root/nested conftest inclusion and unrelated tuning equality, and read-only acceptance with differing or missing image diagnostics.

Agreed that Python fingerprints and package versions cannot detect every native binary change, including the DeepEP patch example. This PR intentionally permits baseline reuse for PR feedback in that situation. Merge-group validation uses the built image and executes the configured unit-test suite without Testmon deselection; the workflow explicitly excludes merge groups from Testmon eligibility.

We accept that failures in these cases may first surface in the merge queue to reduce repeated full runs during PR iteration. Restoring docker/**/* and .dockerignore as blanket invalidators would reintroduce full PR runs for many unrelated build-input changes. The existing database-integrity and supported-version checks remain in place.

Please reassess this finding against that intended policy. The native-build coverage gap in selective PR feedback is acknowledged; full merge-group validation remains the final testing gate.

balasaajay · 2026-10-11T00:11:24Z

Regarding the selected recipe runtime-contract finding:

Agreed that changing the recipe's rank count can leave newly enabled tests unselected during PR validation. We intentionally accept that limitation: selective PR runs provide faster feedback, while merge-group validation runs the configured unit-test buckets in full with the merged recipe. The workflow explicitly excludes merge groups from Testmon eligibility.

Restoring whole-recipe hashing would reintroduce frequent full PR runs, including for edits that do not affect test execution. The intended policy is to retain its removal and accept that these failures may first be detected in the merge queue. That trades earlier failure detection for faster PR iteration; it does not claim that the old baseline proves coverage under the new rank count.

Please reassess this finding against that intended split between selective PR feedback and full merge-group validation.

balasaajay · 2026-10-11T00:17:14Z

/review

svcnvidia-nemo-ci

LGTM — scoped light follow-up to review 5476519829. Both changed files are unchanged from the previously reviewed tree.

I withdraw the prior recipe-runtime and native-build hashing repair requests after checking the clarified PR intent against current CI. The baseline-reuse behavior is unchanged, not fixed; the earlier findings assumed selective PR feedback must detect these changes, whereas the documented design deliberately leaves that detection to full merge-group validation.

The eligibility guard excludes merge groups. Both H100 and GB200 jobs consequently request full mode, whose runner path executes the configured buckets without Testmon selection. Existing workflow guard tests assert merge-queue exclusion for both repository-default and label-based selective requests.

No qualifying findings remain in this follow-up scope. Static review only; tests were not executed.

Review generated by codex

nemo-automation-bot · 2026-10-11T02:54:39Z