Repository navigation
fix(ci): correct dead L0 test matrix condition in build.yml (ISD-7073) - #233
Merged
florentianayuwono merged 15 commits intoOct 8, 2026
Merged
Conversation
This was referenced Oct 6, 2026
The 'L0' step's condition compared matrix.runtime (always 'linux-*'-
prefixed, e.g. 'linux-x64', 'linux-arm64') against bare arch names
('arm64', 'amd64') that only ever appear in the separate matrix.runs_on
field. This mismatch meant the condition never evaluated true for any
matrix leg, so ./dev.sh test has silently never run as part of this
workflow, on any architecture, on PRs or pushes.
Compared against upstream actions/runner's build.yml to confirm intent:
upstream runs L0 on every runtime except the ones that can't execute
natively on its GitHub-hosted runners (linux-arm64, linux-arm, osx-arm64,
win-arm64, since those jobs run cross-compiled on amd64 hosts). This
fork instead builds each target on matching self-hosted native hardware
(runs-on: matrix.runs_on), so linux-arm64 can safely run its own tests
directly, unlike upstream.
Fix: compare against the correct matrix.runtime values and enable L0 for
linux-x64 and linux-arm64 (native, unpatched builds). linux-arm (32-bit
armv7l, cross-targeted), ppc64le, and s390x are intentionally left
disabled for now -- they require their respective patches applied first,
and their dotnet-test behavior under cross/32-bit targeting is not yet
validated. Enabling genuine L0 coverage for those is tracked as separate
follow-up work.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
StepHost.CheckPlatformForAlpineContainer gates Alpine Node support on Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.X64). That value is baked in at compile time via the X64/ARM/ARM64 preprocessor symbols (set from the PackageRuntime MSBuild property), not detected at test-runtime. When the Test assembly is built natively on a linux-arm64 self-hosted runner (no explicit PackageRuntime override), PlatformArchitecture compiles to Arm64, so these three tests deterministically throw NotSupportedException instead of exercising the Alpine-specific behavior they were written for. This was previously invisible because a dead matrix condition in build.yml meant the L0 test step never actually ran on the linux-arm64 leg. Gate the three Alpine-container tests with #if X64 so they only compile/run against X64-targeted Test assembly builds, matching the production code's own intentional restriction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JobExtension.InitializeJob reads ACTIONS_RUNNER_HOOK_JOB_STARTED and ACTIONS_RUNNER_HOOK_JOB_COMPLETED directly from the process environment to decide whether to inject extra job-hook steps into the returned step list. JobExtensionL0's tests never clear or mock these variables, implicitly assuming they're always unset. That assumption doesn't hold on this fork's self-hosted CI runners, which configure a job-started hook for their own lifecycle management (confirmed in the raw CI logs: "A job started hook has been configured by the self-hosted runner administrator"). Since these are real, process-level environment variables, they're inherited by the child dotnet test process and silently add one extra step to preJobSteps, which is why JobExtensionBuildStepsList failed with "Expected 5, Actual 6" identically on both the linux-x64 and linux-arm64 legs of #233's CI run, while never reproducing in a clean local environment without the hook configured. Capture and clear both hook variables in the constructor (xUnit creates a new instance per test) and restore the original values in Dispose(), following the same env-var isolation pattern already used for the ARM32 kill-switch override tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
florentianayuwono
force-pushed
the
fix/ci-l0-test-matrix-condition-isd-5856
branch
from
October 6, 2026 18:09
a3eb74d to
0b46f30
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The reviewed changes are complete; only a minor comment-wording nit remains.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes the inactive L0 test condition and makes newly enabled tests reliable on native CI architectures.
Changes:
- Enables L0 tests for
linux-x64andlinux-arm64. - Restricts Alpine tests to X64 builds.
- Isolates job-hook environment variables during tests.
| File | Summary |
|---|---|
src/Test/L0/Worker/StepHostL0.cs |
Gates Alpine-specific tests to X64 builds. |
src/Test/L0/Worker/JobExtensionL0.cs |
Prevents ambient job-hook variables from affecting tests. |
.github/workflows/build.yml |
Corrects the L0 matrix condition. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
florentianayuwono
marked this pull request as ready for review
October 6, 2026 18:27
florentianayuwono
changed the title
…ars" This reverts commit 0b46f30.
This reverts commit 84bf599.
Upstream actions/runner's own L0 condition excludes linux-arm64 (and other non-x64 targets) from running dotnet test; this fork's matrix condition fix should mirror that intent rather than opportunistically running tests on linux-arm64 as well. Scoping to linux-x64 only avoids exercising test-suite bugs that only manifest on non-x64 builds, without requiring direct modifications to upstream-synced test files that would be at risk of being silently reverted by the daily patch_update_main.yml upstream sync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit 626c6e0.
This reverts commit 7dc8874.
…vars" This reverts commit e9cd518.
…ars" This reverts commit 0b46f30.
…tensionL0 Replaces the JobExtensionL0 IDisposable env-isolation fix with a build.yml-level workaround, mirroring the existing pattern already used in tics.yaml. The self-hosted runner's ACTIONS_RUNNER_HOOK_JOB_STARTED/_COMPLETED hooks (configured by the fleet's runner-provisioning infra, not this repo) leak into the dotnet test child process and inflate JobExtensionBuildStepsList's step count. Unsetting both vars before invoking dev.sh test avoids the leak without modifying the test file itself, keeping JobExtensionL0.cs byte-identical to upstream and removing it from the upstream-sync overwrite risk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit 84bf599.
Upstream actions/runner never runs dotnet test on linux-arm64 (its condition excludes linux-arm64, linux-arm, osx-arm64, and win-arm64, testing only x64 flavors). This fork's build.yml previously added linux-arm64 to the tested runtimes beyond upstream's own scope, which is what exposed the StepHostL0 Alpine-container test failures (compile-time X64/ARM64 architecture baking via Constants.Runner.PlatformArchitecture never being exercised upstream on arm64). Restricting back to linux-x64 only realigns with upstream's actual tested scope and makes the StepHostL0 #if X64 gating fix unnecessary, so it's been reverted. Both JobExtensionL0.cs and StepHostL0.cs are now byte-identical to upstream again, removing any src/Test divergence that would otherwise be at risk of silent reversion by the daily upstream-sync bot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yanksyoon
approved these changes
Oct 8, 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.
Overview
The
L0test step in.github/workflows/build.ymlcomparesmatrix.runtimeagainst bare architecture names (arm64/amd64):But
matrix.runtimevalues are alwayslinux-*-prefixed — the bare names only ever appear in the separatematrix.runs_onfield. This mismatch means the condition never evaluates true for any matrix leg:./dev.sh testhas silently never run as part of this workflow, on any architecture, on any PR or push tomain/releases/*.Found while auditing PR #232 (ARM32 kill-switch override): a review comment flagged untested ARM32 branches, which traced back to CI never running
dotnet testat all, due to this dead condition.Enabling
dotnet testfor the first time surfaced two pre-existing, previously-invisible test failures:StepHostL0Alpine-container tests failing onlinux-arm64—StepHost.CheckPlatformForAlpineContainergates Alpine Node support onConstants.Runner.PlatformArchitecture.Equals(Constants.Architecture.X64), which is baked in at compile time, so the check always throwsNotSupportedExceptionon arm64. Upstreamactions/runner's ownbuild.ymlnever runsdotnet testonlinux-arm64either (its condition excludeslinux-arm64/linux-arm/osx-arm64/win-arm64, testing x64 flavors only) — so this is simply a gap this fork's workflow had opened by testing an architecture upstream never does.JobExtensionL0.JobExtensionBuildStepsListfailing with "Expected 5, Actual 6" (onlinux-x64too) —JobExtension.InitializeJobreadsACTIONS_RUNNER_HOOK_JOB_STARTED/_COMPLETEDdirectly from the real OS environment to decide whether to inject extra hook steps. This fork's self-hosted CI fleet configures a job-started hook for its own lifecycle management (confirmed in raw CI logs: "A job started hook has been configured by the self-hosted runner administrator"), so the real env var leaks into the childdotnet testprocess and inflates the step count the test asserts on.Fix
matrix.runtimevalues.linux-x64only, matching upstream's own tested scope — this also makes theStepHostL0arm64 failure moot, since those tests simply never run on arm64, exactly as upstream intends.ACTIONS_RUNNER_HOOK_JOB_STARTED/ACTIONS_RUNNER_HOOK_JOB_COMPLETEDbefore invokingdotnet testin theL0step, mirroring the existing pattern already used intics.yaml, so this fork's self-hosted job hooks don't leak into the test process.No
src/Testfiles are modified — bothStepHostL0.csandJobExtensionL0.csremain byte-identical to upstream, avoiding any risk of silent reversion by the daily upstream-sync bot (patch_update_main.yml), which wholesale-overwritessrc/from upstream on every sync and only preserves the three named arch patches.Testing
dotnet test --filter "FullyQualifiedName~StepHostL0|FullyQualifiedName~JobExtensionL0"with the hook env vars both set and unset, confirming the unset step reproduces and then resolves the leak.build (linux-x64)now actually runsdotnet testand passes; all other legs (linux-arm64,linux-arm,linux-ppc64le,linux-s390x) pass unaffected (no test step).Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com