-
Notifications
You must be signed in to change notification settings - Fork 289
Fix hangdump not working under dotnet test with .NET 10 SDK #6968
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 <[email protected]>
dotnet test
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.
@copilot This needs to be an integration acceptance test, not unit test.
…angdump Co-authored-by: Youssef1313 <[email protected]>
dotnet testdotnet test
|
Confirmed test is failing: |
dotnet test| [TestMethod] | ||
| public async Task HangDump_WithDotnetTest_CreateDump() | ||
| { | ||
| if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) |
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.
Nit: we can use the oscondition attribute here instead
| _clock = clock; | ||
| if (_commandLineOptions.IsOptionSet(HangDumpCommandLineProvider.HangDumpOptionName) && | ||
| !_commandLineOptions.IsOptionSet(PlatformCommandLineProvider.ServerOptionKey)) | ||
| if (_commandLineOptions.IsOptionSet(HangDumpCommandLineProvider.HangDumpOptionName)) |
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 description mentions checking both server and pipe cli options but the 2 parts of the implementation don't do that. Is this expected?
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.
I don't think it's worth any special casing here at all. Even if this ended up being used in Test Explorer and it doesn't work, I don't think it's a big concern.
HangDump extension is currently broken under
dotnet testbecauseIsEnabledAsync()checks!IsOptionSet(ServerOptionKey), which returns false when--serveris set (as it is withdotnet test).Test Added
Added an integration acceptance test
HangDump_WithDotnetTest_CreateDumptoMicrosoft.Testing.Platform.Acceptance.IntegrationTests/HangDumpTests.csthat demonstrates the bug.The test:
dotnet testwith hangdump options passed after the--separatordotnet testdotnet test(demonstrating the bug)Test Command
Current Broken Logic
Expected Pattern
Should match
RetryOrchestratorpattern to supportdotnet test:This differentiates between generic server mode (disabled) and
dotnet testmode (enabled).Note: Per issue instructions, only a test was added to demonstrate the bug. Production code changes require separate PR.
Original prompt
<issue_description>I think this is currently broken and the hangdump extension is never enabled under
dotnet test.testfx/src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpActivityIndicator.cs
Lines 85 to 86 in dacc39f
</issue_description>
<agent_instructions>Do not change production code. Only attempt to add a test.</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@Youssef1313 Note: there is another use of `IsOptionSet(ServerOptionKey)` in the constructor. @Youssef1313 We have similar usage in `HangDumpEnvironmentVariableProvider` @Youssef1313 We should probably use `_commandLineOptions.IsOptionSet(PlatformCommandLineProvider.ServerOptionKey) && !_commandLineOptions.IsOptionSet(PlatformCommandLineProvider.DotNetTestPipeOptionKey)` instead. Or, alternatively, rely on the actual value of `--server` (for dotnet test, it's `dotnettestcli`), similar to what we do in TestHostBuilder:Original prompt
💡 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.