Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions pkg/cli/init_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,47 @@ func TestInitCommandInteractiveModeDetection(t *testing.T) {
}
}

func TestInitCommandCreatesCustomAgentByDefault(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The new test mutates global process state (os.Chdir) without t.Parallel(), while several sibling tests do run in parallel — this creates a race condition if the Go test runner schedules them concurrently.

💡 Why this matters and how to fix it

Tests that call os.Chdir change the working directory for the entire process, not just the current goroutine. If two such tests run concurrently one will silently operate in the wrong directory.

The safest fix is to not use os.Chdir at all. Pass the working directory explicitly to the command, or use t.Chdir(tmpDir) (Go 1.24+) which automatically serialises directory changes and restores on cleanup.

If the command API requires the cwd to be implicit, mark the test with a serial-safe pattern or accept that the suite must run with -p 1.

Existing tests in this file (TestInitRepositoryBasic, etc.) follow the same os.Chdir pattern, so this is a pre-existing issue — but the new test should not perpetuate it without a comment.

@copilot please address this.

tmpDir := testutil.TempDir(t, "test-*")

originalDir, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get current directory: %v", err)
}
defer func() {
_ = os.Chdir(originalDir)
}()

if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change to temp directory: %v", err)
}

if err := exec.Command("git", "init").Run(); err != nil {
t.Skip("Git not available")
}
Comment on lines +198 to +200
if err := exec.Command("git", "config", "user.name", "Test User").Run(); err != nil {
t.Fatalf("Failed to set git user.name: %v", err)
}
if err := exec.Command("git", "config", "user.email", "test@example.com").Run(); err != nil {
t.Fatalf("Failed to set git user.email: %v", err)
}

cmd := NewInitCommand()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The PR description says this guards against flag wiring regressions, but only the positive case (default path creates the agent) is tested — the inverse is missing.

💡 Suggested inverse test

Add a companion test that passes --no-agent and asserts the agent file is not created:

cmd := NewInitCommand()
cmd.SetArgs([]string{"--no-agent"})
if err := cmd.Execute(); err != nil {
    t.Fatalf("init command failed: %v", err)
}

agentPath := filepath.Join(".github", "agents", "agentic-workflows.md")
if _, err := os.Stat(agentPath); !os.IsNotExist(err) {
    t.Error("Expected --no-agent to suppress custom agent file creation")
}

Without this, the flag wiring can silently break in the other direction (e.g., --no-agent stops suppressing) and no test will catch it.

@copilot please address this.

cmd.SetArgs([]string{})
if err := cmd.Execute(); err != nil {
t.Fatalf("init command failed: %v", err)
}

agentPath := filepath.Join(".github", "agents", "agentic-workflows.md")
agentContent, err := os.ReadFile(agentPath)
if err != nil {
t.Fatalf("Expected Agentic Workflows custom agent file to be created at %s: %v", agentPath, err)
}
if !strings.Contains(string(agentContent), "name: Agentic Workflows") {
t.Error("Expected Agentic Workflows custom agent file to use the Agentic Workflows name")
}
Comment on lines +219 to +221
}

func TestInitRepositoryBasic(t *testing.T) {
tmpDir := testutil.TempDir(t, "test-*")

Expand Down
Loading