Skip to content

Conversation

@satococoa
Copy link
Owner

@satococoa satococoa commented Dec 9, 2025

Resolves #60

Summary

  • add shared helper ConfigureTestRepo under internal/testutil
  • replace duplicated git user config setup in unit/e2e tests
  • add unit test for helper

Testing

  • GOCACHE=/tmp/wtp-clone-MFCs9r/.cache/go-build go tool task test

Summary by CodeRabbit

  • Tests

    • Added a unit test validating centralized test-repo configuration to ensure consistent git settings during tests.
    • Updated test suites and end-to-end test setup to use the centralized configuration helper for repository setup.
  • Chores

    • Consolidated repository initialization logic across test infrastructure to improve maintainability and reduce duplication.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 9, 2025 13:31
@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

Walkthrough

Centralizes duplicated git test configuration by introducing internal/testutil.ConfigureTestRepo and replacing inline git config invocations in multiple test setup locations with calls to that helper.

Changes

Cohort / File(s) Summary
New testutil helper
internal/testutil/git.go
Adds exported ConfigureTestRepo(t *testing.T, repoDir string, runner func(dir string, args ...string)) which runs a fixed sequence of git config commands via the provided runner.
Test for helper
internal/testutil/git_test.go
Adds unit test asserting the helper invokes three git config commands in order (user.name, user.email, commit.gpgsign=false).
Refactored test setup
Repository tests
internal/git/repository_test.go, internal/git/repository_remove_test.go
E2E framework
test/e2e/framework/framework.go
Replace inline git config invocations (user.name, user.email, commit.gpgsign) with calls to testutil.ConfigureTestRepo, delegating execution to existing run-in-dir runner functions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Attention points:
    • internal/testutil/git.go — confirm t.Helper() usage and correct argument/order passed to runner.
    • internal/testutil/git_test.go — verify the test captures commands reliably and cleans up temp dirs.
    • Updated callsites — ensure runner semantics (working directory, arg expansion) match prior inline behavior.

Possibly related issues

Possibly related PRs

Poem

🐇 I hopped through repos, quiet and spry,
Found three configs repeating nearby.
I stitched them as one, neat and sweet,
Now setup's tidy — a hopping feat! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'test: centralize git test repo config' accurately describes the main objective of the PR, which is to consolidate duplicated git configuration logic across multiple test files into a centralized helper function.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/testutil-configure

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a 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 ConfigureTestRepo helper 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.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between ef72813 and 529edbe.

📒 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 path github.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.go
  • internal/git/repository_test.go
  • internal/testutil/git_test.go
  • internal/testutil/git.go
  • internal/git/repository_remove_test.go
test/e2e/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

End-to-end tests must be placed in test/e2e and 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.go files 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.go
  • internal/testutil/git_test.go
  • internal/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.go
  • internal/git/repository_test.go
  • internal/testutil/git_test.go
  • internal/testutil/git.go
  • internal/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.go
  • internal/git/repository_test.go
  • internal/testutil/git_test.go
  • internal/testutil/git.go
  • internal/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.go
  • internal/testutil/git_test.go
  • 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: 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.go
  • internal/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.go
  • internal/git/repository_test.go
  • internal/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 ConfigureTestRepo helper. 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 calls t.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 ConfigureTestRepo helper is well-designed with clear documentation, proper use of t.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.

Copy link

@coderabbitai coderabbitai bot left a 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 using testing.TB instead of *testing.T for flexibility

To 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.T will continue to work unchanged.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 529edbe and 82bee8c.

📒 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 path github.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 configuration

The 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.

@satococoa satococoa force-pushed the feature/testutil-configure branch from 82bee8c to fa8bc97 Compare December 9, 2025 13:39
@satococoa satococoa self-assigned this Dec 12, 2025
@satococoa satococoa merged commit ca7d736 into main Dec 12, 2025
7 checks passed
@satococoa satococoa deleted the feature/testutil-configure branch December 12, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor test repository setup code duplication

2 participants