8000 Fix WorktreeSuite tests by adding required Author field by cappyzawa · Pull Request #1584 · go-git/go-git · GitHub
[go: up one dir, main page]
Skip to content

Fix WorktreeSuite tests by adding required Author field #1584

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2025
Merged

Fix WorktreeSuite tests by adding required Author field #1584

merged 1 commit into from
Jun 26, 2025

Conversation

cappyzawa
Copy link
Contributor
@cappyzawa cappyzawa commented Jun 18, 2025 •
edited
Loading

Summary

Background

Before proceeding with the relative submodule URL resolution in PR #1581, we need to ensure that all WorktreeSuite tests pass on the main branch. These tests were failing due to v6 branch requirement for Author field in CommitOptions.

Changes

  • TestCommitEmptyOptions: add defaultSignature() as Author
  • TestCommitAmendWithChanges: add defaultSignature() as Author
  • TestCheckoutRelativePathSubmoduleInitialized: add defaultSignature() as Author

Test Results

All WorktreeSuite tests now pass successfully:

=== RUN   TestWorktreeSuite
=== RUN   TestWorktreeSuite/TestAddAll
=== RUN   TestWorktreeSuite/TestAddAndCommit
...
--- PASS: TestWorktreeSuite (11.01s)
    --- PASS: TestWorktreeSuite/TestAddAll (0.01s)
    --- PASS: TestWorktreeSuite/TestAddAndCommit (0.01s)
    ...
    --- PASS: TestWorktreeSuite/TestCommitEmptyOptions (0.00s)
    --- PASS: TestWorktreeSuite/TestCommitAmendWithChanges (0.01s)
    --- PASS: TestWorktreeSuite/TestCheckoutRelativePathSubmoduleInitialized (1.94s)
    ...
PASS
ok  	github.com/go-git/go-git/v6	11.813s

@Copilot Copilot AI review requested due to automatic review settings June 18, 2025 03:06
Copilot
Copilot AI reviewed Jun 18, 2025
View reviewed changes
Copy link
Contributor
@Copilot 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 addresses failing WorktreeSuite tests by ensuring the CommitOptions now include the required Author field.

  • Fix failures in TestCommitEmptyOptions by adding defaultSignature() as Author
  • Fix failures in TestCommitAmendWithChanges by including Author in the CommitOptions
  • Fix failures in TestCheckoutRelativePathSubmoduleInitialized by supplying defaultSignature() as Author

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
worktree_test.go Updated Commit call in TestCheckoutRelativePathSubmoduleInitialized to include Author
worktree_commit_test.go Updated Commit calls in TestCommitEmptyOptions and TestCommitAmendWithChanges to include Author

@cappyzawa cappyzawa mentioned this pull request Jun 18, 2025
Merged
@cappyzawa cappyzawa force-pushed the fix/worktree-suite-tests branch from 1b6896e to eed4af6 Compare June 18, 2025 03:43
@cappyzawa
git: worktree, Add required Author field to fix failing tests
4543322 
Tests were failing due to v6 branch requirement for Author field
in CommitOptions. Added defaultSignature() as Author to:
- TestCommitEmptyOptions
- TestCommitAmendWithChanges
- TestCheckoutRelativePathSubmoduleInitialized

Signed-off-by: cappyzawa <cappyzawa@gmail.com>
@cappyzawa cappyzawa force-pushed the fix/worktree-suite-tests branch from eed4af6 to 4543322 Compare June 19, 2025 14:54
@pjbgf
Copy link
Member
pjbgf commented Jun 24, 2025

@cappyzawa I'm not sure this is needed, as per latest merge into main both test related GH actions ran successfully:

Same as for local using the version in main:

go test -timeout 30s -v -run '^TestWorktreeSuite$' -testify.m '^(TestCommitAmendWithChanges)$' github.com/go-git/go-git/v6
=== RUN   TestWorktreeSuite
=== RUN   TestWorktreeSuite/TestCommitAmendWithChanges
--- PASS: TestWorktreeSuite (0.02s)
    --- PASS: TestWorktreeSuite/TestCommitAmendWithChanges (0.01s)
PASS
ok  	github.com/go-git/go-git/v6	0.021s

@cappyzawa
Copy link
Contributor Author

@pjbgf Thanks for checking! I found a potential root cause of the discrepancy between CI and local environments.

Issue: go-git cannot read Git user configuration in my local environment, but can in CI.

Investigation:

  1. My local .gitconfig uses include to load user settings from ~/.gitconfig.local:
    [include]
    path = ~/.gitconfig.local
  2. The actual user configuration is in the included file:
    [user]
    name = cappyzawa
    email = mailto:cappyzawa@gmail.com
  3. go-git's config.LoadConfig() returns empty values for cfg.User.Name and cfg.User.Email in my setup.
  4. When I temporarily set the user config directly in .gitconfig (like CI does with git config --global), go-git can read it and tests pass.

Hypothesis: go-git's configuration parsing may not handle include directives properly, or there's another configuration-related issue in my environment.

Conclusion: The tests fail in certain local configurations where go-git cannot read user settings. The PR fixes this by explicitly providing the required Author field in tests, making them work regardless of configuration parsing issues.

pjbgf
pjbgf approved these changes Jun 26, 2025
View reviewed changes
Copy link
Member
@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cappyzawa thanks for looking into this. The changes should improve the reproduceability of the tests. 🙇

@pjbgf pjbgf merged commit 2d715e7 into go-git:main Jun 26, 2025
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@pjbgf pjbgf pjbgf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cappyzawa @pjbgf
0