fix(paths): Add cross-platform path normalization#18939
Conversation
Introduces a ormalizePath utility function to provide consistent path representations across different operating systems. It resolves paths to absolute, converts backslashes to forward slashes, and handles Windows case-insensitivity by lowercasing the path. fix(tests): Improve tool confirmation test stability Refactors the confirmation.test.ts to ensure reliable execution of asynchronous test cases by correctly mocking and handling the sequence of events in the tool confirmation flow.
Summary of ChangesHello @spencer426, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the application's handling of file paths by introducing a robust cross-platform normalization utility. This ensures consistent path comparisons and operations regardless of the operating system, which is crucial for preventing subtle bugs. Concurrently, the reliability of the test suite has been enhanced, specifically addressing flakiness in asynchronous confirmation flows, leading to more stable and trustworthy tests. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Size Change: +56 B (0%) Total Size: 24.4 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request introduces a normalizePath utility function to ensure consistent path representations across different operating systems, which is a valuable addition for cross-platform compatibility. It correctly handles absolute paths, converts backslashes to forward slashes, and addresses Windows case-insensitivity by lowercasing paths. Additionally, the pull request includes refactoring of confirmation.test.ts to enhance test reliability by improving the mocking strategy and event handling, specifically targeting previously flaky asynchronous test cases. The identified issues in the review comments regarding test redundancy and assertion placement should be addressed to further improve test stability and clarity. Overall, the changes are positive for both functionality and test stability.
|
|
||
| beforeEach(() => { | ||
| signal = new AbortController().signal; | ||
| vi.mocked(resolveEditorAsync).mockResolvedValue('vim'); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| const result = await promise; | ||
| expect(result.outcome).toBe(ToolConfirmationOutcome.ProceedOnce); | ||
| expect(mockModifier.handleModifyWithEditor).toHaveBeenCalled(); | ||
| expect(mockState.updateArgs).toHaveBeenCalled(); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Move updateArgs assertion in confirmation.test.ts to match execution timing. - Update shortenPath to handle both forward and backward slashes. - Refactor isWithinRoot to use normalizePath for consistent Windows behavior. - Use forward slashes for trailing separators in normalized paths.
- Fix assertion placement for updateArgs in ModifyWithEditor loop. - Remove redundant awaitConfirmation unit tests covered by resolveConfirmation integration.
9773ee7 to
4afb04b
Compare
- Move updateArgs assertion to accurately reflect execution flow. - Retain resolveEditorAsync mock with clarification to ensure test stability.
4afb04b to
4c41c2b
Compare

Summary
Introduces a
normalizePathutility function to provide consistent path representations across different operating systems. This helps prevent cross-platform inconsistencies, particularly between Windows and POSIX-based systems.Details
This PR includes two main improvements:
feat(paths): A newnormalizePathutility function is created. It resolves paths to their absolute form, converts all backslashes to forward slashes, and correctly handles Windows case-insensitivity by lowercasing the entire path. This ensures a canonical path format is used throughout the application.fix(tests): The test suite is made more robust by refactoringconfirmation.test.ts. The change ensure the reliable execution of asynchronous test cases by correctly mocking and handling the sequence of events in the tool confirmation flow, reducing potential flakiness.Related Issues
Fixes #17136
How to Validate
npm testnpm test -- packages/core/src/utils/paths.test.tsnpm test -- packages/core/src/scheduler/confirmation.test.tsPre-Merge Checklist