Migrate from VSTest to Microsoft.Testing.Platform #8498
Conversation
github-actions
bot
added
the
needs-area-label
Youssef1313
force-pushed
the
dev/ygerges/migrate-to-mtp
branch
17 times, most recently
from
35c30c1 to
c53039b
Compare
Youssef1313
force-pushed
the
dev/ygerges/migrate-to-mtp
branch
from
c53039b to
e427f13
Compare
danmoseley
added
area-engineering-systems
| </Loggers> | ||
| </LoggerRunSettings> | ||
| </RunSettings> | ||
| --show-live-output on |
There was a problem hiding this comment.
if we wanted to experiment with parallel tests runs for a particular library, we'd put it in a file like this presumably? (right now none may be stable for parallel runs, nor diagnosable, but for traditional unit tests it may be possible)
There was a problem hiding this comment.
Either that, or add it to TestingPlatformCommandLineArguments.
Note: the current file is no longer really runsettings, it's a response file. RunSettings is actually not supported by core MTP (there is limited support for it through VSTestBridge, but xUnit doesn't use the bridge).
There was a problem hiding this comment.
You can also do dotnet run --project path/to/test-project.csproj -- --help and it will show you all the command-line options available.
There was a problem hiding this comment.
why do we still have .runsettings if it's not really supported?
There was a problem hiding this comment.
The file name is just .runsettings but it acts as a command-line response file. I can rename for clarity.
The original intent is very unclear to me, there are too many .runsettings scattered and it's not claer which is used for what and when.
If you can clarify the original intent, maybe there can be a good room to clean it up more.
There was a problem hiding this comment.
I cannot clarify. Possibly @radical can or perhaps it's just happenstance, this isn't a mature repo. Any cleanup welcome!
There was a problem hiding this comment.
Okay, I went ahead and cleaned up all the .runsettings.
Any customizations that are needed for a specific project can be done via TestingPlatformCommandLineArguments MSBuild property. This allows more flexibility as you can customize in Directory.Build.[props|targets] as well, based on any conditions. Previously, only one .runsettings file can be passed, and it was too hard to track which runsettings is used where.
There was a problem hiding this comment.
We have various runsettings to have different logging and timeout setup, and the files themselves weren't compostable. This was useful for local vs CI runs. I did start moving to overriding on the command line. But cleaning all this up to use command line parameters via msbuild properties will be great!!
tests/Aspire.Components.Common.Tests/Aspire.Components.Common.Tests.csproj
Show resolved
Hide resolved
1. Take build agent OS in account when running tests Addresses #8498 (comment) 2. Read trx from correct location Resolves #8683
|
@RussKie Why is the placement of this mattering? Are you parsing the log file programmatically somewhere? The discrepancy mostly comes from Arcade. With VSTest, the command line comes before the test execution: For xunit.v2 runner, it comes after: For xunit.v3:
|
|
Some logs can be very log (hundreds and thousands of lines long), and having the command line at the end hinders the process. |
|
I didn't realise the VSTest and XUnit had different implementations 😲 |
| <!-- https://learn.microsoft.com/dotnet/core/testing/microsoft-testing-platform-exit-codes --> | ||
| <!-- Exit code 8 is "zero tests ran" --> | ||
| <!-- Currently, none of the tests in this project run in CI. All are ignored --> | ||
| <TestingPlatformCommandLineArguments>$(TestingPlatformCommandLineArguments) --ignore-exit-code 8</TestingPlatformCommandLineArguments> |
There was a problem hiding this comment.
@Youssef1313 this doesn't appear to be used in Microsoft.Testing.Platform.targets, which results in the tests to fail
Should this be tucked on to _TestRunnerArgs?
aspire/eng/Xunit3/Microsoft.Testing.Platform.targets
Lines 23 to 31 in fd413eb
There was a problem hiding this comment.
@RussKie TestingPlatformCommandLineArguments is currently used when invoking dotnet test but not when invoked via Arcade.
There was a problem hiding this comment.
What would be the right way to fix this?
There was a problem hiding this comment.
I think you can simply append TestingPlatformCommandLineArguments to _TestRunnerArgs.
There was a problem hiding this comment.
Hmm, what I'm suggesting won't probably be possible without my PR on Arcade side. For now I think you could duplicate the --ignore-exit-code 8 on both sides.
There was a problem hiding this comment.
Or simply just rename Aspire.Components.Common.Tests to end with TestUtilities if it's really not intended to be a test project, then you don't need the --ignore-exit-code 8 at all.
There was a problem hiding this comment.
Or simply just rename Aspire.Components.Common.Tests to end with TestUtilities if it's really not intended to be a test project, then you don't need the --ignore-exit-code 8 at all.
Yep, working on it
There was a problem hiding this comment.
I don't think we can do it easily (if at all).
The project uses XUnit API (like fact and theory), and we need to import the Xunit assemblies. Those are defined in eng/Xunit3/Xunit3.targets and can only be imported by test projects. If the targets imported explicitly, then the build gets failed by _XunitValidateBuild declared in xunit.v3.core.targets
D:\Development\dotnet-aspire\.packages\xunit.v3.core\2.0.0\buildTransitive\xunit.v3.core.targets(15,5): xUnit.net v3 test projects must be executable (set project property '<OutputType>Exe</OutputType>'). If this is not a test project, reference xunit.v3.extensibilty.core instead. [D:\Development\dotnet-aspire\tests\Aspire.Components.Common.Tests\Aspire.Components.Common.TestUtilities.csproj]
There was a problem hiding this comment.
@RussKie You need to reference only xunit.v3.extensibility.core in that project, and not xunit.v3.core, nor xunit.v3
1. Take build agent OS in account when running tests Addresses #8498 (comment) 2. Read trx from correct location Resolves #8683
1. Take build agent OS in account when running tests Addresses #8498 (comment) 2. Read trx from correct location Resolves #8683
1. Take build agent OS in account when running tests Addresses #8498 (comment) 2. Read trx from correct location Resolves #8683
|
Have we validated that these changes work with the Test Explorer in VS Code? I've ran into a few issues debugging in the repo (see microsoft/vscode-dotnettools#1923 and microsoft/vscode-dotnettools#1922) since this change went in. Test discovery and execution appear to behave much more reliably in VS Code if I revert the commit associated with this PR. Does anyone have any insights on the state of Microsoft.Testing.Platform support on VS Code? |
|
@captainsafia It's expected to work in VS Code if you are using DevKit |
|
@captainsafia I don't know if the option is enabled by default or not, but you need to make sure this is turned on:
(we did a change to no longer mark it "experimental" but not sure if the change has already shipped or not) |
|
@Youssef1313 Yep, I'm on a release of the extension without the experimental flag and have the feature enabled. I'm running into issues with debugging tests and lack of reliability with the test discovery. 
Is there an SDK requirement associated with this change? |
|
There shouldn't be SDK requirement associated. From your logs:
It seems like VS Code is treating the project as VSTest and not MTP. @drognanar would be the right contact. |
|
Looks like this could have broken me as well (I'm trying to use Codespaces as my primary environment). |
|
@captainsafia @mitchdenny As a potential temporary workaround, could you try adding a PackageReference to This would go in:
and version would be defined in: Line 2 in 3fc603b |
|
Happy to give it a shot. Can you explain why this might fix it? |
|
|
|
@Youssef1313 I tried this and unfortunately I still run into issues debugging tests in the repo. :/ |
|
@Youssef1313 can you repro this with VS Code? If we can't fix this we'll presumably have to revert your change until we figure it out. |
|
@danmoseley I think we should revert this. microsoft/vscode-dotnettools#1922 has been validated as a bug on the VS Code side and is under investigation. |
This reverts commit ac098f7.
|
@captainsafia I'm not able to reproduce. Added a comment in #8802 (comment) |
|
@captainsafia Meanwhile, while we are trying to figure it out, can you try again the xunit.runner.visualstudio workaround, but also add |
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #8293
Checklist
<remarks />and<code />elements on your triple slash comments?breaking-changetemplate):doc-ideatemplate):