Skip to content
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

Enable shell-related and env var tests Windows already supports #1845

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

EliahKagan
Copy link
Member

This enables two gix-command tests on Windows that had previously only run on Unix-like systems. One required a small change that didn't affect the robustness of the test, while the other required no change. Full details are in the commit messages.

@EliahKagan EliahKagan changed the title Enable shell and env var tests Windows already supports Enable shell-related and env var tests Windows already supports Feb 17, 2025
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great catch, thanks a lot!

I will merge once the PR drops the draft status (in case there was anything else that's missing).

@EliahKagan EliahKagan marked this pull request as ready for review February 17, 2025 07:33
@EliahKagan
Copy link
Member Author

EliahKagan commented Feb 17, 2025

I will merge once the PR drops the draft status (in case there was anything else that's missing).

Sorry about that, I just forgot to mark it ready for review! (My internet connection cut out for a bit and I forgot to come back to this to check the CI results.)

Edit: I've rebased, to make the history easier for me to visualize while working on related code, but there are no other changes.

The `disallow_shell` test case for `gix-command` has two parts. The
first part runs a command that would fail if not really run in a
shell, mostly to verify that the second part of the test is robust
enough. The second part actually tests the `Prepare::without_shell`
method.

While neither part should be made any less robust, only the first
part had failed on Windows, and the reason was (or had become) only
that it would not really run in a shell, due to being eligible for
manual argument splitting and `allow_manual_arg_splitting` being
`true` by default on Windows.

Therefore, this modifies the first part to use a method that sets
`allow_manual_arg_splitting` to `false`. This is sufficient to make
the test pass on Windows, so this also enables it on all platforms.
This enables the `environment_variables_are_passed_one_by_one` test
for `gix-command` on Windows. It was marked to run on Unix-like
systems only. But it runs and passes on Windows, requiring no
changes. (Possibly it hadn't passed at the time it was introduced.)

The `command_may_be_shell_script` call is sufficient even though it
does not disable manual argument splitting, because the command
used in this test contains `$` (to expand variables in the shell)
and is thus already ineligible for manual argument splitting.
@EliahKagan EliahKagan requested a review from Byron February 17, 2025 16:52
@Byron Byron merged commit 2efce72 into GitoxideLabs:main Feb 18, 2025
21 checks passed
@EliahKagan EliahKagan deleted the shell-tests-windows branch February 18, 2025 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants