Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[browser][Wasm.Build.Tests] Use trimmer friendly JSON serialization #94034

Merged
merged 3 commits into from
Oct 28, 2023
Merged

[browser][Wasm.Build.Tests] Use trimmer friendly JSON serialization #94034

merged 3 commits into from
Oct 28, 2023

Conversation

maraf
Copy link
Member

@maraf maraf commented Oct 26, 2023

  • Use JsonSerializer.Serialize overload with JsonSerializerContext
  • Remove SuppressTrimAnalysisWarnings=true from test project

@maraf maraf added arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm labels Oct 26, 2023
@maraf maraf added this to the 9.0.0 milestone Oct 26, 2023
@maraf maraf self-assigned this Oct 26, 2023
@ghost
Copy link

ghost commented Oct 26, 2023

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

Issue Details
  • Use JsonSerializer.Serialize overload with JsonSerializerContext
  • Remove SuppressTrimAnalysisWarnings=true from test project
Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-Build-mono, os-browser
Milestone: 9.0.0

@radical
Copy link
Member

radical commented Oct 26, 2023

@maraf I pushed a change - CI: trigger only WBT on testasset changes

@maraf
Copy link
Member Author

maraf commented Oct 27, 2023 •
edited
Loading

The test has failed, but Helix marked the test as passed https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-94034-merge-f01587ce73c24c259e/Workloads-NoWebcil-Wasm.Build.Tests.TestAppScenarios.LazyLoadingTests/1/xharness-output/testResults.xml?helixlogtype=result

EDIT: It is expected for this test, my bad

@radical
Copy link
Member

radical commented Oct 27, 2023

The test has failed, but Helix marked the test as passed helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-94034-merge-f01587ce73c24c259e/Workloads-NoWebcil-Wasm.Build.Tests.TestAppScenarios.LazyLoadingTests/1/xharness-output/testResults.xml?helixlogtype=result

That results file has:
<test name="Wasm.Build.Tests.TestAppScenarios.LazyLoadingTests.FailOnMissingLazyAssembly" type="Wasm.Build.Tests.TestAppScenarios.LazyLoadingTests" method="FailOnMissingLazyAssembly" time="79.3860332" result="Pass">

.. so xunit marked the test as Pass. Also, the output has:

[error] MONO_WASM:    at LazyLoadingTest.__Wrapper_Run_251373279(JSMarshalerArgument* __arguments_buffer) in C:\helix\work\workitem\e\wbt\LazyLoadingTests_ktk5k3bn_km0\Microsoft.Interop.JavaScript.JSImportGenerator\Microsoft.Inter>
Error: Could not load file or assembly 'System.Text.Json, Version=9.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies.
    at Qn (http://127.0.0.1:49204/_framework/dotnet.runtime.9.0.0-ci.svs40qok5n.js:3:32554)
    at uo (http://127.0.0.1:49204/_framework/dotnet.runtime.9.0.0-ci.svs40qok5n.js:3:63537)
    at Object.<anonymous> (http://127.0.0.1:49204/_framework/dotnet.runtime.9.0.0-ci.svs40qok5n.js:3:182394)
    at http://127.0.0.1:49204/main.js:84:37
[info] WASM EXIT 1

.. which seems to be message expected from the failed run.

All that sounds correct. What am I missing?

@radical
Copy link
Member

radical commented Oct 27, 2023

In a follow PR I'll move testassets to be under WBT folder, as it is used only for that.

@maraf
Copy link
Member Author

maraf commented Oct 27, 2023

All that sounds correct. What am I missing?

Sorry, my bad. It is expected for that test.

@maraf maraf marked this pull request as ready for review October 27, 2023 21:44
@maraf maraf requested a review from radical as a code owner October 27, 2023 21:44
@lewing
Copy link
Member

lewing commented Oct 27, 2023 •
edited
Loading

was I missing the attribute before? or did I misread the failure too...

@radical radical merged commit c2608bd into dotnet:main Oct 28, 2023
22 checks passed
@maraf
Copy link
Member Author

maraf commented Oct 30, 2023

was I missing the attribute before? or did I misread the failure too...

The test is expected to fail and exit code is exptected to be 1.

@maraf maraf deleted the WasmTestAppScenariosFixJson branch October 30, 2023 09:29
liveans pushed a commit to liveans/dotnet-runtime that referenced this pull request Nov 9, 2023
@maraf @radical @liveans
[browser][Wasm.Build.Tests] Use trimmer friendly JSON serialization (d…
9701d18 
…otnet#94034)

* Use trimmer friendly JSON serialization

* CI: trigger only WBT on testasset changes

* CI: don't trigger non-wbt wasm jobs on testassets changes

---------

Co-authored-by: Ankit Jain <radical@gmail.com>
@ghost ghost locked as resolved and limited conversation to collaborators Nov 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@radical radical radical approved these changes

@lewing lewing lewing approved these changes

Assignees

@maraf maraf

Labels
arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@maraf @radical @lewing