From 776d9ef6e38942937b8b4b2a7ae2e03c08f81a03 Mon Sep 17 00:00:00 2001 From: Terry Kong Date: Mon, 14 Apr 2025 12:09:31 -0700 Subject: [PATCH 1/4] ci: labels for docs/L0/L1/L2 and run even if only doc test Signed-off-by: Terry Kong --- .github/workflows/cicd-main.yml | 120 ++++++++++++++++++++++---------- 1 file changed, 84 insertions(+), 36 deletions(-) diff --git a/.github/workflows/cicd-main.yml b/.github/workflows/cicd-main.yml index 4041f31bc3..06b3f90808 100644 --- a/.github/workflows/cicd-main.yml +++ b/.github/workflows/cicd-main.yml @@ -25,9 +25,14 @@ on: inputs: test_to_run: required: false - default: all - type: string - description: Comma-separated list of tests to run. Use "all" to run the full test suite. + default: L2 + type: choice + options: + - docs + - L0 + - L1 + - L2 + description: Test level to run. docs = doc tests only, L0 = unit/docs/lint, L1 = L0 + functional, L2 = L1 + convergence # TODO: Due to limited compute, disabling pushes to main. This is okay to do since we force PRs to be up to date and the CI tests on pull/$PR_NUM/merge #push: # branches: @@ -41,20 +46,8 @@ jobs: pre-flight: runs-on: ubuntu-latest outputs: - test_to_run: ${{ steps.test_to_run.outputs.main }} - all: ${{ steps.all.outputs.main }} - run_ci: ${{ steps.evaluate.outputs.run_ci }} + test_level: ${{ steps.evaluate.outputs.test_level }} steps: - - name: Parse test_to_run - id: test_to_run - run: | - parsed_string=$(echo ${{ inputs.test_to_run || 'all' }} | jq -c --raw-input 'split(",")') - echo "main=${parsed_string}" | tee -a "$GITHUB_OUTPUT" - - name: Parse all - id: all - run: | - echo "main=${{ contains(fromJSON(steps.test_to_run.outputs.main), 'all') }}" | tee -a "$GITHUB_OUTPUT" - - name: Get changed files id: changed-files if: github.event_name == 'pull_request' @@ -81,19 +74,34 @@ jobs: # Some output that's helpful for debugging echo "Docs changed: $CHANGED_DOCS" echo "Src changed: $CHANGED_SRC" - - # echo "DOCS_ONLY: $DOCS_ONLY" echo "LABEL: $LABEL" echo "IS_PULLREQUEST: $IS_PULLREQUEST" - # Run CI only (on main or if label is attached) and if it's not only docs - echo run_ci=$([[ ("$LABEL" = "true" || "$IS_PULLREQUEST" = "false" || "$MERGE_GROUP" = "true") && "$DOCS_ONLY" = "false" ]] && echo "true" || echo "false") | tee -a "$GITHUB_OUTPUT" + # Determine test level based on conditions + if [[ "$DOCS_ONLY" == "true" ]]; then + # For doc-only changes, run only doc tests + TEST_LEVEL="docs" + elif [[ "$LABEL" == "true" || "$IS_PULLREQUEST" == "false" || "$MERGE_GROUP" == "true" ]]; then + # For labeled PRs, pushes to main (IS_PULL_REQUEST=false), or merge group events, run L0 by default + TEST_LEVEL="L0" + else + # Skip tests by default for non-labeled PRs + TEST_LEVEL="none" + fi + + # Override test level if specified in workflow_dispatch + if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then + echo "Overriding test level from $TEST_LEVEL to ${{ inputs.test_to_run }}" + TEST_LEVEL="${{ inputs.test_to_run }}" + fi + + echo "test_level=$TEST_LEVEL" | tee -a "$GITHUB_OUTPUT" lint-check: name: Lint check needs: [pre-flight] runs-on: ubuntu-latest - if: ${{ needs.pre-flight.outputs.run_ci == 'true' }} + if: ${{ contains(fromJSON('["L0", "L1", "L2"]'), needs.pre-flight.outputs.test_level) }} steps: - name: Checkout repository uses: actions/checkout@v4 @@ -107,7 +115,7 @@ jobs: name: Sphinx build needs: [pre-flight] runs-on: ubuntu-latest - if: ${{ needs.pre-flight.outputs.run_ci == 'true' }} + if: ${{ needs.pre-flight.outputs.test_level != 'none' }} steps: - name: Checkout repository uses: actions/checkout@v4 @@ -118,7 +126,7 @@ jobs: uv run --group docs sphinx-build . _build/html build-container: - if: ${{ needs.pre-flight.outputs.run_ci == 'true' }} + if: ${{ needs.pre-flight.outputs.test_level != 'none' }} needs: [pre-flight] uses: NVIDIA/NeMo-FW-CI-templates/.github/workflows/_build_container.yml@v0.22.7 with: @@ -134,29 +142,52 @@ jobs: name: Tests needs: [build-container, pre-flight] uses: ./.github/workflows/_run_test.yml - if: ${{ needs.pre-flight.outputs.run_ci == 'true' }} + if: ${{ needs.pre-flight.outputs.test_level != 'none' }} with: RUNNER: self-hosted-azure TIMEOUT: 15 UNIT_TEST_SCRIPT: | cd /opt/reinforcer - uv run --no-sync bash -x ./tests/run_unit.sh + if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(L0|L1|L2)$ ]]; then + uv run --no-sync bash -x ./tests/run_unit.sh + else + echo "Skipping unit tests for docs-only level" + fi DOC_TEST_SCRIPT: | cd /opt/reinforcer/docs - uv run --no-sync sphinx-build -b doctest . _build/doctest + if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(docs|L0|L1|L2)$ ]]; then + uv run --no-sync sphinx-build -b doctest . _build/doctest + else + echo "Skipping doc tests for level ${{ needs.pre-flight.outputs.test_level }}" + fi FUNCTIONAL_TEST_SCRIPT: | - # TODO: Temporarily disable functional tests until we have more capacity and tests run quicker - # Related: https://github.com/NVIDIA/reinforcer/pull/27 - # cd /opt/reinforcer - # uv run --no-sync bash ./tests/functional/sft.sh - # uv run --no-sync bash ./tests/functional/grpo.sh + cd /opt/reinforcer + if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(L1|L2)$ ]]; then + uv run --no-sync bash ./tests/functional/sft.sh + uv run --no-sync bash ./tests/functional/grpo.sh + else + echo "Skipping functional tests for level ${{ needs.pre-flight.outputs.test_level }}" + fi + CONVERGENCE_TEST_SCRIPT: | + cd /opt/reinforcer + if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(L2)$ ]]; then + echo "Running convergence tests" + # Add your convergence test commands here + # uv run --no-sync bash ./tests/convergence/test.sh + else + echo "Skipping convergence tests for level ${{ needs.pre-flight.outputs.test_level }}" + fi AFTER_SCRIPT: | cd /opt/reinforcer cat <> $GITHUB_STEP_SUMMARY + echo '🤖: CICD Result for test level: ${{ needs.pre-flight.outputs.test_level }}' >> $GITHUB_STEP_SUMMARY echo "$SUMMARY" >> $GITHUB_STEP_SUMMARY + if [[ "$TEST_LEVEL" == "none" ]]; then + echo "No tests were run, passing gate" >> $GITHUB_STEP_SUMMARY + exit 0 + fi + test "$ALL_SUCCESS" = "true" || test "$CI_SKIP" = "true" DCO_merge_group: From e83fa05a29d6fb70755121895c3fe7d1330f7b9a Mon Sep 17 00:00:00 2001 From: Terry Kong Date: Mon, 14 Apr 2025 12:22:09 -0700 Subject: [PATCH 2/4] fix Signed-off-by: Terry Kong --- .github/workflows/cicd-main.yml | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/.github/workflows/cicd-main.yml b/.github/workflows/cicd-main.yml index 06b3f90808..8e3f524be9 100644 --- a/.github/workflows/cicd-main.yml +++ b/.github/workflows/cicd-main.yml @@ -101,7 +101,7 @@ jobs: name: Lint check needs: [pre-flight] runs-on: ubuntu-latest - if: ${{ contains(fromJSON('["L0", "L1", "L2"]'), needs.pre-flight.outputs.test_level) }} + if: ${{ needs.pre-flight.outputs.test_level != 'none' }} steps: - name: Checkout repository uses: actions/checkout@v4 @@ -205,18 +205,15 @@ jobs: - name: main env: JOB_RESULTS: ${{ toJSON(needs) }} - # Check if jobs succeeded based on test level - ALL_SUCCESS: ${{ - needs.pre-flight.outputs.test_level == 'none' || - ( - !contains(needs.*.result, 'failure') && - !contains(needs.*.result, 'cancelled') && - ( - needs.pre-flight.outputs.test_level == 'docs' && needs.sphinx-build.result == 'success' && needs.tests.result == 'success' || - contains(fromJSON('["L0", "L1", "L2"]'), needs.pre-flight.outputs.test_level) && needs.lint-check.result == 'success' && needs.sphinx-build.result == 'success' && needs.tests.result == 'success' - ) - ) - }} + # Job is considered successful if nothing was run, or if all jobs were successful (the tests run even if only docs were run b/c doctests are selected) + ALL_SUCCESS: >- + ${{ + (needs.pre-flight.outputs.test_level == 'none') || + (needs.pre-flight.outputs.test_level != 'none' && + needs.lint-check.result == 'success' && + needs.sphinx-build.result == 'success' && + needs.tests.result == 'success') + }} CI_SKIP: ${{ github.event.label.name == 'Skip CICD' }} TEST_LEVEL: ${{ needs.pre-flight.outputs.test_level }} run: | From 2d80dd16879db26baf531efa3ca6b229d9430d02 Mon Sep 17 00:00:00 2001 From: Terry Kong Date: Mon, 14 Apr 2025 12:23:09 -0700 Subject: [PATCH 3/4] cancel for now Signed-off-by: Terry Kong --- .github/workflows/cicd-main.yml | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/workflows/cicd-main.yml b/.github/workflows/cicd-main.yml index 8e3f524be9..c6ff5bd004 100644 --- a/.github/workflows/cicd-main.yml +++ b/.github/workflows/cicd-main.yml @@ -168,15 +168,16 @@ jobs: else echo "Skipping functional tests for level ${{ needs.pre-flight.outputs.test_level }}" fi - CONVERGENCE_TEST_SCRIPT: | - cd /opt/reinforcer - if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(L2)$ ]]; then - echo "Running convergence tests" - # Add your convergence test commands here - # uv run --no-sync bash ./tests/convergence/test.sh - else - echo "Skipping convergence tests for level ${{ needs.pre-flight.outputs.test_level }}" - fi + # TODO: enable once we have convergence tests in CI + #CONVERGENCE_TEST_SCRIPT: | + # cd /opt/reinforcer + # if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(L2)$ ]]; then + # echo "Running convergence tests" + # # Add your convergence test commands here + # # uv run --no-sync bash ./tests/convergence/test.sh + # else + # echo "Skipping convergence tests for level ${{ needs.pre-flight.outputs.test_level }}" + # fi AFTER_SCRIPT: | cd /opt/reinforcer cat < Date: Mon, 14 Apr 2025 14:33:10 -0700 Subject: [PATCH 4/4] quoting issue Signed-off-by: Terry Kong --- .github/workflows/cicd-main.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cicd-main.yml b/.github/workflows/cicd-main.yml index c6ff5bd004..8667e762c7 100644 --- a/.github/workflows/cicd-main.yml +++ b/.github/workflows/cicd-main.yml @@ -151,14 +151,14 @@ jobs: if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(L0|L1|L2)$ ]]; then uv run --no-sync bash -x ./tests/run_unit.sh else - echo "Skipping unit tests for docs-only level" + echo Skipping unit tests for docs-only level fi DOC_TEST_SCRIPT: | cd /opt/reinforcer/docs if [[ "${{ needs.pre-flight.outputs.test_level }}" =~ ^(docs|L0|L1|L2)$ ]]; then uv run --no-sync sphinx-build -b doctest . _build/doctest else - echo "Skipping doc tests for level ${{ needs.pre-flight.outputs.test_level }}" + echo Skipping doc tests for level ${{ needs.pre-flight.outputs.test_level }} fi FUNCTIONAL_TEST_SCRIPT: | cd /opt/reinforcer @@ -166,7 +166,7 @@ jobs: uv run --no-sync bash ./tests/functional/sft.sh uv run --no-sync bash ./tests/functional/grpo.sh else - echo "Skipping functional tests for level ${{ needs.pre-flight.outputs.test_level }}" + echo Skipping functional tests for level ${{ needs.pre-flight.outputs.test_level }} fi # TODO: enable once we have convergence tests in CI #CONVERGENCE_TEST_SCRIPT: |