From 224548370f1b7855b78c6d8b1b2e6f5f7383d1af Mon Sep 17 00:00:00 2001
From: Ankit Jain <radical@gmail.com>
Date: Fri, 9 Apr 2021 14:16:22 -0400
Subject: [PATCH 1/2] [wasm][tests] Enable line numbers in stack traces

Library tests don't show line numbers, essentially because the .pdb
files never become available to the WasmAppBuilder .

This is enabled by default for `Debug` builds only.

To use it with `Release` builds, add `/p:DebuggerSupport=true` to the
command line.

With the patch:

```
  fail: [FAIL] System.Reflection.Tests.MemberInfoTests.HasSameMetadataDefinitionAs__CornerCase_HasElementTypes
  info: Assert.All() Failure: 10 out of 10 items in the collection did not pass.
  info: [9]: Item: System.Double*
  info:      Xunit.Sdk.AllException: Assert.All() Failure: 2 out of 10 items in the collection did not pass.
  info:      [7]: Item: System.Double&
  info:           Xunit.Sdk.TrueException: Assert.True() Failure
  info:           Expected: True
  info:           Actual:   False
  info:              at Xunit.Assert.True(Nullable`1 condition, String userMessage) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\BooleanAsserts.cs:line 95
  info:              at Xunit.Assert.True(Boolean condition) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\BooleanAsserts.cs:line 62
  info:              at System.Reflection.Tests.MemberInfoTests.<>c__DisplayClass31_1.<HasSameMetadataDefinitionAs__CornerCase_HasElementTypes>b__1(Type t2) in /Users/radical/dev/runtime/src/libraries/System.Reflection/tests/MemberInfoTests.cs:line 491
  info:              at Xunit.Assert.All[Type](IEnumerable`1 collection, Action`1 action) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\CollectionAsserts.cs:line 36
```
---
 .../testing/libraries/testing-wasm.md         |  6 ++++++
 eng/testing/tests.wasm.targets                | 21 ++++++++++++++++++-
 src/mono/wasm/build/WasmApp.targets           |  5 +++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/docs/workflow/testing/libraries/testing-wasm.md b/docs/workflow/testing/libraries/testing-wasm.md
index 242e6c9cee4a90..f060a00f9b4588 100644
--- a/docs/workflow/testing/libraries/testing-wasm.md
+++ b/docs/workflow/testing/libraries/testing-wasm.md
@@ -144,6 +144,12 @@ At the moment supported values are:
 
 By default, `chrome` browser is used.
 
+## Debugging
+
+### Getting more information
+
+- Line numbers: add `/p:DebuggerSupport=true` to the command line, for `Release` builds. It's enabled by default for `Debug` builds.
+
 ## Kicking off outer loop tests from GitHub Interface
 
 Add the following to the comment of a PR.
diff --git a/eng/testing/tests.wasm.targets b/eng/testing/tests.wasm.targets
index 505b3a8bde603a..09311ffd96ef71 100644
--- a/eng/testing/tests.wasm.targets
+++ b/eng/testing/tests.wasm.targets
@@ -2,6 +2,7 @@
   <!-- We need to set this in order to get extensibility on xunit category traits and other arguments we pass down to xunit via MSBuild properties -->
   <PropertyGroup>
     <BundleTestAppTargets>$(BundleTestAppTargets);BundleTestWasmApp</BundleTestAppTargets>
+    <DebuggerSupport Condition="'$(DebuggerSupport)' == '' and '$(Configuration)' == 'Debug'">true</DebuggerSupport>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(RunScriptCommand)' == ''">
@@ -27,7 +28,9 @@
     <UseSystemResourceKeys>true</UseSystemResourceKeys>
     <EnableUnsafeUTF7Encoding>false</EnableUnsafeUTF7Encoding>
     <HttpActivityPropagationSupport>false</HttpActivityPropagationSupport>
-    <DebuggerSupport>false</DebuggerSupport>
+
+    <!-- we want to default to what Blazor has, except if we are building in Debug config -->
+    <DebuggerSupport Condition="'$(Configuration)' != 'Debug'">false</DebuggerSupport>
   </PropertyGroup>
 
   <!-- Don't include InTree.props here, because the test projects themselves can set the target* properties -->
@@ -62,6 +65,9 @@
       <WasmInvariantGlobalization>$(InvariantGlobalization)</WasmInvariantGlobalization>
       <WasmGenerateRunV8Script>true</WasmGenerateRunV8Script>
       <WasmNativeStrip>false</WasmNativeStrip>
+
+      <WasmNativeDebugSymbols Condition="'$(DebuggerSupport)' == 'true' and '$(WasmNativeDebugSymbols)' == ''">true</WasmNativeDebugSymbols>
+      <WasmDebugLevel Condition="'$(DebuggerSupport)' == 'true' and '$(WasmDebugLevel)' == ''">-1</WasmDebugLevel>
     </PropertyGroup>
 
     <ItemGroup>
@@ -79,4 +85,17 @@
       <WasmFilesToIncludeInFileSystem Include="@(WasmFilesToIncludeFromPublishDir -> '$(PublishDir)%(Identity)')" />
     </ItemGroup>
   </Target>
+
+  <!-- linker automatically picks up the .pdb files, but they are not added to the publish list.
+       Add them explicitly here, so they can be used with WasmAppBuilder -->
+  <Target Name="AddPdbFilesToPublishList" AfterTargets="ILLink" Condition="'$(DebuggerSupport)' == 'true'">
+    <ItemGroup>
+      <_PdbFilesToCheck Include="$([System.IO.Path]::ChangeExtension('%(ResolvedFileToPublish.Identity)', '.pdb'))"
+                        Condition="'%(ResolvedFileToPublish.Extension)' == '.dll'" />
+
+      <ResolvedFileToPublish Include="@(_PdbFilesToCheck)"
+                             Condition="Exists(%(_PdbFilesToCheck.Identity))"
+                             RelativePath="%(_PdbFilesToCheck.FileName)%(_PdbFilesToCheck.Extension)" />
+    </ItemGroup>
+  </Target>
 </Project>
diff --git a/src/mono/wasm/build/WasmApp.targets b/src/mono/wasm/build/WasmApp.targets
index 7711fdbe273733..0ab461d05d8377 100644
--- a/src/mono/wasm/build/WasmApp.targets
+++ b/src/mono/wasm/build/WasmApp.targets
@@ -20,7 +20,12 @@
       - $(WasmNativeStrip)   - Whenever to strip the native executable. Defaults to true.
       - $(WasmLinkIcalls)    - Whenever to link out unused icalls. Defaults to $(WasmBuildNative).
       - $(RunAOTCompilation) - Defaults to false.
+
       - $(WasmDebugLevel)
+                              > 0 enables debugging and sets the debug log level to debug_level
+                              == 0 disables debugging and enables interpreter optimizations
+                              < 0 enabled debugging and disables debug logging.
+
       - $(WasmNativeDebugSymbols) - Build with native debug symbols, useful only with `$(RunAOTCompilation)`, or `$(WasmBuildNative)`
                                     Defaults to true.
       - $(WasmDedup)         - Whenever to dedup generic instances when using AOT. Defaults to true.

From 5b68bc93b15e6632926a20a93524e18a2c75de6a Mon Sep 17 00:00:00 2001
From: Ankit Jain <radical@gmail.com>
Date: Fri, 9 Apr 2021 12:38:03 -0400
Subject: [PATCH 2/2] [wasm] WasmAppBuilder: Copy pdb files whenever debugging

.. is enabled, which is for WasmDebugLevel != 0
---
 src/tasks/WasmAppBuilder/WasmAppBuilder.cs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tasks/WasmAppBuilder/WasmAppBuilder.cs b/src/tasks/WasmAppBuilder/WasmAppBuilder.cs
index de7c0e3741b74b..d1cf986b3cccfb 100644
--- a/src/tasks/WasmAppBuilder/WasmAppBuilder.cs
+++ b/src/tasks/WasmAppBuilder/WasmAppBuilder.cs
@@ -150,7 +150,7 @@ public override bool Execute ()
         foreach (var assembly in _assemblies)
         {
             FileCopyChecked(assembly, Path.Join(asmRootPath, Path.GetFileName(assembly)), "Assemblies");
-            if (DebugLevel > 0)
+            if (DebugLevel != 0)
             {
                 var pdb = assembly;
                 pdb = Path.ChangeExtension(pdb, ".pdb");
@@ -173,7 +173,7 @@ public override bool Execute ()
         foreach (var assembly in _assemblies)
         {
             config.Assets.Add(new AssemblyEntry(Path.GetFileName(assembly)));
-            if (DebugLevel > 0) {
+            if (DebugLevel != 0) {
                 var pdb = assembly;
                 pdb = Path.ChangeExtension(pdb, ".pdb");
                 if (File.Exists(pdb))