Cleanup how code is executed in our unit tests #78112
Conversation
ghost
added
Area-Compilers
untriaged
| item = newItem; | ||
| empty = false; | ||
| Console.Error.WriteLine(""{0} wrote {1}"", Thread.CurrentThread.Name, newItem); | ||
| Console.WriteLine(""{0} wrote {1}"", Thread.CurrentThread.Name, newItem); |
There was a problem hiding this comment.
The test verification now verifies output and error output. This test was using error output for a reason that I cannot determine, no other test does this.
| => this with { ILVerifyMessage = message }; | ||
| } | ||
|
|
||
| #nullable disable |
There was a problem hiding this comment.
This file is just 100% turning on NRT and chasing down the errors. There is so much implicit null usage in this layer and it was making it hard to understand what was / wasn't allowed in CompilationVerifier. Decided to take the time to get null clean here.
| } | ||
| } | ||
|
|
||
| internal static class RuntimeEnvironmentUtilities |
There was a problem hiding this comment.
The methods here were just moved to CompilationVerifier. They were all about emitting Compilation and references into ModuleData or dumping them to disk. That doesn't belong in the test execution harness hence moved it to CompilationVerifier which does have responsibility for emitting code.
| { | ||
| public sealed partial class CompilationVerifier | ||
| { | ||
| internal readonly struct EmitOutput |
There was a problem hiding this comment.
Type moved from IRuntimeEnvironment.cs
| return output.ToString(); | ||
| } | ||
|
|
||
| public static string DumpAssemblyData(IEnumerable<ModuleData> modules) |
There was a problem hiding this comment.
Moved from IRuntimeEnvironment.cs
| .ToList(); | ||
| AssertEx.SetEqual(expectedFields, members); | ||
| } | ||
|
|
There was a problem hiding this comment.
This block is moved from IRuntimeEnvironment.cs
| private readonly Compilation _compilation; | ||
| private CompilationTestData _testData; | ||
| private readonly IEnumerable<ModuleData> _dependencies; | ||
| private ImmutableArray<Diagnostic> _diagnostics; | ||
| private IModuleSymbol _lazyModuleSymbol; | ||
| private IList<ModuleData> _allModuleData; |
There was a problem hiding this comment.
The mutable fields here were only valid after consumers had called Emit. There was inconsistent validation in the type about checking for whether emit had occurred before accessing these values. The structure also made it very hard to enable NRT in this file. Moved all of those fields into a new type, EmitData and centralized the validation in GetEmitData.
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| #nullable disable |
There was a problem hiding this comment.
Suggest just looking at the after state of this file vs. the diff.
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| #nullable disable |
There was a problem hiding this comment.
Suggest looking at the after state of this file vs. the diff.
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| #nullable disable |
There was a problem hiding this comment.
This file is just turn on NRT and follow the diagnostics.
|
@dotnet/roslyn-compiler PTAL |
jaredpar
removed
the
untriaged
src/Compilers/Test/Core/Platform/CoreClr/CoreCLRRuntimeEnvironment.cs
Outdated
Show resolved
Hide resolved
src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenWinMdEvents.vb
Outdated
Show resolved
Hide resolved
Co-authored-by: Jan Jones <jan.jones.cz@gmail.com>
| Imports Microsoft.CodeAnalysis.VisualBasic | ||
| Imports Roslyn.Test.Utilities | ||
| Imports Basic.Reference.Assemblies | ||
| Imports Microsoft.CodeAnalysis.Test.Utilities |
There was a problem hiding this comment.
nit: consider sorting the usings
4 participants
As part of #78024 I needed to change how output was handled by our unit tests. In looking at the test execution code I noticed that it was in need of a bit of a cleanup. The responsibilities of the layers was not clear and this lead to a lot of code duplication as well as subtle behavior differences between .NET Core and Framework. This change does the following:
IRuntimeEnvironmentto be only about how code is executed. No more emitting code, calling intoAssert, etc ... This is just about running code in a specific environment.CompilationVerifierCompilationVerifiernullis handled. There is so much implicitnullusage in the low test layers that it was making the hard to follow. Decided to just turn on NRT there and work through the problems.