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.

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

Tests: add trace listener to improve Debug.Assert experience #18785

Closed
alefranz opened this issue Feb 4, 2020 · 4 comments
Closed

Tests: add trace listener to improve Debug.Assert experience #18785

alefranz opened this issue Feb 4, 2020 · 4 comments
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Comments

@alefranz
Copy link
Contributor

alefranz commented Feb 4, 2020

Debug.Assert() cause the test host process to crash when tests target .NET Core.

This could be quite misleading as it is not usually clear why the tests are not running and could sometimes be not obvious that the rest is not running (the VS UI has recently improved on making it more obvious, so it is less of a problem).

As this project makes heavy use of Debug.Assert(), I think it would be nice to improve the experience for contributor.

Describe the solution you'd like

I've raised an issue on microsoft/vstest#2309 to discuss a global solution for the test runner as it would be nice for anyone to get this better behavior out of the box.

@nohwnd has provided in microsoft/vstest#2309 (comment) a possible implementation of a TraceListener to handle this.

Would you be open to include this in the ASP.NET Core tests?

@mkArtakMSFT mkArtakMSFT added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-hosting and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-hosting labels Feb 4, 2020
@Tratcher Tratcher added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework and removed area-servers labels Feb 4, 2020
@nohwnd
Copy link
Member

nohwnd commented Feb 24, 2020

Just FYI the provided implementation will also throw on any Debug message, there is currently a PR under review for this change that applies to both Debug and Trace, and works as expected. microsoft/vstest#2335 The change will hopefully be merged soon so you will get it in the next release / preview.

@nohwnd
Copy link
Member

nohwnd commented Feb 27, 2020

It's on nuget in pre-release channel now https://www.nuget.org/packages/Microsoft.NET.Test.Sdk/16.6.0-preview-20200226-03

@dougbu
Copy link
Member

dougbu commented Nov 16, 2020

We do not ship anything built using the Debug configuration. That should mean all Debug.Assert(...)s are disabled. What scenario would this help with❔

About the only value I can think of would come in conjunction with #11632 but we haven't decided to do anything about that. Or, are you talking about problems that crop up when debugging tests❔

@wtgodbe
Copy link
Member

wtgodbe commented Dec 3, 2020

Closing this for lack of traction, @alefranz please re-open if you'd like to revive the discussion

@wtgodbe wtgodbe closed this as completed Dec 3, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

No branches or pull requests

6 participants