Refactor: Extract common template sync logic to eliminate 80 lines of duplication

pkg/cli/copilot-agents.go

@@ -7,104 +7,78 @@ import (
 	"strings"
 )
 
-// ensureAgentFromTemplate ensures that an agent file exists and matches the embedded template
-func ensureAgentFromTemplate(agentFileName, templateContent string, verbose bool, skipInstructions bool) error {
+// ensureFileMatchesTemplate ensures a file in a subdirectory matches the expected template content
+func ensureFileMatchesTemplate(subdir, fileName, templateContent, fileType string, verbose bool, skipInstructions bool) error {
 	if skipInstructions {
-		return nil // Skip writing agent if flag is set
+		return nil
 	}
 
 	gitRoot, err := findGitRoot()
 	if err != nil {
 		return err // Not in a git repository, skip
 	}
 
-	agentsDir := filepath.Join(gitRoot, ".github", "agents")
-	agentPath := filepath.Join(agentsDir, agentFileName)
+	targetDir := filepath.Join(gitRoot, subdir)
+	targetPath := filepath.Join(targetDir, fileName)
 
-	// Ensure the .github/agents directory exists
-	if err := os.MkdirAll(agentsDir, 0755); err != nil {
-		return fmt.Errorf("failed to create .github/agents directory: %w", err)
+	// Ensure the target directory exists
+	if err := os.MkdirAll(targetDir, 0755); err != nil {
+		return fmt.Errorf("failed to create %s directory: %w", subdir, err)
 	}
 
-	// Check if the agent file already exists and matches the template
+	// Check if the file already exists and matches the template
 	existingContent := ""
-	if content, err := os.ReadFile(agentPath); err == nil {
+	if content, err := os.ReadFile(targetPath); err == nil {
 		existingContent = string(content)
 	}
 
 	// Check if content matches our expected template
 	expectedContent := strings.TrimSpace(templateContent)
 	if strings.TrimSpace(existingContent) == expectedContent {
 		if verbose {
-			fmt.Printf("Agent is up-to-date: %s\n", agentPath)
+			fmt.Printf("%s is up-to-date: %s\n", fileType, targetPath)
 		}
 		return nil
 	}
 
-	// Write the agent file
-	if err := os.WriteFile(agentPath, []byte(templateContent), 0644); err != nil {
-		return fmt.Errorf("failed to write agent file: %w", err)
+	// Write the file
+	if err := os.WriteFile(targetPath, []byte(templateContent), 0644); err != nil {
+		return fmt.Errorf("failed to write %s: %w", fileType, err)
 	}
 
 	if verbose {
 		if existingContent == "" {
-			fmt.Printf("Created agent: %s\n", agentPath)
+			fmt.Printf("Created %s: %s\n", fileType, targetPath)
 		} else {
-			fmt.Printf("Updated agent: %s\n", agentPath)
+			fmt.Printf("Updated %s: %s\n", fileType, targetPath)
 		}
 	}
 
 	return nil
 }
 
+// ensureAgentFromTemplate ensures that an agent file exists and matches the embedded template
+func ensureAgentFromTemplate(agentFileName, templateContent string, verbose bool, skipInstructions bool) error {
+	return ensureFileMatchesTemplate(
+		filepath.Join(".github", "agents"),
+		agentFileName,
+		templateContent,
+		"agent",
+		verbose,
+		skipInstructions,
+	)
+}
+
 // ensureCopilotInstructions ensures that .github/instructions/github-agentic-workflows.md contains the copilot instructions
 func ensureCopilotInstructions(verbose bool, skipInstructions bool) error {
-	if skipInstructions {
-		return nil // Skip writing instructions if flag is set
-	}
-
-	gitRoot, err := findGitRoot()
-	if err != nil {
-		return err // Not in a git repository, skip
-	}
-
-	copilotDir := filepath.Join(gitRoot, ".github", "instructions")
-	copilotInstructionsPath := filepath.Join(copilotDir, "github-agentic-workflows.instructions.md")
-
-	// Ensure the .github/instructions directory exists
-	if err := os.MkdirAll(copilotDir, 0755); err != nil {
-		return fmt.Errorf("failed to create .github/instructions directory: %w", err)
-	}
-
-	// Check if the instructions file already exists and matches the template
-	existingContent := ""
-	if content, err := os.ReadFile(copilotInstructionsPath); err == nil {
-		existingContent = string(content)
-	}
-
-	// Check if content matches our expected template
-	expectedContent := strings.TrimSpace(copilotInstructionsTemplate)
-	if strings.TrimSpace(existingContent) == expectedContent {
-		if verbose {
-			fmt.Printf("Copilot instructions are up-to-date: %s\n", copilotInstructionsPath)
-		}
-		return nil
-	}
-
-	// Write the copilot instructions file
-	if err := os.WriteFile(copilotInstructionsPath, []byte(copilotInstructionsTemplate), 0644); err != nil {
-		return fmt.Errorf("failed to write copilot instructions: %w", err)
-	}
-
-	if verbose {
-		if existingContent == "" {
-			fmt.Printf("Created copilot instructions: %s\n", copilotInstructionsPath)
-		} else {
-			fmt.Printf("Updated copilot instructions: %s\n", copilotInstructionsPath)
-		}
-	}
-
-	return nil
+	return ensureFileMatchesTemplate(
+		filepath.Join(".github", "instructions"),
+		"github-agentic-workflows.instructions.md",
+		copilotInstructionsTemplate,
+		"copilot instructions",
+		verbose,
+		skipInstructions,
+	)
 }
 
 // ensureAgenticWorkflowPrompt removes the old agentic workflow prompt file if it exists

pkg/cli/copilot_agents_test.go

@@ -0,0 +1,210 @@
+package cli
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/githubnext/gh-aw/pkg/testutil"
+)
+
+// TestEnsureFileMatchesTemplate tests the common helper function
+func TestEnsureFileMatchesTemplate(t *testing.T) {
+	tests := []struct {
+		name             string
+		subdir           string
+		fileName         string
+		templateContent  string
+		fileType         string
+		existingContent  string
+		skipInstructions bool
+		expectedFile     bool
+		expectedContent  string
+	}{
+		{
+			name:             "creates new file",
+			subdir:           ".github/test",
+			fileName:         "test.md",
+			templateContent:  "# Test Template",
+			fileType:         "test file",
+			existingContent:  "",
+			skipInstructions: false,
+			expectedFile:     true,
+			expectedContent:  "# Test Template",
+		},
+		{
+			name:             "does not modify existing correct file",
+			subdir:           ".github/test",
+			fileName:         "test.md",
+			templateContent:  "# Test Template",
+			fileType:         "test file",
+			existingContent:  "# Test Template",
+			skipInstructions: false,
+			expectedFile:     true,
+			expectedContent:  "# Test Template",
+		},
+		{
+			name:             "updates modified file",
+			subdir:           ".github/test",
+			fileName:         "test.md",
+			templateContent:  "# Test Template",
+			fileType:         "test file",
+			existingContent:  "# Old Content",
+			skipInstructions: false,
+			expectedFile:     true,
+			expectedContent:  "# Test Template",
+		},
+		{
+			name:             "skips when skipInstructions is true",
+			subdir:           ".github/test",
+			fileName:         "test.md",
+			templateContent:  "# Test Template",
+			fileType:         "test file",
+			existingContent:  "",
+			skipInstructions: true,
+			expectedFile:     false,
+			expectedContent:  "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a temporary directory for testing
+			tempDir := testutil.TempDir(t, "test-*")
+
+			// Change to temp directory and initialize git repo
+			oldWd, _ := os.Getwd()
+			defer func() {
+				_ = os.Chdir(oldWd)
+			}()
+			err := os.Chdir(tempDir)
+			if err != nil {
+				t.Fatalf("Failed to change directory: %v", err)
+			}
+
+			// Initialize git repo
+			if err := exec.Command("git", "init").Run(); err != nil {
+				t.Fatalf("Failed to init git repo: %v", err)
+			}
+
+			targetDir := filepath.Join(tempDir, tt.subdir)
+			targetPath := filepath.Join(targetDir, tt.fileName)
+
+			// Create initial content if specified
+			if tt.existingContent != "" {
+				if err := os.MkdirAll(targetDir, 0755); err != nil {
+					t.Fatalf("Failed to create target directory: %v", err)
+				}
+				if err := os.WriteFile(targetPath, []byte(tt.existingContent), 0644); err != nil {
+					t.Fatalf("Failed to create initial file: %v", err)
+				}
+			}
+
+			// Call the helper function
+			err = ensureFileMatchesTemplate(tt.subdir, tt.fileName, tt.templateContent, tt.fileType, false, tt.skipInstructions)
+			if err != nil {
+				t.Fatalf("ensureFileMatchesTemplate() returned error: %v", err)
+			}
+
+			// Check file existence
+			_, statErr := os.Stat(targetPath)
+			if tt.expectedFile && os.IsNotExist(statErr) {
+				t.Fatalf("Expected file to exist but it doesn't: %s", targetPath)
+			}
+			if !tt.expectedFile && !os.IsNotExist(statErr) {
+				t.Fatalf("Expected file to not exist but it does: %s", targetPath)
+			}
+
+			// Check content if file should exist
+			if tt.expectedFile {
+				content, err := os.ReadFile(targetPath)
+				if err != nil {
+					t.Fatalf("Failed to read file: %v", err)
+				}
+
+				contentStr := strings.TrimSpace(string(content))
+				expectedStr := strings.TrimSpace(tt.expectedContent)
+
+				if contentStr != expectedStr {
+					t.Errorf("Expected content does not match.\nExpected: %q\nActual: %q", expectedStr, contentStr)
+				}
+			}
+		})
+	}
+}
+
+// TestEnsureFileMatchesTemplate_VerboseOutput tests verbose logging
+func TestEnsureFileMatchesTemplate_VerboseOutput(t *testing.T) {
+	tests := []struct {
+		name            string
+		existingContent string
+		expectedLog     string
+	}{
+		{
+			name:            "logs creation",
+			existingContent: "",
+			expectedLog:     "Created",
+		},
+		{
+			name:            "logs update",
+			existingContent: "# Old Content",
+			expectedLog:     "Updated",
+		},
+		{
+			name:            "logs up-to-date",
+			existingContent: "# Test Template",
+			expectedLog:     "up-to-date",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a temporary directory for testing
+			tempDir := testutil.TempDir(t, "test-*")
+
+			// Change to temp directory and initialize git repo
+			oldWd, _ := os.Getwd()
+			defer func() {
+				_ = os.Chdir(oldWd)
+			}()
+			err := os.Chdir(tempDir)
+			if err != nil {
+				t.Fatalf("Failed to change directory: %v", err)
+			}
+
+			// Initialize git repo
+			if err := exec.Command("git", "init").Run(); err != nil {
+				t.Fatalf("Failed to init git repo: %v", err)
+			}
+
+			subdir := ".github/test"
+			fileName := "test.md"
+			targetDir := filepath.Join(tempDir, subdir)
+			targetPath := filepath.Join(targetDir, fileName)
+
+			// Create initial content if specified
+			if tt.existingContent != "" {
+				if err := os.MkdirAll(targetDir, 0755); err != nil {
+					t.Fatalf("Failed to create target directory: %v", err)
+				}
+				if err := os.WriteFile(targetPath, []byte(tt.existingContent), 0644); err != nil {
+					t.Fatalf("Failed to create initial file: %v", err)
+				}
+			}
+
+			// Call the helper function with verbose=true
+			// Note: This test doesn't capture stdout, it just verifies no errors occur
+			err = ensureFileMatchesTemplate(subdir, fileName, "# Test Template", "test file", true, false)
+			if err != nil {
+				t.Fatalf("ensureFileMatchesTemplate() returned error: %v", err)
+			}
+
+			// Verify file exists
+			if _, err := os.Stat(targetPath); os.IsNotExist(err) {
+				t.Fatalf("Expected file to exist")
+			}
+		})
+	}
+}