[browser] Run Wasm.Build.Tests on CoreCLR the same way as Mono - #132478
[browser] Run Wasm.Build.Tests on CoreCLR the same way as Mono#132478maraf wants to merge 5 commits into
Conversation
The wasm-tools workload now supports CoreCLR (#130380), so the bespoke build output delivery added in #127073 / #128296 is no longer needed, and CoreCLR can run the same two Wasm.Build.Tests lanes as Mono: one NoWorkload and one with the workload installed. Remove the custom Helix delivery: - staged eng/, minipal and vm/wasm header correlation payloads, and the CoreCLR-only EMSDK payload - the HelixPreCommand env vars bridging those payloads to MSBuild properties, and the matching test-side plumbing (EnvironmentVariables, BuildEnvironment, data/Local.Directory.Build.*, RunScriptTemplate.sh, RunScriptCommands) - the env-var overrides for the relink include dirs in BrowserWasmApp.CoreCLR.targets, which now resolve from the runtime pack - the emsdk provisioning step in the CoreCLR Wasm.Build.Tests pipeline job Run both lanes for CoreCLR: - sendtohelix.proj produces the workload cross product for CoreCLR too, pinned to the default webcil/fingerprinting values so it stays at two lanes - sendtohelix-wasm.targets picks the work item shape from the lane rather than the runtime flavor, so both flavors fan out per test class in the workload lane and use a single work item in the no-workload lane - the CoreCLR xunit trait filter now appends the Mono-only exclusions instead of replacing the workload/no-workload selection - workloads-browser.targets installs wasm-tools and wasm-experimental for CoreCLR, and the pipeline job stages the Mono runtime packs those require Also guard the multithreading runtime pack assertion in BuildEnvironment, as CoreCLR has no multithreaded browser-wasm runtime pack. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d566c082-e742-4aa5-bb44-a03b6f05cce2
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Removes bespoke CoreCLR-only Helix payload plumbing for Wasm.Build.Tests and aligns CoreCLR test execution with Mono by running both workload and no-workload lanes using the standard workload/runtime-pack flow.
Changes:
- Dropped CoreCLR-specific env-var/payload bridging and local import plumbing now that
wasm-toolssupports CoreCLR. - Updated Helix lane/work item selection to be driven by “workload vs no-workload” lane shape for both Mono and CoreCLR.
- Updated CI pipeline to install the needed workloads for CoreCLR and stage required runtime packs referenced by the workload manifest.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mono/wasm/Wasm.Build.Tests/data/RunScriptTemplate.sh | Removes CoreCLR payload-relative env var exports that are no longer needed. |
| src/mono/wasm/Wasm.Build.Tests/data/Local.Directory.Build.targets | Removes ad-hoc import of BrowserWasmApp.CoreCLR.targets for CoreCLR no-workload runs. |
| src/mono/wasm/Wasm.Build.Tests/data/Local.Directory.Build.props | Removes CoreCLR-only MSBuild property bridging from env vars. |
| src/mono/wasm/Wasm.Build.Tests/Wasm.Build.Tests.csproj | Adjusts CoreCLR trait filtering and removes local CoreCLR EMSDK/env-var wiring. |
| src/mono/wasm/Wasm.Build.Tests/README.md | Documents that CI now runs workload/no-workload for both Mono and CoreCLR. |
| src/mono/wasm/Wasm.Build.Tests/Common/EnvironmentVariables.cs | Removes CoreCLR-only env vars that supported the bespoke payload plumbing. |
| src/mono/wasm/Wasm.Build.Tests/Common/BuildEnvironment.cs | Guards multithreading runtime pack assertion for CoreCLR workload lane. |
| src/mono/browser/build/BrowserWasmApp.CoreCLR.targets | Resolves include dirs from restored runtime pack; removes env var overrides. |
| src/libraries/sendtohelix.proj | Extends workload cross-product to CoreCLR while pinning fingerprinting defaults. |
| src/libraries/sendtohelix-wasm.targets | Makes work item fan-out depend on lane shape instead of runtime flavor. |
| src/libraries/sendtohelix-browser.targets | Removes CoreCLR-specific EMSDK/payload bridging; fixes CoreCLR trait arg composition. |
| eng/testing/workloads-browser.targets | Installs wasm-tools + wasm-experimental for CoreCLR workload testing. |
| eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml | Updates CI to support CoreCLR workload lane and stages Mono runtime packs required by the workload manifest. |
| <_XUnitTraitArg Condition="'$(WasmFingerprintAssets)' == 'false'">-trait category=no-fingerprinting</_XUnitTraitArg> | ||
| <_XUnitTraitArg Condition="'$(WasmBundlerFriendlyBootConfig)' == 'true'">-trait category=bundler-friendly</_XUnitTraitArg> |
| <!-- The workload lane fans out per test class; the no-workload lane (and the webcil / | ||
| fingerprinting / bundler permutations) runs as a single work item. Identical for | ||
| every runtime flavor - only $(BuildWasmAppsJobsList) differs. --> |
| buildArgs: >- | ||
| $(wbtProjectArg) $(Build.SourcesDirectory)/src/mono/wasm/Wasm.Build.Tests/Wasm.Build.Tests.csproj | ||
| /bl:$(Build.SourcesDirectory)/artifacts/log/$(_BuildConfig)/Build.binlog | ||
| /p:Configuration=$(_BuildConfig) | ||
| /p:TestUsingWorkloads=false | ||
| /p:TestUsingWorkloads=true | ||
| /p:InstallWorkloadForTesting=true | ||
| /p:WasmSkipMissingRuntimePackBuild=true | ||
| /p:PreparePackagesForWorkloadInstall=false |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d518821e-6489-46c4-8db6-848b28e0d693
| <!-- CoreCLR only runs the two workload lanes (no-workload and workload); the webcil and | ||
| fingerprinting permutations stay Mono-only. Fingerprinting is pinned to its default so | ||
| the lane still selects the same Helix work items as the Mono workload lane. --> | ||
| <_TestUsingFingerprintingValues Include="true" Condition="'$(TargetOS)' == 'browser' and '$(RuntimeFlavor)' == 'CoreCLR'" /> | ||
|
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d518821e-6489-46c4-8db6-848b28e0d693
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/mono/wasm/Wasm.Build.Tests/Wasm.Build.Tests.csproj:109
- The _XUnitTraitArg construction isn’t cumulative: the fingerprinting/bundler conditions overwrite the workload/no-workload selection. Now that CoreCLR appends to $(_XUnitTraitArg), this can drop the intended lane selection (especially for local runs with fingerprinting/bundler toggles). Use the same “append to existing” pattern as sendtohelix-browser.targets so all trait switches compose correctly.
<_XUnitTraitArg Condition="'$(TestUsingWorkloads)' == 'true'">-notrait category=no-workload</_XUnitTraitArg>
<_XUnitTraitArg Condition="'$(TestUsingWorkloads)' != 'true'">-trait category=no-workload</_XUnitTraitArg>
<_XUnitTraitArg Condition="'$(WasmFingerprintAssets)' == 'false'">-trait category=no-fingerprinting</_XUnitTraitArg>
<_XUnitTraitArg Condition="'$(WasmBundlerFriendlyBootConfig)' == 'true'">-trait category=bundler-friendly</_XUnitTraitArg>
<_XUnitTraitArg Condition="'$(RuntimeFlavor)' == 'CoreCLR'">$(_XUnitTraitArg) -notrait category=native-mono -notrait category=mono</_XUnitTraitArg>
src/libraries/sendtohelix.proj:87
- This comment no longer matches the actual lane cross-product: Mono also varies fingerprinting (and has a separate bundler-friendly lane), and CoreCLR now also has a workload/no-workload cross product. Updating the comment will prevent future confusion when modifying the lane matrix.
<!-- For BuildWasmApps we want to build the project 4 times, with: TestUsingWorkloads=true, and TestUsingWorkloads=false and WasmEnableWebcil=true and WasmEnableWebcil=false-->
eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml:122
- Spelling: “altough” should be “although”.
# Download WBT - TODO-WASM: This points to "mono" build, altough
Remove the generated override that disabled browser runtime workloads for CoreCLR test apps. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d752104-9131-48cb-bf89-b90d39d9f33f
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/libraries/sendtohelix.proj:87
- The comment claims BuildWasmApps builds “4 times”, but the logic below now builds a larger cross-product for Mono (workloads × fingerprinting × webcil, plus a JS-bundler lane) and only two lanes for CoreCLR. This comment is now misleading for anyone maintaining the lane matrix.
<!-- For BuildWasmApps we want to build the project 4 times, with: TestUsingWorkloads=true, and TestUsingWorkloads=false and WasmEnableWebcil=true and WasmEnableWebcil=false-->
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d518821e-6489-46c4-8db6-848b28e0d693
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
Contributes to #126100
Summary
In #127073 and #128296 Wasm.Build.Tests were enabled on CoreCLR at a time when the
wasm-toolsworkload did not support CoreCLR. To get native relink working on Helix, those PRs added a bespoke way of delivering build outputs to the Helix work items, and ran CoreCLR in a single ad-hoc configuration.Since #130380 the
wasm-toolsworkload works with CoreCLR, and the CoreCLR runtime pack shipsnative/include/{callhelpers.hpp,minipal/*.h}. This PR removes the custom delivery and makes CoreCLR run the same two lanes as Mono — oneNoWorkloadand one with the workload installed.Removing the custom build output delivery
eng/,src/native/minipalandsrc/coreclr/vm/wasmcorrelation payloads, and the CoreCLR-only EMSDK payload.HelixPreCommandenv vars that bridged those payloads to MSBuild properties (REPOSITORY_ENGINEERING_DIR,BROWSER_BUILD_TARGETS_DIR,WASM_APP_BUILDER_TASKS_ASSEMBLY_PATH,MINIPAL_INCLUDE_DIR,CORECLR_VM_WASM_INCLUDE_DIR) along with the test-side plumbing that consumed them:EnvironmentVariables.cs,BuildEnvironment.cs,data/Local.Directory.Build.{props,targets}(which manually importedBrowserWasmApp.CoreCLR.targets),RunScriptTemplate.shand the matchingRunScriptCommands.BrowserWasmApp.CoreCLR.targets; they now resolve from the restored runtime pack, with the in-repo fallback kept for full repo builds.Provision emsdkstep from the CoreCLR Wasm.Build.Tests job — emsdk comes from the workload's Emscripten packs.WasiApp.CoreCLR.targetsstill has the equivalent env-var overrides and is left untouched, as WASI CoreCLR still runs without a workload.Running the same lanes as Mono
sendtohelix.projnow produces the workload cross product for CoreCLR too, with webcil/fingerprinting pinned to their defaults so CoreCLR stays at exactly two lanes while Mono stays at six.sendtohelix-wasm.targetspicks the work item shape from the lane instead of the runtime flavor, so both flavors fan out per test class in the workload lane and use a single work item in the no-workload lane. The per-class fan-out is what keeps the (already slow) Windows leg within the work item timeout.category=workloadtests now actually run.workloads-browser.targetsinstallswasm-toolsandwasm-experimentalfor CoreCLR (the latter carries the WebAssembly templates pack the template-based tests need), and the pipeline job stages the Mono runtime packs those workloads list.Also guarded the multithreading runtime pack assertion in
BuildEnvironment, which would otherwise throw on the new CoreCLR workload lane — CoreCLR has no multithreaded browser-wasm runtime pack.Validation
Wasm.Build.Testscompiles cleanly.Verified by MSBuild evaluation that the lane/work-item logic produces, for the default fingerprinting/bundler values:
WBT-NoWorkload-MONO-ST-Wasm.Build.Tests-trait category=no-workloadWBT-Workloads-MONO-ST-<Class>-notrait category=no-workloadWBT-NoWorkload-CLR-ST-Wasm.Build.Tests-trait category=no-workload -notrait category=native-mono -notrait category=monoWBT-Workloads-CLR-ST-<Class>-notrait category=no-workload -notrait category=native-mono -notrait category=monoand that the CoreCLR cross product yields exactly 2 lanes with Mono unchanged at 6.
The rest needs CI to verify, hence the draft.
Note
This PR was generated with the assistance of GitHub Copilot.