Skip to content

[wasm][coreclr] Re-enable System.Formats.Nrbf tests#129334

Open
radekdoulik wants to merge 5 commits into
dotnet:mainfrom
radekdoulik:radekdoulik/wasm-coreclr-lib-test-nrbf
Open

[wasm][coreclr] Re-enable System.Formats.Nrbf tests#129334
radekdoulik wants to merge 5 commits into
dotnet:mainfrom
radekdoulik:radekdoulik/wasm-coreclr-lib-test-nrbf

Conversation

@radekdoulik

Copy link
Copy Markdown
Member

Summary

Re-enables the System.Formats.Nrbf.Tests suite on browser-wasm + CoreCLR (removes the ProjectExclusions entry in src/libraries/tests.proj) and fixes the underlying bundling bug that made it crash.

Problem

On browser-wasm + CoreCLR, test discovery crashed with:

FileNotFoundException: System.Runtime.Serialization.Formatters, Version=11.0.0.0

System.Runtime.Serialization.Formatters has a version split: the shared framework ships a non-functional 8.1.0.0 stub, while the Nrbf test project references the functional 11.0.0.0 build app-local (Private="true" SetTargetFramework="...NetCoreAppMinimum").

For a self-contained browser-wasm app, @(ReferenceCopyLocalPaths) ends up with both copies. The WebAssembly SDK task ComputeWasmBuildAssets dedupes webcil bundle candidates by relative path on a first-wins, version-blind basis, so the 8.1.0.0 stub got converted to webcil and bundled into _framework, shadowing the app-local 11.0.0.0 copy. At runtime, xunit discovery materializes [InlineData(FormatterTypeStyle.X)] attribute blobs → Assembly.Load of Formatters 11.0.0.0 → only the stub is present → FileNotFoundException → discovery aborts (only 6 of ~153 cases were ever found).

Why desktop isn't affected

  • Framework-dependent (desktop default): the shared framework / stub is never copied app-local; only the 11.0.0.0 copy is in the output and the host's deps.json resolution prefers the higher version.
  • Single-file publish: the HostModel bundler packs the already version-conflict-resolved ResolvedFileToPublish set (ComputeResolvedFilesToPublishList prefers the higher AssemblyVersion), so the stub is dropped before bundling.

The wasm webcil bundle is assembled at build time from candidates that are not version-conflict-resolved against ProjectReferences, so it bypassed the resolution that protects the desktop paths.

Fix

Add a CoreCLR-wasm target (_CoreCLRPreferAppLocalAssembliesOverRuntimePack in BrowserWasmApp.CoreCLR.targets) that applies copy-local-wins semantics before the bundle candidates are gathered: it drops runtime-pack copy-local assemblies that are shadowed by a same-named app-local copy. This matches how a self-contained app actually loads assemblies. Apps without an app-local copy of a framework assembly are unaffected (nothing shadows the runtime-pack copy).

Validation

Ran ./dotnet.sh build -c Debug /t:Test src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj /p:TargetOS=browser /p:TargetArchitecture=wasm /p:RuntimeFlavor=CoreCLR /p:Scenario=WasmTestOnChrome:

  • Bundled _framework Formatters webcil: 69913 → 156953 bytes (8.1.0.0 stub → functional 11.0.0.0).
  • Discovery: 6 → 153 test cases, no FileNotFoundException.
  • Result: 4 passed, 0 failed, exit 0. The ReadTests hierarchy correctly skips on browser via [ConditionalClass(IsBinaryFormatterSupported)] (BinaryFormatter is unsupported on browser by design), leaving the 4 platform-independent StartsWithPayloadHeaderTests cases running and passing — the same behavior as any platform without BinaryFormatter support.

Note

This pull request was authored by GitHub Copilot.

The System.Formats.Nrbf.Tests suite was excluded on browser-wasm +
CoreCLR because test discovery crashed with a FileNotFoundException for
System.Runtime.Serialization.Formatters, Version=11.0.0.0.

Root cause: for a self-contained browser-wasm app, @(ReferenceCopyLocalPaths)
contains two copies of System.Runtime.Serialization.Formatters - the shared
framework's non-functional 8.1.0.0 stub (from the runtime pack) and the
functional 11.0.0.0 build the test references app-local. The WebAssembly SDK
task ComputeWasmBuildAssets dedupes webcil bundle candidates by relative path
on a first-wins, version-blind basis, so the 8.1.0.0 stub got bundled into
_framework and shadowed the app-local 11.0.0.0 copy. xunit discovery then
materializes [InlineData(FormatterTypeStyle.X)] attribute blobs, triggering an
Assembly.Load of the 11.0.0.0 build, which is not present - aborting discovery.

Unlike desktop (framework-dependent deployment, or single-file publish whose
bundler consumes the already version-resolved ResolvedFileToPublish set), the
wasm webcil bundle is assembled at build time from candidates that are not
version-conflict-resolved against ProjectReferences.

Fix: add a CoreCLR-wasm target that applies copy-local-wins semantics before
the bundle candidates are gathered - it drops runtime-pack copy-local
assemblies that are shadowed by a same-named app-local copy. Apps without an
app-local copy of a framework assembly are unaffected.

With the fix the bundled Formatters webcil is the functional 11.0.0.0 build,
discovery finds 153 test cases (was 6) and the suite passes; the ReadTests
hierarchy correctly skips on browser since BinaryFormatter is unsupported there.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 12, 2026 15:29
@radekdoulik radekdoulik requested a review from maraf as a code owner June 12, 2026 15:29
@radekdoulik radekdoulik added this to the Future milestone Jun 12, 2026
@radekdoulik radekdoulik added arch-wasm WebAssembly architecture area-Build-mono labels Jun 12, 2026
@radekdoulik radekdoulik requested a review from akoeplinger as a code owner June 12, 2026 15:29
@radekdoulik radekdoulik added arch-wasm WebAssembly architecture area-Build-mono labels Jun 12, 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.

@radekdoulik radekdoulik changed the title [wasm] Re-enable System.Formats.Nrbf tests on browser CoreCLR [wasm][coreclr] Re-enable System.Formats.Nrbf tests Jun 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR re-enables System.Formats.Nrbf.Tests for browser-wasm + CoreCLR by removing the test exclusion and adding an MSBuild-time workaround to ensure app-local assemblies win over runtime-pack copies when building the WASM bundle.

Changes:

  • Add a CoreCLR browser-wasm target that removes runtime-pack ReferenceCopyLocalPaths entries when an app-local assembly with the same name exists (so the app-local assembly is the one bundled).
  • Remove the System.Formats.Nrbf.Tests exclusion from src/libraries/tests.proj for browser/CoreCLR.

Reviewed changes

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

File Description
src/mono/browser/build/BrowserWasmApp.CoreCLR.targets Adds a pre-bundling MSBuild target to prefer app-local assemblies over runtime-pack copies for browser-wasm CoreCLR builds.
src/libraries/tests.proj Re-enables System.Formats.Nrbf.Tests on browser/CoreCLR by removing its ProjectExclusions entry.

Comment thread src/mono/browser/build/BrowserWasmApp.CoreCLR.targets
Comment thread src/libraries/tests.proj Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 15, 2026 08:43
$(MicrosoftNetCoreAppRuntimePackDir) is not normalized in this file (it can
come straight from %(ResolvedRuntimePack.PackageDirectory) or a global
property), so a missing trailing separator or non-native path separators could
make the StartsWith checks fail to identify runtime-pack assemblies, silently
skipping the dedupe. Normalize it once with [MSBuild]::NormalizeDirectory and
use that for both checks so it reliably matches %(FullPath).

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread src/mono/browser/build/BrowserWasmApp.CoreCLR.targets
The target previously only ran when $(MicrosoftNetCoreAppRuntimePackDir) was
set, but WBT/Helix-created CoreCLR-wasm projects can leave that property empty
and rely on %(ResolvedRuntimePack.PackageDirectory) instead, so the target was
skipped and the runtime-pack vs app-local shadowing persisted for them.

Resolve the runtime pack dir inside the target the same way the WebAssembly SDK
does (Browser.targets): prefer the property, fall back to the ResolvedRuntimePack
metadata, then normalize. Both ItemGroups are guarded so an unresolved dir is a
safe no-op.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 15, 2026 10:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread src/mono/browser/build/BrowserWasmApp.CoreCLR.targets
shadowed by a same-named app-local copy before the candidates are gathered. Apps without an app-local
copy of a framework assembly are unaffected.
-->
<Target Name="_CoreCLRPreferAppLocalAssembliesOverRuntimePack"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why don't we need the same thing on Mono?
My understanding was that this is "handled" in more general place for runtime repository

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good question. This test suite is disabled for mono (for different reason - timeout), so I assume it has the same problem on mono too.

Which brings another question, should we rather fix it in the ComputeWasmBuildAssets instead?

_GatherWasmFilesToBuild has no DependsOnTargets, so a target hooked only
via BeforeTargets could run before ResolveReferences populates
@(ReferenceCopyLocalPaths), making the dedupe a no-op. Add
DependsOnTargets=ResolveReferences so the prune is correct-by-construction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants