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

tests: keep user's $PATH while running jj commands #5468

Merged
merged 1 commit into from
Jan 26, 2025

Conversation

scott2000
Copy link
Contributor

Currently, the Git subprocess tests only work on Linux due to a default path being used for the git executable when $PATH is unset. This can break if Git isn't installed at the expected path. Also, I believe it is currently necessary to set the $TEST_GIT_EXECUTABLE_PATH environment variable on Windows for tests to pass. Instead, we should use the user's $PATH to locate the git executable, as well as any other executables that are needed. This also makes $TEST_GIT_EXECUTABLE_PATH no longer necessary, so it can be removed.

Discussed in #5463.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

.github/workflows/build.yml Outdated Show resolved Hide resolved
cli/tests/common/mod.rs Outdated Show resolved Hide resolved
@scott2000 scott2000 force-pushed the push-msplqmqzksqv branch 3 times, most recently from 19d357b to 9542318 Compare January 26, 2025 01:55
@bsdinis
Copy link
Contributor

bsdinis commented Jan 26, 2025

This looks great! Thank you for doing this, getting a hang of it was very annoying to begin with and this is much cleaner!

I think we should definitely do the LC_ALL bit, but definitely in another PR

Currently, the Git subprocess tests only work on Linux due to a default
path being used for the `git` executable when `$PATH` is unset. This can
break if Git isn't installed at the expected path. Also, I believe it is
currently necessary to set the `$TEST_GIT_EXECUTABLE_PATH` environment
variable on Windows for tests to pass. Instead, we should use the user's
`$PATH` to locate the `git` executable, as well as any other executables
that are needed. This also makes `$TEST_GIT_EXECUTABLE_PATH` no longer
necessary, so it can be removed.
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

Thanks!

Sorry, I should have given approval yesterday. I didn't mean to blackmail you into changing the $PATH vs git.executable-path to my preference.

@scott2000 scott2000 added this pull request to the merge queue Jan 26, 2025
Merged via the queue into jj-vcs:main with commit 0006911 Jan 26, 2025
19 checks passed
@scott2000 scott2000 deleted the push-msplqmqzksqv branch January 26, 2025 17:31
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.

4 participants