-
-
Notifications
You must be signed in to change notification settings - Fork 414
feat: Zoxide tests #1042
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
feat: Zoxide tests #1042
Conversation
lazysegtree
commented
Sep 4, 2025
- Replace all instances of 'github.com/lazysegtree/go-zoxide' imports with 'zoxidelib' alias for consistency across codebase
- Add comprehensive zoxide UI navigation test that types "dir2" to search and presses Enter to navigate
- Fix zoxide modal close behavior by adding m.Close() call in handleConfirm action
- Add GetResults() method to zoxide UI component for test access
- Create defaultTestModelWithZClient() helper function for zoxide testing
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| "reflect" | ||
| "runtime" | ||
|
|
||
| "github.com/lazysegtree/go-zoxide" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do a strict review for this PR
Check if any refactoring is breaking existing behaviour.
Check if any manual testing or unit testing is must have to ensure no regressions.
Check for any bugs, over-engineering or changes that will reduce code readability/maintainability in the long term
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the full diff for PR #1042 to understand all changes
echo "=== Getting full PR diff ==="
gh pr diff 1042
echo -e "\n=== Checking for any zoxide-related imports in the codebase ==="
rg -n "zoxide" --type go
echo -e "\n=== Checking for any references to zoxidelib in the codebase ==="
rg -n "zoxidelib" --type go
echo -e "\n=== Looking for any new imports that might replace the removed one ==="
rg -n "go-zoxide" --type go
Length of output: 16294
🏁 Script executed:
#!/bin/bash
# Check if there are any build issues by examining go.mod and dependencies
echo "=== Checking go.mod for zoxide dependency ==="
cat go.mod | grep -i zoxide
echo -e "\n=== Checking if there are any other inconsistent import usages ==="
echo "Files using 'zoxide' without alias:"
rg -l 'import.*"github.com/lazysegtree/go-zoxide"$' --type go
echo -e "\nFiles using 'zoxidelib' alias:"
rg -l 'zoxidelib.*"github.com/lazysegtree/go-zoxide"' --type go
echo -e "\n=== Checking for any potential issues with the test setup ==="
echo "Checking test file structure and imports:"
rg -n -A5 -B5 "func TestZoxide" src/internal/model_test.go
echo -e "\n=== Checking if the new GetResults method is properly defined ==="
rg -n -A3 -B3 "GetResults" src/internal/ui/zoxide/
echo -e "\n=== Checking the zoxide modal close fix ==="
rg -n -A5 -B5 "m.Close()" src/internal/ui/zoxide/model.go
Length of output: 2946
After conducting a thorough strict review of PR #1042, here's my comprehensive analysis:
✅ Overall Assessment: This is a well-executed refactoring with significant improvements
Changes Analysis:
-
Import Consistency Fix ✅
- Makes
zoxidelibalias consistent across all files - Previously
config_function.gowas the only file using barezoxideimport - No breaking changes - purely cosmetic improvement
- Makes
-
Comprehensive Test Coverage Addition ✅
- Adds 70 lines of robust zoxide functionality tests
- Tests both enabled/disabled zoxide scenarios
- Proper OS-specific handling (skips on non-Linux platforms)
- Uses isolated temporary directories for test data
- Tests modal interaction, navigation, and search functionality
-
Bug Fix ✅
- Fixes missing
m.Close()call after zoxide modal confirmation - Previously modal remained open after selection - now properly closes
- Fixes missing
-
Testing Infrastructure ✅
- Adds
defaultTestModelWithZClient()helper function - Adds
GetResults()method for test verification - Well-structured test isolation and cleanup
- Adds
No Regressions Identified:
- Import alias change doesn't affect runtime behavior
- All function signatures remain identical
- Dependency is properly declared in go.mod
- No breaking API changes
Recommended Testing:
While the code changes are safe, manual testing would be valuable to verify:
- Zoxide modal opening/closing behavior
- Directory navigation via zoxide search
- Keyboard interaction flow (especially the confirm action fix)
- Different zoxide configuration scenarios
Code Quality:
- Maintains excellent code readability
- No over-engineering detected
- Follows existing patterns and conventions
- Test coverage significantly improved
Verdict: This PR improves code consistency, adds missing test coverage, and fixes an actual bug. No breaking changes or regressions identified.
🧠 Learnings used
Learnt from: lazysegtree
PR: yorukot/superfile#979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:27:20.420Z
Learning: lazysegtree specifically asked for a "strict review to find bad stuff" in PR reviews, indicating he wants comprehensive analysis that identifies all potential issues regardless of scope, not just issues within the current PR's scope.
Learnt from: lazysegtree
PR: yorukot/superfile#950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
PR: yorukot/superfile#979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:54:31.444Z
Learning: When lazysegtree says a review is "inaccurate and pre-existing issues. Not much useful," it means I should focus specifically on bugs or problems introduced by the current PR changes, not architectural concerns or code quality issues that were already present. The refactoring work in superfile PRs is generally well-implemented and should be evaluated on whether the specific changes work correctly, not on broader codebase architecture.
Learnt from: lazysegtree
PR: yorukot/superfile#967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
PR: yorukot/superfile#963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
Learnt from: lazysegtree
PR: yorukot/superfile#1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
PR: yorukot/superfile#781
File: src/internal/model_render.go:254-256
Timestamp: 2025-04-28T04:27:33.074Z
Learning: When race conditions or other issues that are unrelated to the current PR's focus are identified during review, lazysegtree prefers creating separate GitHub issues to track them rather than addressing them in the current PR. This helps maintain PR focus and scope.
Learnt from: lazysegtree
PR: yorukot/superfile#781
File: src/internal/ui/sidebar/render.go:46-48
Timestamp: 2025-04-28T03:48:46.327Z
Learning: The user (lazysegtree) prefers to keep PRs focused and manageable in size, sometimes intentionally leaving TODO comments to track minor issues for later PRs rather than addressing everything at once.
Learnt from: lazysegtree
PR: yorukot/superfile#970
File: src/internal/ui/processbar/model.go:11-26
Timestamp: 2025-08-02T17:00:37.262Z
Learning: lazysegtree prefers to track memory leak issues and feature enhancements via dedicated GitHub issues when identified during PR reviews but are beyond the current PR scope, particularly for processbar-related improvements like cleanup mechanisms and keyboard shortcuts.
Learnt from: lazysegtree
PR: yorukot/superfile#979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:54:31.444Z
Learning: When reviewing PRs, especially large refactoring ones, focus specifically on issues introduced by the changes rather than flagging pre-existing architectural problems or code smells that were already present before the PR. Distinguish between new bugs versus existing technical debt.
Learnt from: lazysegtree
PR: yorukot/superfile#963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
PR: yorukot/superfile#963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
PR: yorukot/superfile#969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
PR: yorukot/superfile#985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
PR: yorukot/superfile#973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
PR: yorukot/superfile#924
File: src/internal/model_test.go:147-168
Timestamp: 2025-08-09T02:52:22.516Z
Learning: In superfile tests, lazysegtree prefers tests that accurately mimic the actual production behavior, including file operations like creating and deleting the LastDir file, rather than isolating such operations to temporary locations when the real behavior involves modifying/deleting those specific files.
9e4f93f to
8979545
Compare
- Replace all instances of 'github.com/lazysegtree/go-zoxide' imports with 'zoxidelib' alias for consistency across codebase - Add comprehensive zoxide UI navigation test that types "dir2" to search and presses Enter to navigate - Fix zoxide modal close behavior by adding m.Close() call in handleConfirm action - Add GetResults() method to zoxide UI component for test access - Create defaultTestModelWithZClient() helper function for zoxide testing - Install zoxide in GitHub Actions workflow for Linux to support zoxide tests Co-Authored-By: Nitin Kumar <59679977+lazysegtree@users.noreply.github.com>
8979545 to
1987c24
Compare
…ing default directory causing zoxide issues
43f04f0 to
27c3960
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gates Failed
Prevent hotspot decline
(1 hotspot with Deep, Nested Complexity)
Enforce critical code health rules
(1 file with Deep, Nested Complexity)
Gates Passed
1 Quality Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| config_function.go | 1 rule in this hotspot | 9.54 → 8.96 | Suppress |
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| config_function.go | 1 critical rule | 9.54 → 8.96 | Suppress |
Quality Gate Profile: The Bare Minimum
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
- Replace all instances of 'github.com/lazysegtree/go-zoxide' imports with 'zoxidelib' alias for consistency across codebase - Add comprehensive zoxide UI navigation test that types "dir2" to search and presses Enter to navigate - Fix zoxide modal close behavior by adding m.Close() call in handleConfirm action - Add GetResults() method to zoxide UI component for test access - Create defaultTestModelWithZClient() helper function for zoxide testing --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: yorukot <107802416+yorukot@users.noreply.github.com>
- Replace all instances of 'github.com/lazysegtree/go-zoxide' imports with 'zoxidelib' alias for consistency across codebase - Add comprehensive zoxide UI navigation test that types "dir2" to search and presses Enter to navigate - Fix zoxide modal close behavior by adding m.Close() call in handleConfirm action - Add GetResults() method to zoxide UI component for test access - Create defaultTestModelWithZClient() helper function for zoxide testing --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: yorukot <107802416+yorukot@users.noreply.github.com>
PRs - #1036 - #1037 - #1041 - #1042 Issues - #657 <img width="645" height="295" alt="Image" src="https://github.com/user-attachments/assets/238f6549-5318-49d1-a3a0-14cf8a686955" /> <img width="544" height="114" alt="image" src="https://github.com/user-attachments/assets/a4f28cee-83be-4918-a5d1-b1c7f574ff9d" /> <img width="563" height="238" alt="image" src="https://github.com/user-attachments/assets/923a983e-3a1f-40c4-a989-7d89deff6257" /> when zoxide is set to false, or zoxide is not installed <img width="543" height="101" alt="image" src="https://github.com/user-attachments/assets/b717df10-b9f9-478e-a846-4a4d3461dad6" /> Library - https://pkg.go.dev/github.com/lazysegtree/go-zoxide <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Zoxide navigation modal: press "z" to search, view top matches, and jump to directories with keyboard navigation and live suggestions. * **Bug Fixes** * Safer directory resolution with zoxide fallback and integrity checks for invalid paths. * **Chores** * Added zoxide dependency and package metadata; CI updated to install zoxide. * **Documentation** * Added Zoxide README, hotkey docs, and plugin usage. * **Tests** * End-to-end tests covering Zoxide-enabled and disabled flows. * **Refactor** * Moved panel directory update logic onto the model. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ---------