Skip to content

Commit a98d3f4

Browse files
fix: combine CI matrix strategy and action description fixes
This combines three related CI fixes so they can be reviewed together: - `.github/actions/test-template/action.yml`: corrects the `is_optional` description so it matches the actual behavior. - `.github/workflows/cicd-main.yml`: adds missing `strategy`/`matrix` blocks to the trt-onnx, vllm, and inframework e2e jobs, and uses `${{ matrix.script }}` and `${{ matrix.is_optional }}` instead of hard-coded values. Tests - `unit_tests/github_actions/test_optional_description.py`: verifies the `is_optional` description no longer inverts the behavior. - `tests/test_cicd_yaml.py`: verifies the e2e jobs define a strategy block before referencing matrix context. Closes #728, #749, #750. Signed-off-by: Andrew White <andrewwhitecdw@users.noreply.github.com>
1 parent c080fb9 commit a98d3f4

4 files changed

Lines changed: 87 additions & 7 deletions

File tree

‎.github/actions/test-template/action.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ inputs:
2323
description: "Test script to execute"
2424
required: true
2525
is_optional:
26-
description: "Failure will cancel all other tests if set to true"
26+
description: "Failure is ignored and other tests continue if set to true"
2727
required: false
2828
default: "false"
2929
is_unit_test:

‎.github/workflows/cicd-main.yml‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,12 @@ jobs:
196196
runner: ${{ matrix.runner }}
197197

198198
cicd-e2e-tests-trt-onnx:
199+
strategy:
200+
fail-fast: false
201+
matrix:
202+
include:
203+
- script: L2_ONNX_TRT
204+
is_optional: false
199205
needs: [pre-flight, cicd-unit-tests-vllm]
200206
runs-on: ${{ needs.pre-flight.outputs.runner_prefix }}
201207
name: ${{ matrix.is_optional && 'PLEASEFIXME_' || '' }}${{ matrix.script }}
@@ -222,8 +228,8 @@ jobs:
222228
- name: main
223229
uses: ./.github/actions/test-template
224230
with:
225-
script: L2_ONNX_TRT
226-
is_optional: ${{ matrix.is_optional || false }}
231+
script: ${{ matrix.script }}
232+
is_optional: ${{ matrix.is_optional }}
227233
is_unit_test: "false"
228234
timeout: 60
229235
PAT: ${{ steps.app-token.outputs.token }}
@@ -233,6 +239,12 @@ jobs:
233239
runner: ${{ needs.pre-flight.outputs.runner_prefix }}
234240

235241
cicd-e2e-tests-vllm:
242+
strategy:
243+
fail-fast: false
244+
matrix:
245+
include:
246+
- script: L2_Launch_vLLM
247+
is_optional: false
236248
needs: [cicd-unit-tests-vllm, pre-flight]
237249
runs-on: ${{ needs.pre-flight.outputs.runner_prefix }}
238250
name: ${{ matrix.is_optional && 'PLEASEFIXME_' || '' }}${{ matrix.script }}
@@ -257,8 +269,8 @@ jobs:
257269
- name: main
258270
uses: ./.github/actions/test-template
259271
with:
260-
script: L2_Launch_vLLM
261-
is_optional: ${{ matrix.is_optional || false }}
272+
script: ${{ matrix.script }}
273+
is_optional: ${{ matrix.is_optional }}
262274
is_unit_test: "false"
263275
timeout: 60
264276
PAT: ${{ steps.app-token.outputs.token }}
@@ -268,7 +280,19 @@ jobs:
268280
runner: ${{ needs.pre-flight.outputs.runner_prefix }}
269281

270282
cicd-e2e-tests-inframework:
283+
strategy:
284+
fail-fast: false
285+
matrix:
286+
include:
287+
- script: L2_Launch_InFramework
288+
is_optional: false
271289
needs: [pre-flight, cicd-unit-tests-vllm]
290+
strategy:
291+
fail-fast: false
292+
matrix:
293+
include:
294+
- script: L2_Launch_InFramework
295+
is_optional: false
272296
runs-on: ${{ needs.pre-flight.outputs.runner_prefix }}
273297
name: ${{ matrix.is_optional && 'PLEASEFIXME_' || '' }}${{ matrix.script }}
274298
environment: nemo-ci
@@ -292,8 +316,8 @@ jobs:
292316
- name: main
293317
uses: ./.github/actions/test-template
294318
with:
295-
script: L2_Launch_InFramework
296-
is_optional: ${{ matrix.is_optional || false }}
319+
script: ${{ matrix.script }}
320+
is_optional: ${{ matrix.is_optional }}
297321
is_unit_test: "false"
298322
timeout: 60
299323
PAT: ${{ steps.app-token.outputs.token }}

‎tests/test_cicd_yaml.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Copyright (c) 2025, NVIDIA CORPORATION. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
from pathlib import Path
16+
17+
import yaml
18+
19+
WORKFLOW_FILE = Path(__file__).resolve().parents[1] / ".github" / "workflows" / "cicd-main.yml"
20+
21+
22+
def test_inframework_e2e_job_has_matching_matrix():
23+
"""Regression test for the inframework e2e job referencing matrix values without a strategy block."""
24+
workflow = yaml.safe_load(WORKFLOW_FILE.read_text())
25+
job = workflow["jobs"]["cicd-e2e-tests-inframework"]
26+
27+
assert "strategy" in job, "inframework e2e job must define a strategy block"
28+
assert "matrix" in job["strategy"], "strategy block must define a matrix"
29+
30+
include = job["strategy"]["matrix"].get("include", [])
31+
assert len(include) == 1, "expected exactly one matrix include for the inframework e2e job"
32+
matrix = include[0]
33+
assert matrix["script"] == "L2_Launch_InFramework"
34+
assert matrix["is_optional"] is False
35+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#!/usr/bin/env python3
2+
"""Regression test: is_optional description matches behavior."""
3+
4+
import sys
5+
6+
7+
def main():
8+
with open(".github/actions/test-template/action.yml") as f:
9+
content = f.read()
10+
11+
if "Failure will cancel all other tests if set to true" in content:
12+
print("BUG: is_optional description inverts the actual behavior")
13+
sys.exit(1)
14+
15+
if "is_optional:" not in content or "description:" not in content:
16+
print("Could not find is_optional input description")
17+
sys.exit(1)
18+
19+
print("OK")
20+
21+

0 commit comments

Comments
 (0)