Skip to content

[browser] More Wasm.Build.Tests on CoreCLR#127281

Draft
maraf wants to merge 7 commits intodotnet:mainfrom
maraf:maraf/wbt-coreclr-enable-non-aot-tests
Draft

[browser] More Wasm.Build.Tests on CoreCLR#127281
maraf wants to merge 7 commits intodotnet:mainfrom
maraf:maraf/wbt-coreclr-enable-non-aot-tests

Conversation

@maraf
Copy link
Copy Markdown
Member

@maraf maraf commented Apr 22, 2026

Note

This PR description was AI-generated (GitHub Copilot CLI).

Follow-up to #127073. Now that WASM templates install and native relink work on CoreCLR in Wasm.Build.Tests (WBT), enable the non-AOT tests that were previously excluded by the class-level [TestCategory("native")] trait filter.

What changes

src/mono/wasm/Wasm.Build.Tests/Wasm.Build.Tests.csproj excludes category=native on CoreCLR:

<_XUnitTraitArg Condition="'$(RuntimeFlavor)' == 'CoreCLR'">-notrait category=native -notrait category=mono -notrait category=workload</_XUnitTraitArg>

xunit merges class-level and method-level traits, so moving native to a sub-category (e.g. native-coreclr) at method level does not help — the class-level native still causes exclusion. The fix is to remove the class-level tag and re-apply native only on the AOT-only methods.

File Change
NativeBuildTests.cs Remove class tag. Method-tag AOTNotSupportedWithNoTrimming, IntermediateBitcodeToObjectFilesAreNotLLVMIR, NativeBuildIsRequired (all AOT-only). Non-AOT methods (SimpleNativeBuild, ZipArchiveInteropTest) now run on CoreCLR.
DllImportTests.cs Remove class tag. All methods are non-AOT — all run on CoreCLR.
PInvokeTableGeneratorTests.cs Remove class tag. Method-tag EnsureWasmAbiRulesAreFollowedInAOT, EnsureComInteropCompilesInAOT.
NativeLibraryTests.cs Remove class tag. Split the four theories that had both [BuildAndRun(aot:false)] and [BuildAndRun(aot:true,…)] into a non-AOT method and an _AOT method tagged native (they delegate to a shared …Core helper). The pre-existing [ActiveIssue(#103566)] on ProjectUsingSkiaSharp is applied to both halves.

Splitting (rather than runtime-skip) keeps test reporting honest: AOT-on-CoreCLR shows up as "excluded by filter" instead of a false pass.

Not in scope: other [TestCategory("native")] classes in WBT (BuildPublishTests, NativeRebuildTests/*, WasmSIMDTests, IcuTests, SatelliteAssembliesTests, …). Those often encode reasons beyond "class-level native is too broad" and are left for a follow-up.

Validation

  • Local build: ./dotnet.sh build src/mono/wasm/Wasm.Build.Tests/Wasm.Build.Tests.csproj -c Release /p:TargetOS=browser /p:RuntimeFlavor=CoreCLR /p:Scenario=BuildWasmApps → 0W/0E.
  • Helix end-to-end validation: this PR's CI.

Copilot AI review requested due to automatic review settings April 22, 2026 15:30

This comment was marked as resolved.

@maraf maraf added arch-wasm WebAssembly architecture test-enhancement Improvements of test source code os-browser Browser variant of arch-wasm labels Apr 22, 2026
@maraf maraf added this to the 11.0.0 milestone Apr 22, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

Remove class-level [TestCategory("native")] from NativeBuildTests,
NativeLibraryTests, PInvokeTableGeneratorTests, and DllImportTests so
non-AOT methods pass the CoreCLR xunit filter (-notrait category=native).
AOT-only methods get a method-level [TestCategory("native")] to keep
them excluded on CoreCLR.

NativeLibraryTests: four mixed theories (both aot=false and aot=true
inline data) are split into a non-AOT theory and a separate AOT-only
theory tagged 'native', because xunit trait filtering is per-method,
not per-theory-row.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 24, 2026 08:08
@maraf maraf force-pushed the maraf/wbt-coreclr-enable-non-aot-tests branch from 07aa682 to 8debc4a Compare April 24, 2026 08:08

This comment was marked as resolved.

Copilot AI and others added 2 commits April 24, 2026 10:30
Change [TestCategory("native")] to [TestCategory("native-coreclr")] for tests
that use native relink but should also run on CoreCLR:

- IcuShardingTests: CustomIcuShard, AutomaticShardSelectionDependingOnEnvLocale
- IcuShardingTests2: DefaultAvailableIcuShardsFromRuntimePack
- IcuTests: FullIcuFromRuntimePackWithInvariant, FullIcuFromRuntimePackWithCustomIcu
- InvariantGlobalizationTests: RelinkingWithoutAOT
- MemoryTests: entire class (AllocateLargeHeapThenRepeatedlyInterop)

CoreCLR always has native relink support, so these tests can run on both
Mono and CoreCLR. The CoreCLR xunit filter excludes category=native but
not category=native-coreclr.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Without these entries in BuildWasmAppsJobsListCoreCLR.txt, the test
classes whose class-level [TestCategory("native")] was removed (or
retagged to native-coreclr) in this PR would still not be scheduled
as Helix work items for the CoreCLR BuildWasmApps scenario.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 24, 2026 10:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Comment on lines 54 to 57
[Theory]
[MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { Configuration.Release })]
[TestCategory("native")]
[TestCategory("native-coreclr")]
public async Task FullIcuFromRuntimePackWithInvariant(Configuration config=Configuration.Release, bool aot=false, bool invariant=true, bool fullIcu=true, string testedLocales="Array.Empty<Locale>()") =>
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing these theories to TestCategory("native-coreclr") makes them eligible to run under the CoreCLR lane filter (which only excludes category=native). However, the test data includes aot=true cases (boolOptions), and CoreCLR builds set <UseMonoRuntime>false</UseMonoRuntime>; AOT compilation is likely unsupported and will attempt to run anyway. Consider splitting into non-AOT methods (tagged native-coreclr) and AOT methods (keep category=native so they remain excluded on CoreCLR), or adjust the MemberData to exclude aot=true when BuildTestBase.IsCoreClrRuntime.

Copilot uses AI. Check for mistakes.
Comment on lines 42 to 45
[Theory]
[MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { Configuration.Release })]
[TestCategory("native")]
[TestCategory("native-coreclr")]
public async Task CustomIcuShard(Configuration config, bool aot, string customIcuPath, string customLocales, bool onlyPredefinedCultures) =>
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests use boolOptions for the aot parameter (so they generate AOT and non-AOT variants). Retagging them to native-coreclr will cause the AOT variants to run on the CoreCLR lane (since only category=native is excluded), which is likely unsupported for CoreCLR (UseMonoRuntime=false). Split/tag AOT variants as category=native (excluded on CoreCLR) or filter aot=true out for CoreCLR runs.

Copilot uses AI. Check for mistakes.
Comment on lines 39 to 42
[Theory]
[MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { Configuration.Release })]
[TestCategory("native")]
[TestCategory("native-coreclr")]
public async Task DefaultAvailableIcuShardsFromRuntimePack(Configuration config, bool aot, string shardName, string testedLocales) =>
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This theory generates both aot=false and aot=true cases via boolOptions. After changing the category to native-coreclr, the AOT variants will no longer be excluded by the CoreCLR trait filter (-notrait category=native) and may run on CoreCLR where AOT is likely unsupported. Consider splitting/tagging AOT variants as category=native (excluded) or changing the data source to omit aot=true on CoreCLR.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to 17
[TestCategory("native-coreclr")]
public class MemoryTests : WasmTemplateTestsBase
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the class retagged to native-coreclr, this test will start running in the CoreCLR lane. The build assertion currently uses ExpectSuccess: BuildTestBase.IsUsingWorkloads, but CoreCLR WBT runs with TestUsingWorkloads=false while native relink is now supported (emsdk is provisioned separately). That combination likely makes the test fail (it will expect the native build to fail even if it succeeds). Consider basing ExpectSuccess/execution on whether native builds are actually available (e.g., IsUsingWorkloads || IsCoreClrRuntime), or keep the test excluded until its expectations are updated.

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +23
Wasm.Build.Tests.IcuShardingTests
Wasm.Build.Tests.IcuShardingTests2
Wasm.Build.Tests.IcuTests
Wasm.Build.Tests.InvariantGlobalizationTests
Wasm.Build.Tests.MemoryTests
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says additional TestCategory("native") classes like IcuTests are "not in scope" and left for follow-up, but this change adds Icu*, InvariantGlobalizationTests, and MemoryTests to the CoreCLR job list. Either update the PR description to reflect the expanded scope or remove these additions to match the stated intent.

Copilot uses AI. Check for mistakes.
maraf and others added 2 commits April 24, 2026 12:41
EnsureWasmTemplatesInstalled placed .dotnet-cli-home next to the
template nupkg (which on Linux Helix agents is under the read-only
correlation payload), causing 'Read-only file system' / 'Access denied'
errors for every test class that calls CreateWasmTemplateProject.

Always use BuildEnvironment.TmpPath — the harness's writable scratch
dir — instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
EnsureWasmTemplatesInstalled set DOTNET_CLI_HOME to TmpPath/.dotnet-cli-home
when running 'dotnet new install'. The subsequent 'dotnet new <template>'
call goes through DotNetCommand(useDefaultArgs: false), which does not
apply BuildEnvironment.EnvVars to the child process, so it inherited the
Helix-set DOTNET_CLI_HOME (e.g. /root/helix/work/workitem/e/.dotnet).

Two different DOTNET_CLI_HOME values mean two different template caches:
the install lands in one, the lookup misses in the other, producing
'No templates or subcommands found matching: wasmbrowser' (exit 103) on
every test class that calls CreateWasmTemplateProject.

Prefer the inherited DOTNET_CLI_HOME so install and lookup share a cache;
fall back to TmpPath only when not set (preserves the read-only-FS fix
for environments without an inherited value).

Reproduced locally with DOTNET_CLI_HOME=/tmp/helix-cli-home; templates
were installed under wbt artifacts/.dotnet-cli-home but 'dotnet new'
read from /tmp/helix-cli-home. After the fix, both paths share
/tmp/helix-cli-home and 'dotnet new wasmbrowser' succeeds.

Also adds [diag] Console.WriteLine entries logging both the install-time
and invocation-time DOTNET_CLI_HOME so Helix work-item console logs make
similar future regressions trivial to diagnose.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 27, 2026 08:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTestsBase.cs:104

  • EnsureWasmTemplatesInstalled sets DOTNET_CLI_HOME only for the dotnet new install process. The subsequent dotnet new <template> invocation (via DotNetCommand(..., useDefaultArgs:false)) does not set DOTNET_CLI_HOME, so it may look in a different template cache when DOTNET_CLI_HOME is not already set in the test process environment (e.g., local runs). To make this robust, propagate the chosen dotnetCliHome to the dotnet new <template> command (or set it on the current process via Environment.SetEnvironmentVariable before invoking dotnet new).
        // [diag] Log DOTNET_CLI_HOME inherited by the `dotnet new <template>` invocation.
        // The template engine reads installed templates from this location; if it differs
        // from the DOTNET_CLI_HOME used by EnsureWasmTemplatesInstalled, the template
        // won't be found.
        string inheritedCliHome = Environment.GetEnvironmentVariable("DOTNET_CLI_HOME") ?? "<unset>";
        string envVarsCliHome = s_buildEnv.EnvVars.TryGetValue("DOTNET_CLI_HOME", out string? evCliHome) ? evCliHome : "<unset-in-EnvVars>";
        Console.WriteLine($"[diag] DOTNET_CLI_HOME inherited by `dotnet new {template.ToString().ToLower()}`: '{inheritedCliHome}' (buildEnv.EnvVars: '{envVarsCliHome}')");

        using DotNetCommand cmd = new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false);
        CommandResult result = cmd.WithWorkingDirectory(_projectDir)
            .WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
            .ExecuteWithCapturedOutput($"new {template.ToString().ToLower()} {extraArgs}")
            .EnsureSuccessful();

maraf and others added 2 commits April 27, 2026 10:26
- Inline callhelpers.hpp / minipal/entrypoints.h declarations into the
  pre-included coreclr_compat.h and stop emitting #include lines for
  those headers from the CoreCLR generators, so pinvoke-table.cpp and
  wasm_m2n_invoke.g.cpp compile on Helix where the in-repo header paths
  do not exist.
- Make BuildEnvironment.GetRuntimePackDir flavor-aware: on CoreCLR the
  runtime pack is Microsoft.NETCore.App.Runtime.<rid> (no .Mono. segment).
- Mark DllImportTests.NativeLibraryWithVariadicFunctions as Mono-only;
  CoreCLR's PInvoke generator does not emit the expected varargs warning.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
….targets

The previous attempt to generate coreclr_compat.h from a _CompatHeaderLines
ItemGroup hit an MSBuild item-spec dedup/parsing issue: items whose Include
value contained both `{...}` braces and embedded `%3B` semicolons were
silently dropped from the output (verified with a minimal repro). That left
the generated header with missing `StringToWasmSigThunk` and
`ReverseThunkMapEntry` struct declarations, so pinvoke-table.cpp /
wasm_m2n_invoke.g.cpp failed to compile on Helix.

Replace the inline ItemGroup generation with a static coreclr_compat.h next
to BrowserWasmApp.CoreCLR.targets and `-include` it via
$(MSBuildThisFileDirectory). The file ships into the Helix correlation
payload via the existing HelixCorrelationPayload Include="$(BrowserBuildTargetsDir)"
mapping (sendtohelix-browser.targets), so it lands at
correlation/build/wasm/coreclr_compat.h alongside the targets file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 27, 2026 11:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Comment on lines +21 to +28
// CoreCLR type stubs
#ifndef _CORECLR_COMPAT_TYPES
#define _CORECLR_COMPAT_TYPES
typedef void MethodDesc;
typedef uintptr_t PCODE;
typedef uint32_t ULONG;
#define INTERP_STACK_SLOT_SIZE 8u
#endif
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coreclr_compat.h is intended to replace <minipal/utils.h>, but it doesn’t currently define NOINLINE. The generated wasm_m2n_invoke.g.cpp uses NOINLINE for portable-entrypoint thunks, so compilation will fail unless this macro is provided (or the generator includes the proper header again).

Copilot uses AI. Check for mistakes.
// from the DOTNET_CLI_HOME used by EnsureWasmTemplatesInstalled, the template
// won't be found.
string inheritedCliHome = Environment.GetEnvironmentVariable("DOTNET_CLI_HOME") ?? "<unset>";
string envVarsCliHome = s_buildEnv.EnvVars.TryGetValue("DOTNET_CLI_HOME", out string? evCliHome) ? evCliHome : "<unset-in-EnvVars>";
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EnvVars is an IDictionary<string, string> (non-null values), but this uses out string? evCliHome and assigns it to a non-null string. With #nullable enable, this is likely to introduce a nullable warning (and may fail the build if warnings are treated as errors). Use out string evCliHome (or null-coalesce) to keep nullability consistent.

Suggested change
string envVarsCliHome = s_buildEnv.EnvVars.TryGetValue("DOTNET_CLI_HOME", out string? evCliHome) ? evCliHome : "<unset-in-EnvVars>";
string envVarsCliHome = s_buildEnv.EnvVars.TryGetValue("DOTNET_CLI_HOME", out string evCliHome) ? evCliHome : "<unset-in-EnvVars>";

Copilot uses AI. Check for mistakes.
Comment on lines +227 to 236
// writable workitem path); otherwise fall back to TmpPath. Aligning with
// the inherited value ensures `dotnet new install` and the subsequent
// `dotnet new <template>` invocation share the same template cache —
// those `dotnet new <template>` calls (via DotNetCommand with
// useDefaultArgs:false) inherit DOTNET_CLI_HOME from the test process.
string? inheritedCliHome = Environment.GetEnvironmentVariable("DOTNET_CLI_HOME");
string dotnetCliHome = !string.IsNullOrWhiteSpace(inheritedCliHome)
? inheritedCliHome
: Path.Combine(BuildEnvironment.TmpPath, ".dotnet-cli-home");
Directory.CreateDirectory(dotnetCliHome);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When DOTNET_CLI_HOME is not set in the test process environment, EnsureWasmTemplatesInstalled installs templates into BuildEnvironment.TmpPath via psi.Environment["DOTNET_CLI_HOME"], but the later dotnet new <template> invocation does not set DOTNET_CLI_HOME (and DotNetCommand is created with useDefaultArgs:false, so it won’t apply s_buildEnv.EnvVars). This makes the fallback path inconsistent and can cause templates to not be found on local runs. Consider setting DOTNET_CLI_HOME for the current process and/or passing it explicitly into the dotnet new command as well.

Suggested change
// writable workitem path); otherwise fall back to TmpPath. Aligning with
// the inherited value ensures `dotnet new install` and the subsequent
// `dotnet new <template>` invocation share the same template cache —
// those `dotnet new <template>` calls (via DotNetCommand with
// useDefaultArgs:false) inherit DOTNET_CLI_HOME from the test process.
string? inheritedCliHome = Environment.GetEnvironmentVariable("DOTNET_CLI_HOME");
string dotnetCliHome = !string.IsNullOrWhiteSpace(inheritedCliHome)
? inheritedCliHome
: Path.Combine(BuildEnvironment.TmpPath, ".dotnet-cli-home");
Directory.CreateDirectory(dotnetCliHome);
// writable workitem path); otherwise fall back to TmpPath. Persist the
// chosen value into the current test process as well so subsequent
// `dotnet new <template>` invocations (via DotNetCommand with
// useDefaultArgs:false) inherit the same template cache location.
string? inheritedCliHome = Environment.GetEnvironmentVariable("DOTNET_CLI_HOME");
string dotnetCliHome = !string.IsNullOrWhiteSpace(inheritedCliHome)
? inheritedCliHome
: Path.Combine(BuildEnvironment.TmpPath, ".dotnet-cli-home");
Directory.CreateDirectory(dotnetCliHome);
Environment.SetEnvironmentVariable("DOTNET_CLI_HOME", dotnetCliHome);

Copilot uses AI. Check for mistakes.
Comment on lines +274 to +278
<!-- All declarations needed by ManagedToNativeGenerator output (pinvoke-table.cpp,
wasm_m2n_invoke.g.cpp) are provided by coreclr_compat.h, which ships next to this
targets file. This avoids relying on the in-repo coreclr/vm/wasm and
native/minipal headers, which are not present in the WBT Helix payload. -->
<_EmccCFlags Include="-include &quot;$(MSBuildThisFileDirectory)coreclr_compat.h&quot;" />
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_CoreCLRGenerateManagedToNative still adds Dependencies="$(_WasmCompileRsp);$(_WasmIntermediateOutputPath)coreclr_compat.h" for the generated pinvoke/interp sources later in the file, but this PR removes the intermediate coreclr_compat.h generation and switches to $(MSBuildThisFileDirectory)coreclr_compat.h. Update those remaining Dependencies values to match the new header location (or drop the dependency) to keep incremental builds correct.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm test-enhancement Improvements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants