Repository navigation
fix(ci,tests): propagate dynamic workunits count to verification scripts - #987
Merged
Merged
Conversation
Ensure custom num_work_units specified in workflow_dispatch triggers is propagated to simple_job_verify and simple_job_ui_verify. Add environment fallback in common.resolve_expected_work_units and pass explicit CLI flags in nightly-test-suite.yml. TAG=agy CONV=df3c1c34-924a-42d3-81dc-9755d79b93f5
janzak-dev
temporarily deployed
to
xmc-nightly
September 25, 2026 10:48 — with
GitHub Actions
Inactive
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function resolve_expected_work_units to dynamically resolve the expected work units count from a command-line flag, the NUM_WORK_UNITS environment variable, or a default fallback value. This helper is integrated into both the UI and backend verification scripts, and corresponding unit tests are added. The feedback suggests validating that the resolved work units count is strictly positive to prevent downstream issues, along with adding test cases to cover non-positive input scenarios.
…cted_work_units Address reviewer feedback on PR #987 by ensuring non-positive values (<= 0) passed via flag or NUM_WORK_UNITS env var are ignored and fall back to default. Also add corresponding unit test assertions. TAG=agy CONV=df3c1c34-924a-42d3-81dc-9755d79b93f5
janzak-dev
marked this pull request as ready for review
September 25, 2026 10:57
janzak-dev
requested review from
andrzejfudro
and removed request for
AntonTyb
September 25, 2026 10:58
andrzejfudro
approved these changes
Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Propagates dynamic
num_work_unitsconfiguration from GitHub Actionsworkflow_dispatchmanual triggers to downstream verification scripts (simple_job_verifyandsimple_job_ui_verify), preventing assertion timeouts when non-default WorkUnit counts (e.g., 2) are launched.It used to fail when you specified a number of workunits different than the default one https://github.com/google/xmc/actions/runs/36124267073/job/108036661702
Motivation & Context
In manual dispatch runs of
nightly-test-suite.yml, users can specify custom WorkUnit counts (e.g.,num_work_units: 2). While the workload launcher honored this setting, the SDK and UI verification steps lacked--expected_work_unitsflags and environment variable fallbacks, causing them to fall back to hardcoded defaults (3 WorkUnits) and time out waiting for non-existent units.Key Changes
.github/workflows/nightly-test-suite.yml): Explicitly passes--expected_work_units="${NUM_WORK_UNITS}"to bothtools.nightly_tests.simple_job_verifyandtools.nightly_tests.simple_job_ui_verify.tools/nightly_tests/common.py): Addedresolve_expected_work_units(default)which checksFLAGS.expected_work_units, falls back toos.environ.get("NUM_WORK_UNITS"), validates that values are strictly positive (> 0), and defaults to 3.tools/nightly_tests/simple_job_verify.py&simple_job_ui_verify.py): Parametrized expected WorkUnit counts and expected worker names (tuple(str(i) for i in range(1, expected_work_units + 1))).tools/nightly_tests/common_test.py): Added test coverage forresolve_expected_work_unitscovering flag precedence, environment fallback, non-positive integer filtering, and default values.Assumptions
None — requirements were fully specified.
Verification & Testing
bazel test //tools/nightly_tests:all(4/4 tests passed)./.githooks/pre-commit(all linters passed)./tools/analyze_diff_size.py --base-ref origin/main...(0 threshold exceedances)workflow_dispatchon branchfix/dynamic-workunits-propagationwithnum_work_units: 2.demo-worker-1,demo-worker-2) on XID 24066.Expected WorkUnits Count: 2and verified 2/2 WorkUnits completed oncpu-queue.Demo Workload (2 units, 60s execution)and verified 2 WorkUnits rendered in the DOM.Related Issues / Bugs
N/A
TAG=agy
CONV=df3c1c34-924a-42d3-81dc-9755d79b93f5