fix: Don't escape regex characters in test commands on Windows

[BillionClaw]

Fixes #3759

Shellwords.escape() is designed for Unix shells and incorrectly escapes characters like $ on Windows. This causes the "Run Test In Terminal" CodeLens feature to fail on Windows because the regex pattern becomes incorrectly escaped.

Changes:

• Added escape_for_shell() helper that uses Gem.win_platform? to detect Windows
• On Windows: returns text unescaped (Windows cmd parsing doesn't need same escaping)
• On Unix: continues using Shellwords.escape()

[rafaelfranca]

Fix looks good. Can you sign the CLA please?

[vinistock]

Are you positive there's no need for any escaping? There are differences between PowerShell and exe and we have been bitten before with syntax errors that only happened in one or the other.

Would it be possible to add integration tests that generate the test commands and then attempt to run it on both PowerShell and exe? That would be super helpful for ensuring correctness.

[rafaelfranca]

All shells need escaping, but not for the arguments being passed to this method. They are all generated by our code, not based on user input, no? They would only need escaping if we were passing quoted strings as arguments to this method.

[rafaelfranca]

We should also make sure we avoid escaping in the calls that would receive a regexp. I think it is only the method name

[vinistock]

This is escaping the user's test class names and test method names, which may include regex wild cards for dynamically defined classes.

For example:

module Foo
  module variable
    class MyTest < Minitest::Test
    end
  end
end

This will generate a regex with a wild card to allow us to execute the test despite the dynamic portion of the nesting.

Are all characters involved valid for PowerShell and exe? That's the part I'm not sure.

For example, we started escaping the regex anchor $ because not escaping it breaks execution on the fish shell.

At the end of the day, we need proper tests to verify that we generate can be correctly executed on Windows or we'll always be subject to a small modification not being syntax compatible with its shells.

[rafaelfranca]

Thank you for the pull request. I think we should go with #4012 instead

[rafaelfranca]

Nevermind, this fix need to happen in multiple place. I do think we should not be escaping on windows for the method name, but we should fix this problem in all places that need to be fixed.