-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Process] Return built-in cmd.exe commands directly in ExecutableFinder #58735
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
Conversation
Seldaek
commented
Nov 2, 2024
Q | A |
---|---|
Branch? | 5.4 |
Bug fix? | yes |
New feature? | no |
Deprecations? | no |
Issues | Refs #58734 |
License | MIT |
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.
(with minor CS change)
Thank you @Seldaek. |
} | ||
|
||
$finder = new ExecutableFinder(); | ||
$this->assertSame('rmdir', $finder->find('RMDIR')); |
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.
AppVeyor returns RMDIR
but not rmdir
(see the failing test). Do we need to fix the expectation here or is there still some flaw in the code?
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 command names are case insensitive so I guess the implementation is fine but test expectations should be fixed to match the input's case.
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.
👍 thanks for the feedback (see #58748 then)