Fix DNX command casting error with DnxCommandDefinition#52820
Fix DNX command casting error with DnxCommandDefinition#52820marcpopMSFT merged 4 commits intorelease/10.0.2xxfrom
Conversation
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a casting exception in the dnx command by changing ToolExecuteCommand to use the base class ToolExecuteCommandDefinitionBase instead of the concrete ToolExecuteCommandDefinition class.
Changes:
- Modified
ToolExecuteCommandto acceptToolExecuteCommandDefinitionBaseinstead ofToolExecuteCommandDefinition, allowing it to work with bothDnxCommandDefinitionandToolExecuteCommandDefinition
|
@copilot there should be tests for the tool exec command - these tests should be updated to be parameterized to make sure that |
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Added parameterized tests for both |
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
|
Tested the fix in a codespace and looks good to me. @tmat I think #52162 broke this. What are the likelihood that there are other breaks from this refactor? We map dnx to tool exec so do we have any other similar mappings we should double check work? I don't think so within the workload or tooling space. |
|
The only reasonable way is to check if we have code coverage. |
The
dnxcommand throws a casting exception when executing tools:Changes
ToolExecuteCommandwas parameterized withCommandBase<ToolExecuteCommandDefinition>, butDnxCommandParsercreates a parse result containingDnxCommandDefinition. Both types inherit fromToolExecuteCommandDefinitionBase, which defines all arguments/options the command uses.Changed
ToolExecuteCommandto use the base class:This allows the command to work with both
DnxCommandDefinition(dnx) andToolExecuteCommandDefinition(tool execute/exec).Tests Added
Added parameterized tests to ensure both
dnxandexeccommands work correctly:InstallAndRunToolFromAnyRidfrom[Fact]to[Theory]with test cases for both "exec" and "dnx" commandsInstallAndRunToolFromAnyRidWhenOtherRidsArePresentButIncompatiblefrom[Fact]to[Theory]with test cases for both "exec" and "dnx" commandsThese tests verify that both command variants function correctly with the same tool execution logic.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.