-
Notifications
You must be signed in to change notification settings - Fork 275
[MTP] Error when VSTest arguments are passed via dotnet test when they will be ignored #5698
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>
dotnet test
when they will be ignored
...rosoft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.props
Outdated
Show resolved
Hide resolved
src/Platform/Microsoft.Testing.Platform.MSBuild/Resources/MSBuildResources.resx
Outdated
Show resolved
Hide resolved
...soft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.targets
Outdated
Show resolved
Hide resolved
...IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MSBuildTests.Test.cs
Outdated
Show resolved
Hide resolved
...IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MSBuildTests.Test.cs
Outdated
Show resolved
Hide resolved
… feedback Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>
Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
src/Platform/Microsoft.Testing.Platform.MSBuild/Resources/MSBuildResources.resx
Outdated
Show resolved
Hide resolved
...IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MSBuildTests.Test.cs
Outdated
Show resolved
Hide resolved
...IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MSBuildTests.Test.cs
Outdated
Show resolved
Hide resolved
...IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MSBuildTests.Test.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>
Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5698 +/- ##
==========================================
+ Coverage 73.89% 73.97% +0.07%
==========================================
Files 602 602
Lines 36850 36849 -1
==========================================
+ Hits 27231 27259 +28
+ Misses 9619 9590 -29
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
...soft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.targets
Outdated
Show resolved
Hide resolved
Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>
Drafting, this will break |
You're absolutely right - the validation will break existing DotNetCoreCLI@2 tasks that pass VSTest properties. Should I modify the implementation to be less aggressive (e.g., warning instead of error, or different default behavior) or would you prefer to discuss the approach first before I make further changes? |
This PR implements validation to show errors when VSTest-specific command-line switches are used with Microsoft Testing Platform (MTP), helping users understand why their VSTest properties are being ignored.
Problem
Users were confused when using VSTest-specific properties with
dotnet test
while using MTP, because these properties get silently ignored. For example:dotnet test --filter something
- filter is ignoreddotnet test --logger trx
- logger is ignoredThis led to users wondering why their test filtering or logging wasn't working as expected.
Solution
Added comprehensive validation that produces an error when VSTest-specific properties are set with MTP:
Key Changes
New opt-out property:
TestingPlatformIgnoreVSTestProperties
(default:false
) allows users to suppress the error when needed.Validation target: Added
_ValidateVSTestProperties
target that checks for all VSTest properties mentioned in the issue:VSTestSetting
,VSTestListTests
,VSTestTestCaseFilter
,VSTestTestAdapterPath
VSTestLogger
,VSTestDiag
,VSTestResultsDirectory
,VSTestCollect
VSTestBlame
,VSTestBlameCrash
,VSTestBlameHang
Clear error message: Provides helpful guidance including:
Integration with existing workflow: The validation runs as part of the
InvokeTestingPlatform
target, ensuring it catches issues early in the build process.Example Error Message
Usage Examples
Error case:
Opt-out:
Project file opt-out:
Testing
Added comprehensive integration tests covering:
Breaking Change Notice
This is a breaking change as it will now show errors for previously "working" (but ignored) VSTest property usage. However, this is intentional to help users identify and fix configuration issues. Users can opt-out during migration using the
TestingPlatformIgnoreVSTestProperties
property.Fixes #5697.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
8xbvsblobprodcus382.vsblob.vsassets.io
dotnet build src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj --configuration Release --verbosity minimal
(dns block)bcnvsblobprodcus378.vsblob.vsassets.io
dotnet build src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj --configuration Release --verbosity minimal
(dns block)http://168.63.129.16:80/machine/
/usr/bin/python3 -u bin/WALinuxAgent-2.13.1.1-py3.9.egg -collect-logs
(http block)i1qvsblobprodcus353.vsblob.vsassets.io
dotnet build src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj --configuration Release --verbosity minimal
(dns block)l49vsblobprodcus358.vsblob.vsassets.io
dotnet build src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj --configuration Release --verbosity minimal
(dns block)mfjvsblobprodcus373.vsblob.vsassets.io
dotnet build src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj --configuration Release --verbosity minimal
(dns block)s4uvsblobprodcus326.vsblob.vsassets.io
dotnet build src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj --configuration Release --verbosity minimal
(dns block)s8mvsblobprodcus38.vsblob.vsassets.io
dotnet build src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj --configuration Release --verbosity minimal
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.