You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(desktop): account for PATH augmentation on macOS - #9659
On macOS a login shell can omit /usr/local/sbin. The subprocess environment intentionally prepends missing Homebrew/common directories, but the integration test compared the result to the raw shell PATH. It failed on a working setup while validating #9625.
Compare the result with the shell path after its public platform-specific augmentation. Keep the existing checks for string variables and Windows Path. The separate fallback tests cover exactly which directories get added and how existing entries are handled.
How I tested it
macOS, Bun 1.4.3: the original assertion failed with an extra /usr/local/sbin. Including that directory in the shell environment also reproduced why this passes on some machines.
After the change, the complete git utility and PATH fallback test files passed: 46 tests, 83 assertions.
Fixes the shell environment integration test on macOS so it no longer fails on valid setups.
The test compared the process PATH to the raw shell PATH, but the subprocess intentionally prepends missing Homebrew directories on macOS. The test now applies the same platform-specific augmentation before comparing, while keeping existing checks for string variables and Windows Path.
Written for commit 31e8131. Summary will update on new commits.
Summary by CodeRabbit
Tests
Updated the shell environment test to account for macOS-specific PATH adjustments.
No actionable comments were generated in the recent review.
Walkthrough
The shell environment test now applies augmentPathForMacOS to the expected PATH before comparing it with the value returned by getProcessEnvWithShellPath.
ParadevAI/Paradev#8891: This PR also changes the macOS PATH expectation in the same test.
Suggested reviewers:kitenite
Merge Risk: Minimal · up to 31e81
This is a test-only change that makes a PATH assertion account for the macOS PATH augmentation already applied in production code. It has no production behavior impact and is ready to merge.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title "test(desktop): account for PATH augmentation on macOS" follows conventional commit format and matches the changeset. The test now applies augmentPathForMacOS to the expected shell PATH befo…
Description check
The description covers the problem (macOS login shells can omit /usr/local/sbin, and the test compared against the raw shell PATH), the fix (compare against the augmented shell PATH while keeping the …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "test(desktop): account for PATH augmentation on macOS" follows conventional commit format and matches the changeset. The test now applies augmentPathForMacOS to the expected shell PATH before comparing it with getProcessEnvWithShellPath output, which is the main change.
Full details: Description check
Explanation
The description covers the problem (macOS login shells can omit /usr/local/sbin, and the test compared against the raw shell PATH), the fix (compare against the augmented shell PATH while keeping the string-variable and Windows Path checks), and testing details (macOS with Bun 1.4.3, 46 tests and 83 assertions passing, lint and typecheck passing, and a separate failure tracked in #9658). It does not follow the template's 'Link the issue' guidance for the related issue beyond #9625 and #9658 references, and it has no screenshots, which the template says are not needed for this test-only change. The checklist items are not marked, and the description does not state whether the PR title follows conventional commits (it does), whether maintainer edits are allowed, or CLA status. These are minor gaps relative to the template.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
On macOS a login shell can omit
/usr/local/sbin. The subprocess environment intentionally prepends missing Homebrew/common directories, but the integration test compared the result to the raw shell PATH. It failed on a working setup while validating #9625.Compare the result with the shell path after its public platform-specific augmentation. Keep the existing checks for string variables and Windows Path. The separate fallback tests cover exactly which directories get added and how existing entries are handled.
How I tested it
/usr/local/sbin. Including that directory in the shell environment also reproduced why this passes on some machines.Summary by cubic
Fixes the shell environment integration test on macOS so it no longer fails on valid setups.
The test compared the process PATH to the raw shell PATH, but the subprocess intentionally prepends missing Homebrew directories on macOS. The test now applies the same platform-specific augmentation before comparing, while keeping existing checks for string variables and Windows Path.
Written for commit 31e8131. Summary will update on new commits.
Summary by CodeRabbit
PATHadjustments.