fix: resolve 4 bugs found during e2e testing#25
Merged
Conversation
1. Checkout remote-only branches: add git fetch before checkout so git can create a local tracking branch from the remote ref (was failing silently because ls-remote doesn't update local refs). 2. Status DIRTY column: change label from 'N modified' to 'N changed' since the count includes untracked files, not just modifications. 3. Repo add alias conflict: return non-zero exit code when an alias collision occurs with a different repo path. Previously exited 0 with a red message but no error, confusing scripts/CI. 4. Update/checkout exit code: propagate failures from runner.Run so commands return non-zero when any repository operation fails. Added HasErrors/ErrorCount helpers to the runner package.
|
✅ All checks passed (Go 1.26) Coverage: 68.1% |
There was a problem hiding this comment.
Pull request overview
Fixes multiple CLI correctness issues uncovered by e2e testing, primarily around git branch checkout behavior, exit codes on failure, and clearer status/add-repo behavior.
Changes:
- Add
git.FetchBranch()and use it to support checking out remote-only branches. - Propagate parallel runner failures to command exit codes via new
runner.HasErrors()/runner.ErrorCount()helpers. - Improve UX/correctness: status “DIRTY” label wording, and
repo addalias-collision handling (including symlink-normalized path comparison).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/runner/parallel.go | Adds HasErrors() / ErrorCount() helpers for result slices. |
| internal/runner/parallel_test.go | Adds unit tests for new runner helpers. |
| internal/git/git.go | Adds FetchBranch() helper to fetch a single remote branch. |
| internal/git/git_test.go | Adds tests covering fetching remote-only branches and non-existent branches. |
| internal/cli/checkout.go | Fetches remote-only branches before checkout; returns errors when runner results contain failures. |
| internal/cli/checkout_run_test.go | Adds regression tests for remote-only checkout and non-zero on failure. |
| internal/cli/update.go | Returns an error when any repo update fails (based on runner results). |
| internal/cli/update_run_test.go | Adds regression test that update returns an error on repo failures. |
| internal/cli/status.go | Renames dirty label from “modified” to “changed”. |
| internal/cli/repo.go | Returns non-zero on alias conflicts; resolves symlinks when comparing stored vs new paths. |
| internal/cli/repo_test.go | Adds tests for alias conflict error and idempotent re-add of same path. |
|
✅ All checks passed (Go 1.26) Coverage: 68.1% |
alexandreafj
added a commit
that referenced
this pull request
May 1, 2026
- Use filepath.Join for non-existent path test (cross-platform) - Assert exit codes in AutoDetectWithDepth, remove _ = r1/r2 workaround - Replace generic "1" with "changed" in DirtyModified check - Use specific "No files matching" assertion in untrack test - Assert deterministic non-zero exit for unknown --repo alias - Assert idempotent behavior for duplicate path add (exit 0 + warning) - Assert non-zero exit for conflicting alias (fixed in PR #25)
alexandreafj
added a commit
that referenced
this pull request
May 1, 2026
* test: add e2e test suite for CLI binary-level testing Adds 77 automated e2e tests that build the gitm binary and exercise it as a real user would, using isolated HOME directories and real git repos. Covers: repo add/list/remove/rename, status, branch create/rename, checkout, update, track/untrack, commit, discard, stash, reset, upgrade, help/version. Key findings documented in test output via t.Log: - checkout fails for unfetched remote-only branches - status DIRTY column counts untracked files as 'modified' - duplicate alias on repo add silently ignored * fix: resolve golangci-lint issues in e2e tests - Check os.MkdirAll return values (errcheck) - Remove unused gitLog and isDirty helper methods (unused) - Use errors.As instead of type assertion on *exec.ExitError (errorlint) - Add missing errors import * fix: address PR review comments on e2e test suite - Cross-platform: replace /dev/null with os.DevNull, add Windows HOME env vars (USERPROFILE/HOMEDRIVE/HOMEPATH), clean up TestMain temp dir - Stronger assertions: replace t.Log-only tests with real assertions for remote-only checkout, diverged update, dirty branch create, non-existent branch rename, and stash list empty state - Broader match fixes: 'No' -> specific messages like 'No dirty repositories found', 'No untracked files found' - Skip TUI-dependent tests that cannot run in non-TTY environments (commit protection, reset, track file picker) - Remove custom contains/findSubstring reimplementing strings.Contains - Use filepath.Join instead of string concatenation with '/' - Remove unused dataDir struct field - Use upgrade --help instead of real network call in TestUpgrade * fix: address second round of PR review comments - Use filepath.Join for non-existent path test (cross-platform) - Assert exit codes in AutoDetectWithDepth, remove _ = r1/r2 workaround - Replace generic "1" with "changed" in DirtyModified check - Use specific "No files matching" assertion in untrack test - Assert deterministic non-zero exit for unknown --repo alias - Assert idempotent behavior for duplicate path add (exit 0 + warning) - Assert non-zero exit for conflicting alias (fixed in PR #25)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes 4 bugs discovered during comprehensive e2e testing (PR #24):
Checkout remote-only branches (critical):
git checkoutfailed for branches that only exist on origin becauseRemoteBranchExistsusesls-remote(which doesn't update local refs). Now fetches the branch first viagit fetch origin <branch>before attempting checkout.Status DIRTY column mislabel: Changed label from "N modified" to "N changed" since the count includes untracked files (
??), not just tracked modifications.repo addalias conflict exits 0: When adding a repo with a duplicate alias (pointing to a different path), the command printed a red error message but exited with code 0. Now correctly returns non-zero. Also handles macOS symlink path resolution (/var→/private/var) to avoid false alias collisions.update/checkoutexit 0 on failure: Both commands ignoredrunner.Run()results and always returnednil. Now check for errors and return appropriate exit codes. AddedHasErrors()/ErrorCount()helpers to the runner package.Changes
internal/git/git.go: AddFetchBranch()functioninternal/cli/checkout.go: Fetch before checkout for remote-only branches; propagate runner errorsinternal/cli/status.go: Fix label "modified" → "changed"internal/cli/repo.go: Return error on alias conflicts; resolve symlinks before comparing pathsinternal/cli/update.go: Propagate runner errorsinternal/runner/parallel.go: AddHasErrors()/ErrorCount()helpersTesting
-raceflagTestFetchBranch,TestFetchBranch_NonExistentBranch,TestRunCheckoutBranch_RemoteOnly,TestRunCheckoutDefault_ReturnsErrorOnFailure,TestRunUpdate_ReturnsErrorOnFailure,TestRepoAddAliasConflictReturnsError,TestRepoAddSamePathIdempotent,TestHasErrors,TestErrorCount