Skip to content

Enable coverage for l1 tests#144

Merged
skyw merged 1 commit intomainfrom
skyw/enable_coverag_for_l1_tests
Mar 22, 2026
Merged

Enable coverage for l1 tests#144
skyw merged 1 commit intomainfrom
skyw/enable_coverag_for_l1_tests

Conversation

@skyw
Copy link
Contributor

@skyw skyw commented Mar 22, 2026

So that it doesn't fail on coverage collection steps.

Signed-off-by: Hao Wu <skyw@nvidia.com>
@copy-pr-bot
Copy link

copy-pr-bot bot commented Mar 22, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 22, 2026

Greptile Summary

This PR updates tests/ci/L1_Tests_GPU.sh to instrument the nightly convergence tests with coverage run -p, matching the pattern already established by the L0 test scripts, so the downstream coverage collection steps in test-template/action.yml (which call coverage combine and coverage xml) no longer fail on missing coverage data.

Key changes:

  • Adds mkdir -p test-results/tests/convergence so the XML report directory exists before tests run.
  • Wraps the python $test invocation with coverage run -p --source=emerging_optimizers and captures per-run .coverage.* files (combined by the CI template afterwards).
  • Adds --xml_output_file to produce JUnit-style XML reports consumed by the publish-nightly-test-results workflow step.
  • Extracts the fixed seed into a fix_seed=77 variable (changing from the original hardcoded 42) to avoid producing identical report filenames/coverage data to L0_Tests_GPU.sh, which already runs these same convergence tests with fix_seed=42.

Confidence Score: 5/5

  • Safe to merge — the change is a straightforward alignment with the existing L0 coverage pattern and introduces no logical risk.
  • The new script is structurally identical to the corresponding coverage loop already present in L0_Tests_GPU.sh (lines 35–39). The mkdir, coverage run -p, --xml_output_file, and seed variable are all consistent with how L0 tests are instrumented, and coverage combination is handled externally by the CI template. No test logic or coverage source is altered.
  • No files require special attention.

Important Files Changed

Filename Overview
tests/ci/L1_Tests_GPU.sh Adds coverage instrumentation to nightly convergence tests, creates the test-results directory, and changes the fixed seed from 42 to 77 to avoid colliding with the same seed used by L0_Tests_GPU.sh.

Sequence Diagram

sequenceDiagram
    participant CI as cicd-nightly-tests
    participant Action as test-template/action.yml
    participant Container as Docker Container
    participant Script as L1_Tests_GPU.sh
    participant Coverage as coverage tool

    CI->>Action: invoke with script=L1_Tests_GPU
    Action->>Container: docker run (GPU)
    Action->>Container: bash tests/ci/L1_Tests_GPU.sh

    Container->>Script: execute
    Script->>Container: mkdir -p test-results/tests/convergence
    loop for each *_test.py in tests/convergence/
        Script->>Coverage: coverage run -p --source=emerging_optimizers <test> --seed=77 --xml_output_file=...
        Coverage->>Container: writes .coverage.<pid> partial file
        Coverage->>Container: writes test-results/tests/convergence/*_seed77.xml
    end

    Action->>Container: coverage combine (merges .coverage.* files)
    Action->>Container: coverage xml
    Action->>CI: upload .coverage + coverage.xml artifacts
    Action->>CI: upload test-results/**/*.xml artifacts
Loading

Reviews (1): Last reviewed commit: "enable coverage for l1 tests" | Re-trigger Greptile

@skyw skyw merged commit 82664ea into main Mar 22, 2026
4 checks passed
@skyw skyw deleted the skyw/enable_coverag_for_l1_tests branch March 22, 2026 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant