Fix VB tests on MacOSX #78024
Draft
Fix VB tests on MacOSX #78024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
Area-Compilers
untriaged
Looking at the tests they are compiling against .NET Framework but then executing on core. That seems like the most likely cause of the problem here. closes dotnet#77861
arunchndr
added a commit
that referenced
this pull request
Apr 18, 2025
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: - Changes the responsibility of `IRuntimeEnvironment` to be _only_ about how code is executed. No more emitting code, calling into `Assert`, etc ... This is just about running code in a specific environment. - Moves the responsibility of verifying the output is as expected to `CompilationVerifier` - Moves the responsibility of emitting the code to be `CompilationVerifier` - Enables NRT in several places to better track how `null` is handled. There is so much implicit `null` usage 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.
jaredpar
commented
Oct 8, 2025
|
|
||
| internal override string VisualizeRealIL( | ||
| IModuleSymbol peModule, CompilationTestData.MethodData methodData, IReadOnlyDictionary<int, string> markers, bool areLocalsZeroed) | ||
| IModuleSymbol peModule, CompilationTestData.MethodData methodData, IReadOnlyDictionary<int, string>? markers, bool areLocalsZeroed) |
jaredpar
commented
Oct 8, 2025
| empty = true; | ||
| T result = item; | ||
| Console.Error.WriteLine(""{0} read {1}"", Thread.CurrentThread.Name, result); | ||
| Console.WriteLine(""{0} read {1}"", Thread.CurrentThread.Name, result); |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes VB tests on macOS by aligning the test execution model with the correct target framework. The tests were compiling against .NET Framework but then executing on Core, causing failures on macOS.
Key changes:
- Refactored test compilation and execution infrastructure to use .NET Core consistently
- Updated target framework selection in VB test files to use
TargetFramework.StandardAndVBRuntime - Simplified execution validation and output capture mechanisms
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Compilers/VisualBasic/Test/Semantic/Semantics/BinaryOperators.vb | Updated target framework usage and execution validation for VB tests |
| src/Compilers/VisualBasic/Test/Semantic/Semantics/PrintResultTestSource.vb | Added space normalization function for cross-platform compatibility |
| src/Compilers/Test/Utilities/VisualBasic/BasicTestBase.vb | Added baseline validation methods and execution validators |
| src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs | Updated nullable annotations and execution validation APIs |
| src/Compilers/Test/Core/CompilationVerifier.cs | Major refactoring of emit and execution infrastructure |
| src/Compilers/Test/Core/ExecutionValidators.cs | New utilities for validating test execution output |
| src/Test/PdbUtilities/EditAndContinue/EditAndContinueTest.cs | Fixed emit method call signature |
| src/Scripting/CoreTestUtilities/ScriptingTestHelpers.cs | Updated output capture to use tuples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| Assert.Equal(expectedOutput, output); | ||
| Assert.Equal(expectedErrorOutput, expectedErrorOutput); |
There was a problem hiding this comment.
This assertion is comparing expectedErrorOutput with itself instead of comparing it with errorOutput. Should be Assert.Equal(expectedErrorOutput, errorOutput).
Suggested change
| Assert.Equal(expectedErrorOutput, expectedErrorOutput); | |
| Assert.Equal(expectedErrorOutput, errorOutput); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Looking at the tests they are compiling against .NET Framework but then executing on core. That seems like the most likely cause of the problem here.
closes #77861