-
Notifications
You must be signed in to change notification settings - Fork 10
test: centralize git test repo config #62
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
Conversation
WalkthroughCentralizes duplicated git test configuration by introducing internal/testutil.ConfigureTestRepo and replacing inline Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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.
Pull request overview
This PR centralizes duplicated git test repository configuration setup by introducing a shared helper function ConfigureTestRepo in the internal/testutil package.
- Adds
ConfigureTestRepohelper function to eliminate duplicated git config setup across test files - Replaces three instances of duplicated git user configuration in unit and e2e tests
- Includes comprehensive unit test for the new helper function
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/testutil/git.go |
New helper function that centralizes git config setup (user.name, user.email, commit.gpgsign) with flexible runner interface |
internal/testutil/git_test.go |
Unit test verifying ConfigureTestRepo correctly calls git commands with expected arguments |
test/e2e/framework/framework.go |
Replaces inline git config commands with ConfigureTestRepo helper |
internal/git/repository_test.go |
Replaces duplicated git config setup with ConfigureTestRepo helper |
internal/git/repository_remove_test.go |
Replaces duplicated git config setup with ConfigureTestRepo helper |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
internal/git/repository_remove_test.go(2 hunks)internal/git/repository_test.go(2 hunks)internal/testutil/git.go(1 hunks)internal/testutil/git_test.go(1 hunks)test/e2e/framework/framework.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Follow standard Go style with tabs and gofmt; package names must be short and lowercase
Keep import groups tidy; use goimports for organization with local prefix following module pathgithub.com/satococoa/wtp/v2
Adhere to linting rules in.golangci.yml(vet, staticcheck, gosec, mnd, lll=120)
Wrap errors with context; avoid ignoring errors in Go code
Use snake_case for Go filenames (e.g.,remove.go,shell_integration.go)
Document exported items in Go when non-trivial; follow godoc conventions
Files:
test/e2e/framework/framework.gointernal/git/repository_test.gointernal/testutil/git_test.gointernal/testutil/git.gointernal/git/repository_remove_test.go
test/e2e/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
End-to-end tests must be placed in
test/e2eand exercise real git workflows using the built binary
Files:
test/e2e/framework/framework.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Unit tests must be placed alongside packages as*_test.gofiles in the same directory
Target 70% code coverage with unit tests focusing on fast feedback and mocked git interactions using table-driven test cases
Files:
internal/git/repository_test.gointernal/testutil/git_test.gointernal/git/repository_remove_test.go
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to **/*_test.go : Target 70% code coverage with unit tests focusing on fast feedback and mocked git interactions using table-driven test cases
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to **/*_test.go : Target 70% code coverage with unit tests focusing on fast feedback and mocked git interactions using table-driven test cases
Applied to files:
test/e2e/framework/framework.gointernal/git/repository_test.gointernal/testutil/git_test.gointernal/testutil/git.gointernal/git/repository_remove_test.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to test/e2e/**/*.go : End-to-end tests must be placed in `test/e2e` and exercise real git workflows using the built binary
Applied to files:
test/e2e/framework/framework.gointernal/git/repository_test.gointernal/testutil/git_test.gointernal/testutil/git.gointernal/git/repository_remove_test.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Internal packages are organized in `internal/{git,config,hooks,command,errors,io}`; do not add new packages outside this structure without discussion
Applied to files:
test/e2e/framework/framework.gointernal/testutil/git_test.gointernal/testutil/git.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: The root module is `github.com/satococoa/wtp/v2` running on Go 1.24; maintain module path consistency
Applied to files:
test/e2e/framework/framework.gointernal/git/repository_test.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: CLI entrypoint is located at `cmd/wtp`; new command implementations must integrate with the main CLI structure
Applied to files:
test/e2e/framework/framework.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to **/*.go : Keep import groups tidy; use goimports for organization with local prefix following module path `github.com/satococoa/wtp/v2`
Applied to files:
test/e2e/framework/framework.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Run commands from inside a worktree to mimic real usage (e.g., `go run ../cmd/wtp add feature/new-feature`)
Applied to files:
test/e2e/framework/framework.gointernal/git/repository_test.gointernal/git/repository_remove_test.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to **/*_test.go : Unit tests must be placed alongside packages as `*_test.go` files in the same directory
Applied to files:
internal/testutil/git_test.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Use `go run ./cmd/wtp <args>` for rapid feedback during development instead of building binaries
Applied to files:
internal/git/repository_remove_test.go
🧬 Code graph analysis (4)
test/e2e/framework/framework.go (1)
internal/testutil/git.go (1)
ConfigureTestRepo(9-21)
internal/git/repository_test.go (1)
internal/testutil/git.go (1)
ConfigureTestRepo(9-21)
internal/testutil/git_test.go (1)
internal/testutil/git.go (1)
ConfigureTestRepo(9-21)
internal/git/repository_remove_test.go (1)
internal/testutil/git.go (1)
ConfigureTestRepo(9-21)
🪛 GitHub Actions: CI
internal/testutil/git.go
[error] 1-1: golangci-lint failed: package-comments: should have a package comment (revive). Command: 'golangci-lint run --timeout=5m'.
🪛 GitHub Check: Lint
internal/testutil/git.go
[failure] 1-1:
package-comments: should have a package comment (revive)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: E2E Tests (macos-latest)
🔇 Additional comments (8)
test/e2e/framework/framework.go (2)
12-13: LGTM!The import addition follows Go conventions with proper separation between standard library and internal packages.
102-104: LGTM!The refactoring successfully centralizes git configuration setup. The adapter function properly delegates to
runInDir, maintaining the existing error handling behavior while reducing code duplication.internal/git/repository_remove_test.go (2)
9-10: LGTM!The import addition follows Go conventions and is consistent with the changes in other test files.
37-39: LGTM!The refactoring successfully centralizes git configuration setup. The adapter function properly delegates to
runGitCommand, maintaining consistency with the existing test infrastructure.internal/testutil/git_test.go (1)
8-30: LGTM!The test is well-structured and provides comprehensive coverage for the
ConfigureTestRepohelper. It effectively verifies that all three git config commands are executed in the correct order with the expected arguments.internal/git/repository_test.go (2)
9-10: LGTM!The import addition follows Go conventions and is consistent with the changes in other test files.
16-24: LGTM!The refactoring introduces a clean local helper function that encapsulates git command execution and properly integrates with
ConfigureTestRepo. The helper correctly callst.Helper()and handles errors appropriately, maintaining consistency with Go testing best practices.Also applies to: 38-38
internal/testutil/git.go (1)
5-21: LGTM!The
ConfigureTestRepohelper is well-designed with clear documentation, proper use oft.Helper(), and a flexible runner pattern that works consistently across unit and e2e tests. The implementation successfully centralizes git configuration setup and reduces code duplication.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/testutil/git.go (1)
10-21: Consider usingtesting.TBinstead of*testing.Tfor flexibilityTo allow reuse in benchmarks or helper code that only has a
testing.TB, you could generalize the signature slightly:-import "testing" +import "testing" @@ -func ConfigureTestRepo(t *testing.T, repoDir string, runner func(dir string, args ...string)) { +func ConfigureTestRepo(t testing.TB, repoDir string, runner func(dir string, args ...string)) {Call sites that currently pass
*testing.Twill continue to work unchanged.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/testutil/git.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Follow standard Go style with tabs and gofmt; package names must be short and lowercase
Keep import groups tidy; use goimports for organization with local prefix following module pathgithub.com/satococoa/wtp/v2
Adhere to linting rules in.golangci.yml(vet, staticcheck, gosec, mnd, lll=120)
Wrap errors with context; avoid ignoring errors in Go code
Use snake_case for Go filenames (e.g.,remove.go,shell_integration.go)
Document exported items in Go when non-trivial; follow godoc conventions
Files:
internal/testutil/git.go
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to **/*_test.go : Target 70% code coverage with unit tests focusing on fast feedback and mocked git interactions using table-driven test cases
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to **/*_test.go : Target 70% code coverage with unit tests focusing on fast feedback and mocked git interactions using table-driven test cases
Applied to files:
internal/testutil/git.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to test/e2e/**/*.go : End-to-end tests must be placed in `test/e2e` and exercise real git workflows using the built binary
Applied to files:
internal/testutil/git.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Run `go tool task dev` before committing to ensure code is formatted, linted, and tests pass
Applied to files:
internal/testutil/git.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
internal/testutil/git.go (1)
1-22: Helper correctly centralizes git test configurationThe package comment, exported helper doc, and use of
t.Helper()look good. The command list matches typical test-only git settings, and delegating execution/error handling to the injected runner keeps this utility flexible for both unit and e2e tests. Based on learnings, this nicely supports consistent git setup across mocked unit tests and real e2e workflows.
82bee8c to
fa8bc97
Compare
Resolves #60
Summary
Testing
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.