[wasm][coreclr] Re-enable System.Formats.Nrbf tests#129334
Open
radekdoulik wants to merge 1 commit into
Open
Conversation
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>
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
radekdoulik
changed the title
Contributor
There was a problem hiding this comment.
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
ReferenceCopyLocalPathsentries when an app-local assembly with the same name exists (so the app-local assembly is the one bundled). - Remove the
System.Formats.Nrbf.Testsexclusion fromsrc/libraries/tests.projfor 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 on lines
+195
to
+213
| <Target Name="_CoreCLRPreferAppLocalAssembliesOverRuntimePack" | ||
| BeforeTargets="_ComputeWasmBuildCandidates;_GatherWasmFilesToBuild" | ||
| Condition="'$(IsBrowserWasmProject)' == 'true' and '$(MicrosoftNetCoreAppRuntimePackDir)' != ''"> | ||
| <ItemGroup> | ||
| <!-- App-local (non-runtime-pack) managed assemblies copied next to the app. --> | ||
| <_CoreCLRAppLocalAssembly Include="@(ReferenceCopyLocalPaths)" | ||
| Condition="'%(Extension)' == '.dll' and !$([System.String]::Copy('%(FullPath)').StartsWith('$(MicrosoftNetCoreAppRuntimePackDir)'))" /> | ||
| </ItemGroup> | ||
| <PropertyGroup> | ||
| <_CoreCLRAppLocalAssemblyNames>;@(_CoreCLRAppLocalAssembly->'%(FileName)%(Extension)');</_CoreCLRAppLocalAssemblyNames> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <!-- Drop runtime-pack copies shadowed by a same-named app-local assembly. --> | ||
| <ReferenceCopyLocalPaths Remove="@(ReferenceCopyLocalPaths)" | ||
| Condition="'%(Extension)' == '.dll' | ||
| and $([System.String]::Copy('%(FullPath)').StartsWith('$(MicrosoftNetCoreAppRuntimePackDir)')) | ||
| and $(_CoreCLRAppLocalAssemblyNames.Contains(';%(FileName)%(Extension);'))" /> | ||
| </ItemGroup> | ||
| </Target> |
| @@ -185,8 +185,6 @@ | |||
|
|
|||
| <!-- https://github.com/dotnet/runtime/issues/125495 - These tests are disabled on browser/CoreCLR because of crashes and test failures. --> | |||
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
Re-enables the
System.Formats.Nrbf.Testssuite on browser-wasm + CoreCLR (removes theProjectExclusionsentry insrc/libraries/tests.proj) and fixes the underlying bundling bug that made it crash.Problem
On browser-wasm + CoreCLR, test discovery crashed with:
System.Runtime.Serialization.Formattershas 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 taskComputeWasmBuildAssetsdedupes 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.Loadof 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
deps.jsonresolution prefers the higher version.ResolvedFileToPublishset (ComputeResolvedFilesToPublishListprefers the higherAssemblyVersion), 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 (
_CoreCLRPreferAppLocalAssembliesOverRuntimePackinBrowserWasmApp.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:_frameworkFormatters webcil: 69913 → 156953 bytes (8.1.0.0 stub → functional 11.0.0.0).FileNotFoundException.ReadTestshierarchy correctly skips on browser via[ConditionalClass(IsBinaryFormatterSupported)](BinaryFormatter is unsupported on browser by design), leaving the 4 platform-independentStartsWithPayloadHeaderTestscases running and passing — the same behavior as any platform without BinaryFormatter support.Note
This pull request was authored by GitHub Copilot.