Improve debugging experience #3464

sharwell · 2020-01-31T18:17:43Z

Do not optimize test framework (stepping in debugger will have full information about parameters, locals, and breakpoints)
Embed symbols in assemblies (eliminates the need to copy PDB files)
Embed sources in assemblies (eliminates the need to download sources separately)

jnm2 · 2020-02-02T19:17:31Z

Do not optimize test framework (stepping in debugger will have full information about parameters, locals, and breakpoints)

I'm concerned about this. Can you give us some idea of what performance we are giving up as well as some idea of how much time this would save you and how often this would happen? Stepping through release builds of libraries is a well-understood scenario. I'm not sure what makes NUnit a special case.

Embed symbols in assemblies (eliminates the need to copy PDB files)

I'm a fan. Isn't .snupkg the current recommendation and also working out of the box these days for anyone with VS2019+, with a very simple fallback (add symbol download address) for folks on older VS?

Embed sources in assemblies (eliminates the need to download sources separately)

I'm a fan. I do this absolutely everywhere I'm allowed.

rprouse · 2020-02-28T01:15:23Z

@sharwell because of our history, I trust you and this looks good, but I would appreciate it if you could provide a bit more explanation or point us to the documentation for the build properties you introduced. I like the change, but I'd like to read up a bit more on it if possible.

sharwell · 2020-02-28T01:54:43Z

build.cake

-Task("GitLink")
-    .IsDependentOn("CreateImage")
-    .Description("Source-indexes PDBs in the images directory to the current commit")
-    .Does(() =>
-    {
-        var settings = new GitLink3Settings
-        {
-            BaseDir = PROJECT_DIR
-        };
-        GitLink3(GetFiles($"{CurrentImageDir}**/*.pdb"), settings);
-    });


📝 GitLink (and SourceLink) are used to help the debugger locate the original sources used for building an assembly, allowing for a seamless debugging experience. This pull request removes this functionality because the original sources are now embedded inside the DLL itself. It provides the best possible debugging experience (zero latency, zero configuration required) at the expense of a larger assembly (the DLL now contains the PDB and all the source files). Note that the embedded PDB uses the portable PDB format so even with the sources it's likely smaller than the "full" PDB format used in the past.

When I did the numbers, the portable PDB+embedded source was 3.75× the size and Windows PDB+GitLink was 2.51× the size. I don't think size should be the determining factor though.

Embedded source requires VS 2017 Update 5+ for the debugger to locate the source. Do you know what version of VS is required to locate the source when the PDB is also embedded in the assembly?

I'm thinking that's far enough behind us now that it isn't a concern. Guessing that latest 2017 and 2019 both work.

sharwell · 2020-02-28T01:58:35Z

src/NUnitFramework/Directory.Build.props

+    <Optimize>false</Optimize>
+    <DebugType>embedded</DebugType>
+    <DebugSymbols>true</DebugSymbols>
+    <EmbedAllSources>true</EmbedAllSources>


Two of these properties (Optimize=false and DebugSymbols=true) are the default settings you always see when building with /p:Configuration=Debug.

DebugType=portable is the default for .NET Standard and .NET Core (and perhaps all SDK projects). DebugType=embedded uses the same PDB format as portable, but instead of creating a standalone PDB it embeds the PDB itself inside the DLL.

EmbedAllSources=true embeds all source files inside the PDB, which allows the debugger to locate source files without needing to look for them elsewhere. The source files end up embedded inside the DLL as a side effect of the PDB being embedded inside the DLL.

At this point, the debugging experience for end users will be very close to the experience NUnit developers have when debugging with a Debug build configuration. There are two differences:

I doubt Edit and Continue will work.

Release builds still do not define the DEBUG preprocessor symbol, so things like Debug.Assert will still not be included in the compiled code.

sharwell · 2020-02-28T02:04:12Z

I'm concerned about this. Can you give us some idea of what performance we are giving up as well as some idea of how much time this would save you and how often this would happen?

Since NUnit doesn't have heavy computation loops, I'm not expecting a significant observable change. The most likely observations will be increases in allocations associated with methods marked async in the NUnit code base, though these should not be significant compared to the cost of the tests themselves.

The best test case would likely be a project with a very large number of very small tests.

sharwell · 2020-02-28T02:06:13Z

I'm going to rebase this change to address the merge conflict.

jnm2 · 2020-02-28T21:51:37Z

@sharwell I'm still curious what makes NUnit special, or is this coming from a blanket recommendation that all .NET libraries should consider switching to shipping debug builds? (And what is the source of the recommendation?)

sharwell · 2020-02-28T22:28:42Z

More the idea that unit testing frameworks are part of the developer's inner loop more than part of production code. The case where I found myself wishing for better debugging was trying to step through ThrowsAsync to see how it interacted with synchronization contexts.

jnm2 · 2020-02-28T23:28:14Z

That helps, thanks! This type of change would thus not be applicable to the NUnit console, engine, or VSTest adapters then, I'm thinking?

rprouse

I'm good with this but it is redoing the work you did @jnm2. Can you give it a second review?

jnm2 · 2020-03-02T22:16:51Z

https://github.com/nunit/nunit/blob/master/README.md#debugger-source-stepping also needs to be updated or deleted.

clairernovotny · 2020-03-26T14:33:47Z

Just came across this and it looks like GitLink was generating invalid Source Link data:

It's embedding its data as lower-case instead of matching what's in the PDB so we can't correctly substitute. Moot point now but something we should be aware of.

mikkelbu · 2021-01-22T19:56:18Z

@sharwell, @jnm2 After this change is seems that the debugger will always enter NUnit code when people are debugging their tests (no matter if "Enable Just My Code" is checked or not). It has reported in https://groups.google.com/g/nunit-discuss/c/f2yV4G_kdi8/m/ypWZfLSEDgAJ and I can also reproduce this. Do you know if it is possible to configure VS such that it does not enter NUnit code.

jnm2 · 2021-01-22T20:02:19Z

I see, even with Just My Code turned off. I'm in favor of moving to the snupkg format then, since that uses the well-known nuget.org symbol server that has been built into VS for a while.

jnm2 · 2021-01-22T20:34:12Z

https://docs.microsoft.com/en-us/dotnet/standard/library-guidance/nuget#symbol-packages
https://docs.microsoft.com/en-us/nuget/create-packages/symbol-packages-snupkg
https://devblogs.microsoft.com/nuget/improved-package-debugging-experience-with-the-nuget-org-symbol-server/

sharwell · 2021-01-26T15:48:32Z

@mikkelbu I would suggest filing an issue through Developer Community. I can't wrap my head around the idea that someone could have Just My Code enabled and not think it's a bug, so the response that makes the most sense to me is "obviously this behavior is correct, so I'm not sure what the question is 😕 ". But based on what I know of Just My Code, the behavior would be considered a bug in that scenario.

jnm2 · 2021-01-26T16:01:49Z

@sharwell Just My Code is a lifesaver when you are working with libraries that handle exceptions or when you don't want extra waits every time you debug where VS loads symbols (even when using a whitelist).

mikkelbu · 2021-01-26T21:07:01Z

I've created the following issue in Developer Community - https://developercommunity2.visualstudio.com/t/The-debugger-enters-code-in-PackageRefer/1321266

gfoidl · 2021-01-28T21:33:55Z

From .NET Just My Code:

In .NET projects, Just My Code uses symbol (.pdb) files and program optimizations to classify user and non-user code. The .NET debugger considers optimized binaries and non-loaded .pdb files to be non-user code.

So one can argue that the .NET Debugger is correct by thinking NUnit* is user-code, as pdb is there and program optimizations are off.

But I agree that this is counterintuitive and "Just my code" should be tightened to only consider "my" code.

* Moq, etc. which also embeds the pdb have the same "issue".

- Build portable PDBs - Create snupkg files for uploading symbols - Reinstate gitlink

sharwell force-pushed the easy-debug branch 3 times, most recently from 808e0a1 to bb1abae Compare January 31, 2020 18:34

jnm2 requested a review from rprouse February 2, 2020 19:07

sharwell commented Feb 28, 2020

View reviewed changes

sharwell marked this pull request as ready for review February 28, 2020 02:04

Improve debugging experience

6d3482d

sharwell force-pushed the easy-debug branch from bb1abae to 6d3482d Compare February 28, 2020 02:07

rprouse approved these changes Feb 29, 2020

View reviewed changes

rprouse requested a review from jnm2 February 29, 2020 20:14

jnm2 approved these changes Mar 1, 2020

View reviewed changes

rprouse merged commit 22ea163 into nunit:master Mar 3, 2020

rprouse added this to the 3.13 milestone Mar 3, 2020

sharwell deleted the easy-debug branch March 3, 2020 01:21

jnm2 mentioned this pull request Mar 3, 2020

No need to mention the wiki instructions since debugging lights up whenever Just My Code is off #3490

Merged

jnm2 mentioned this pull request Jun 29, 2020

[PR] The proj files have been updated to enable SourceLink #3586

Closed

mikkelbu mentioned this pull request Jan 26, 2021

After upgrading to NUnit 3.13.0 the debugger enters NUnit code despite having checked "Enable Just My Code" #3745

Closed

rprouse mentioned this pull request Jan 30, 2021

Create NuGet source packages, snupkg #3748

Merged

rprouse added a commit that referenced this pull request Jan 30, 2021

- Partially reverts #3464

35aefae

- Build portable PDBs - Create snupkg files for uploading symbols - Reinstate gitlink

This was referenced Mar 6, 2021

Bump nunit from 3.13.0 to 3.13.1 markwhitaker/MimeTypes.NET#8

Merged

Bump NUnit from 3.12.0 to 3.13.1 in /tests/PVOutput.Net.Tests pyrocumulus/pvoutput.net#55

Merged

Bump NUnit from 3.12.0 to 3.13.1 Brightspace/D2L.Security.OAuth2#146

Merged

This was referenced Mar 14, 2021

build(deps): bump NUnit from 3.12.0 to 3.13.1 in /services skyra-project/skyra#1643

Merged

Bump nunit from 3.12.0 to 3.13.1 NeilMacMullen/jumpfs#1

Closed

mikkelbu mentioned this pull request Jun 18, 2021

nunit.framework.dll is built in debug mode #3878

Closed

lahma mentioned this pull request Jul 30, 2022

Reduce allocations caused by string.Split #4035

Merged

stevenaw mentioned this pull request Aug 4, 2022

Distribute optimized framework builds with easy debugging #4149

Closed

Improve debugging experience #3464

Improve debugging experience #3464

Uh oh!

Conversation

sharwell commented Jan 31, 2020

Uh oh!

jnm2 commented Feb 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rprouse commented Feb 28, 2020

Uh oh!

sharwell Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

jnm2 Mar 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharwell Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

sharwell commented Feb 28, 2020

Uh oh!

sharwell commented Feb 28, 2020

Uh oh!

jnm2 commented Feb 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sharwell commented Feb 28, 2020

Uh oh!

jnm2 commented Feb 28, 2020

Uh oh!

rprouse left a comment

Choose a reason for hiding this comment

Uh oh!

jnm2 commented Mar 2, 2020

Uh oh!

clairernovotny commented Mar 26, 2020

Uh oh!

mikkelbu commented Jan 22, 2021

Uh oh!

jnm2 commented Jan 22, 2021

Uh oh!

jnm2 commented Jan 22, 2021

Uh oh!

sharwell commented Jan 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnm2 commented Jan 26, 2021

Uh oh!

mikkelbu commented Jan 26, 2021

Uh oh!

gfoidl commented Jan 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jnm2 commented Feb 2, 2020 •

edited

Loading

jnm2 Mar 1, 2020 •

edited

Loading

jnm2 commented Feb 28, 2020 •

edited

Loading

sharwell commented Jan 26, 2021 •

edited

Loading

gfoidl commented Jan 28, 2021 •

edited

Loading