-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add --self-contained and --no-self-contained options to dotnet test #51662
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
…ommand Co-authored-by: marcpopMSFT <[email protected]>
Youssef1313
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the intent here to do that for MTP only? Or both MTP and VSTest?
I'm not sure if any of this will work well with VSTest. Looping @nohwnd in as well.
|
@Youssef1313 any test type that supports exe tests should have these. I don't know if there is a way to filter the CLI like that so probably it should error if you use one of those flags with a test type that doesn't support that app type. The argument from Chet was that anything that does a build and builds an app should have these flag for consistency. |
|
In that case it is MTP only and is known at CLI level |
|
I don't see this issue happening on net10.0 anymore, also noted by marc here: #27151 (comment) Red because I don't have x86 runtime so tests cannot run, but no warning before:
Same for MTP, no warning:
|
|
@copilot can you resolve the merge conflict |
|
@nohwnd for clarity, in triage we determined the other issue wasn't happening anymore but also Chet indicated that we should be consistent across our commands. Hence this PR (ie this is different than the intent of the original issue). Let me know if ya'll don't want these flags in dotnet test or there's a way to limit them to just MTP that I can tell copilot to do. |
- TestCommandParser was refactored in main branch with options moved to TestCommandDefinition - Added SelfContainedOption and NoSelfContainedOption to TestCommandDefinition - Added options to both ConfigureVSTestCommand and ConfigureTestingPlatformCommand - Updated all references from TestCommandParser to TestCommandDefinition throughout codebase - Fixed validation in TestCommand and MicrosoftTestingPlatformTestCommand to use TestCommandDefinition Co-authored-by: marcpopMSFT <[email protected]>
c6fa7f3 to
07ef2e5
Compare
Per review feedback: - Moved SelfContainedOption and NoSelfContainedOption to MicrosoftTestingPlatformOptions - Removed these options from ConfigureVSTestCommand (VSTest) - Removed validation from TestCommand.cs (VSTest) - Updated MicrosoftTestingPlatformTestCommand.cs to reference MicrosoftTestingPlatformOptions - Removed VSTest-specific tests for these options since they're now MTP-only Co-authored-by: marcpopMSFT <[email protected]>
|
@copilot resolve the conflicts |
| Arity = ArgumentArity.Zero | ||
| }; | ||
|
|
||
| public static readonly Option<bool> SelfContainedOption = CommonOptions.SelfContainedOption; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The common option definition has the bug where it's a bool but the true/false value isn't considered right? Do we want to replicate this bug again in test command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Searching GH, I see around 15k instances of --self-contained true or --self-contained false
If we remove this, it'll break people but if we leave it, all of the false people will continue to be wrong.
Feels like we should break people or actually honor the bool. @baronfel ?
|
@copilot try resolving the conflicts again |


Plan: Add --self-contained and --no-self-contained options to dotnet test command
Implementation Checklist:
SelfContainedOptionandNoSelfContainedOptiontoMicrosoftTestingPlatformOptions.cs(MTP only)ConfigureTestingPlatformCommandonly (not VSTest)MicrosoftTestingPlatformTestCommand.csonly (not VSTest)Changes Made (Per Review Feedback):
Based on reviewer feedback that these options should be MTP-only, the implementation was updated:
MicrosoftTestingPlatformOptions.cs:
SelfContainedOptionandNoSelfContainedOptionpropertiesTestCommandDefinition.cs:
SelfContainedOptionandNoSelfContainedOptionpropertiesConfigureVSTestCommand()ConfigureTestingPlatformCommand()to useMicrosoftTestingPlatformOptions.SelfContainedOptionTestCommand.cs (VSTest):
MicrosoftTestingPlatformTestCommand.cs:
MicrosoftTestingPlatformOptions.SelfContainedOptionTests:
Verification:
✅ Build succeeds in Debug configuration
✅ Options appear in
dotnet test --helponly when using MTP mode (via global.json)✅ Options do NOT appear in VSTest mode
dotnet testcommand #31066Original prompt
dotnet testcommand #31066✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.