Update Microsoft.CodeAnalysis.Workspaces.UnitTests to target netcoreapp3.1 #45813
Conversation
|
Is there any CI configuration needs to be done to include those tests in CoreClr legs? |
|
It seems to already be running them there. |
| { | ||
| Debug.Assert(false, output); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
@tmat This is causing a problem because we expect the ThrowingTraceListener to be active, but it's not taking effect in .NET Core scenarios so we failfast instead.
There was a problem hiding this comment.
This doesn't seem like the right solution. We have Debug.Assert in many places.
There was a problem hiding this comment.
Any suggestions for how to proceed here?
There was a problem hiding this comment.
Do we know why it doesn't work on Core? For now, perhaps we can skip fail-fast on Core.
There was a problem hiding this comment.
Do we know why it doesn't work on Core?
Debug.Assert doesn't use trace listeners on .NET Core.
For now, perhaps we can skip fail-fast on Core.
I have not found a way to change this behavior without private reflection, and even if we go that route I'm not sure how to do it from a single location in the xunit startup process. Perhaps a custom MemberData property so it runs during discovery?
There was a problem hiding this comment.
I discussed this with @stephentoub , and it looks like we can still use ThrowingTraceListener, but we have to add it explicitly:
Trace.Listeners.Clear();
Trace.Listeners.Add(new ThrowingTraceListener());
There was a problem hiding this comment.
K. That pattern + module initailizers on .NET Core would seem to essentially cover this issue for us then.
There was a problem hiding this comment.
What release is the first to support module initializers? Are we already able to add one for the test helper assembly?
There was a problem hiding this comment.
The feature has been available in the runtime since 1.0 I believe. The language support will come in C# 9.0. The feature branch should get merged into the main branch sometime next week.
There was a problem hiding this comment.
This has been updated to use a module initializer to install the trace listener. There are two resulting impacts:
- Visual Basic test projects targeting .NET Core cannot use
Debug.Assertin any location where assertion might fail. Doing so will cause aFailFast. Workaround: if/when this occurs, make the test class extendTestBaseto force module initialization via inheritance. - This PR is now blocked on using the
previewlanguage version (Enable usage of preview language features #44485).
|
@sharwell Any update? |
|
Ping @sharwell |
sharwell
force-pushed
the
netcoreapp-workspaces-tests
branch
from
d77b7d3
to
5a20683
Compare
|
@sharwell Looks like the tests are running and pass on .Net Core legs, is this good to go now? |
ghost
added this to the 
No description provided.