diff --git a/.github/workflows/super-linter.lock.yml b/.github/workflows/super-linter.lock.yml index 39595e0b196..5333f7987c4 100644 --- a/.github/workflows/super-linter.lock.yml +++ b/.github/workflows/super-linter.lock.yml @@ -169,7 +169,7 @@ jobs: with: persist-credentials: false - name: Download super-linter log - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 + uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 with: name: super-linter-log path: /tmp/gh-aw/ @@ -4532,7 +4532,7 @@ jobs: persist-credentials: false - name: Super-linter id: super-linter - uses: super-linter/super-linter@2bdd90ed3262e023ac84bf8fe35dc480721fc1f2 + uses: super-linter/super-linter@v8.2.1 env: CREATE_LOG_FILE: "true" DEFAULT_BRANCH: main @@ -4554,7 +4554,7 @@ jobs: fi - name: Upload super-linter log if: always() - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 with: name: super-linter-log path: super-linter.log diff --git a/pkg/cli/compile_command_test.go b/pkg/cli/compile_command_test.go new file mode 100644 index 00000000000..a9a98ec5aa3 --- /dev/null +++ b/pkg/cli/compile_command_test.go @@ -0,0 +1,476 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/githubnext/gh-aw/pkg/workflow" +) + +// TestCompileConfig tests the CompileConfig structure +func TestCompileConfig(t *testing.T) { + config := CompileConfig{ + MarkdownFiles: []string{"test.md"}, + Verbose: true, + EngineOverride: "copilot", + Validate: true, + Watch: false, + WorkflowDir: ".github/workflows", + NoEmit: false, + Purge: false, + TrialMode: false, + Strict: false, + Dependabot: false, + ForceOverwrite: false, + Zizmor: false, + Poutine: false, + Actionlint: false, + } + + // Verify all fields are accessible + if len(config.MarkdownFiles) != 1 { + t.Errorf("Expected 1 markdown file, got %d", len(config.MarkdownFiles)) + } + if !config.Verbose { + t.Error("Expected Verbose to be true") + } + if config.EngineOverride != "copilot" { + t.Errorf("Expected EngineOverride to be 'copilot', got %q", config.EngineOverride) + } +} + +// TestCompilationStats tests the CompilationStats structure +func TestCompilationStats(t *testing.T) { + stats := &CompilationStats{ + Total: 5, + Errors: 2, + Warnings: 3, + FailedWorkflows: []string{"workflow1.md", "workflow2.md"}, + } + + if stats.Total != 5 { + t.Errorf("Expected Total to be 5, got %d", stats.Total) + } + if stats.Errors != 2 { + t.Errorf("Expected Errors to be 2, got %d", stats.Errors) + } + if stats.Warnings != 3 { + t.Errorf("Expected Warnings to be 3, got %d", stats.Warnings) + } + if len(stats.FailedWorkflows) != 2 { + t.Errorf("Expected 2 failed workflows, got %d", len(stats.FailedWorkflows)) + } +} + +// TestPrintCompilationSummary tests the printCompilationSummary function +func TestPrintCompilationSummary(t *testing.T) { + tests := []struct { + name string + stats *CompilationStats + }{ + { + name: "no workflows", + stats: &CompilationStats{ + Total: 0, + Errors: 0, + Warnings: 0, + }, + }, + { + name: "successful compilation", + stats: &CompilationStats{ + Total: 5, + Errors: 0, + Warnings: 0, + }, + }, + { + name: "with warnings", + stats: &CompilationStats{ + Total: 5, + Errors: 0, + Warnings: 3, + }, + }, + { + name: "with errors", + stats: &CompilationStats{ + Total: 5, + Errors: 2, + Warnings: 1, + FailedWorkflows: []string{"workflow1.md", "workflow2.md"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // printCompilationSummary writes to stderr, we just verify it doesn't panic + printCompilationSummary(tt.stats) + }) + } +} + +// Note: TestHandleFileDeleted is already tested in commands_file_watching_test.go + +// TestCompileWorkflowWithValidation_InvalidFile tests error handling +func TestCompileWorkflowWithValidation_InvalidFile(t *testing.T) { + compiler := workflow.NewCompiler(false, "", "test") + + // Try to compile a non-existent file + err := CompileWorkflowWithValidation( + compiler, + "/nonexistent/file.md", + false, // verbose + false, // zizmor + false, // poutine + false, // actionlint + false, // strict + false, // validateActionSHAs + ) + + if err == nil { + t.Error("Expected error when compiling non-existent file, got nil") + } +} + +// TestCompileWorkflows_DependabotValidation tests dependabot flag validation +func TestCompileWorkflows_DependabotValidation(t *testing.T) { + tests := []struct { + name string + config CompileConfig + expectError bool + errorMsg string + }{ + { + name: "dependabot with specific files", + config: CompileConfig{ + Dependabot: true, + MarkdownFiles: []string{"test.md"}, + }, + expectError: true, + errorMsg: "cannot be used with specific workflow files", + }, + { + name: "dependabot with custom workflow dir", + config: CompileConfig{ + Dependabot: true, + WorkflowDir: "custom/workflows", + }, + expectError: true, + errorMsg: "cannot be used with custom --workflows-dir", + }, + { + name: "dependabot with default settings", + config: CompileConfig{ + Dependabot: true, + WorkflowDir: "", + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := CompileWorkflows(tt.config) + + if tt.expectError { + if err == nil { + t.Error("Expected error but got nil") + } else if !strings.Contains(err.Error(), tt.errorMsg) { + t.Errorf("Expected error containing %q, got %q", tt.errorMsg, err.Error()) + } + } + }) + } +} + +// TestCompileWorkflows_PurgeValidation tests purge flag validation +func TestCompileWorkflows_PurgeValidation(t *testing.T) { + config := CompileConfig{ + Purge: true, + MarkdownFiles: []string{"test.md"}, + } + + _, err := CompileWorkflows(config) + + if err == nil { + t.Error("Expected error when using purge with specific files, got nil") + } + + if !strings.Contains(err.Error(), "can only be used when compiling all markdown files") { + t.Errorf("Expected error about purge flag, got: %v", err) + } +} + +// TestCompileWorkflows_WorkflowDirValidation tests workflow directory validation +func TestCompileWorkflows_WorkflowDirValidation(t *testing.T) { + tests := []struct { + name string + workflowDir string + expectError bool + errorMsg string + }{ + { + name: "absolute path not allowed", + workflowDir: "/absolute/path", + expectError: true, + errorMsg: "must be a relative path", + }, + { + name: "relative path allowed", + workflowDir: "custom/workflows", + expectError: false, + }, + { + name: "default empty path", + workflowDir: "", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := CompileConfig{ + WorkflowDir: tt.workflowDir, + } + + _, err := CompileWorkflows(config) + + if tt.expectError { + if err == nil { + t.Error("Expected error but got nil") + } else if !strings.Contains(err.Error(), tt.errorMsg) { + t.Errorf("Expected error containing %q, got %q", tt.errorMsg, err.Error()) + } + } + }) + } +} + +// TestCompileWorkflowDataWithValidation_NoEmit tests validation without emission +func TestCompileWorkflowDataWithValidation_NoEmit(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.md") + + // Create a simple test workflow + workflowContent := `--- +on: push +permissions: + contents: read +--- + +# Test Workflow + +This is a test workflow. +` + if err := os.WriteFile(testFile, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + // Create compiler with noEmit flag + compiler := workflow.NewCompiler(false, "", "test") + compiler.SetNoEmit(true) + + // Parse the workflow + workflowData, err := compiler.ParseWorkflowFile(testFile) + if err != nil { + t.Fatalf("Failed to parse workflow: %v", err) + } + + // Compile without emitting + err = CompileWorkflowDataWithValidation( + compiler, + workflowData, + testFile, + false, // verbose + false, // zizmor + false, // poutine + false, // actionlint + false, // strict + false, // validateActionSHAs + ) + + // Should complete without error + if err != nil { + t.Errorf("Unexpected error with noEmit: %v", err) + } + + // Verify lock file was not created + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + if _, err := os.Stat(lockFile); !os.IsNotExist(err) { + t.Error("Lock file should not exist with noEmit flag") + } +} + +// TestCompileWorkflowWithValidation_YAMLValidation tests YAML validation +func TestCompileWorkflowWithValidation_YAMLValidation(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.md") + + // Create a simple test workflow + workflowContent := `--- +on: push +permissions: + contents: read +engine: copilot +--- + +# Test Workflow + +This is a test workflow. +` + if err := os.WriteFile(testFile, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + // Create compiler + compiler := workflow.NewCompiler(false, "", "test") + + // Compile the workflow + err := CompileWorkflowWithValidation( + compiler, + testFile, + false, // verbose + false, // zizmor + false, // poutine + false, // actionlint + false, // strict + false, // validateActionSHAs + ) + + // Should complete without error + if err != nil { + t.Errorf("Unexpected error during compilation: %v", err) + } + + // Verify lock file was created + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + if _, err := os.Stat(lockFile); os.IsNotExist(err) { + t.Error("Lock file should have been created") + } + + // Clean up + os.Remove(lockFile) +} + +// TestCompileConfig_DefaultValues tests default configuration values +func TestCompileConfig_DefaultValues(t *testing.T) { + config := CompileConfig{} + + // Verify default values + if config.Verbose { + t.Error("Expected Verbose to default to false") + } + if config.Validate { + t.Error("Expected Validate to default to false") + } + if config.Watch { + t.Error("Expected Watch to default to false") + } + if config.NoEmit { + t.Error("Expected NoEmit to default to false") + } + if config.Purge { + t.Error("Expected Purge to default to false") + } + if config.TrialMode { + t.Error("Expected TrialMode to default to false") + } + if config.Strict { + t.Error("Expected Strict to default to false") + } + if config.Dependabot { + t.Error("Expected Dependabot to default to false") + } + if config.ForceOverwrite { + t.Error("Expected ForceOverwrite to default to false") + } + if config.Zizmor { + t.Error("Expected Zizmor to default to false") + } + if config.Poutine { + t.Error("Expected Poutine to default to false") + } + if config.Actionlint { + t.Error("Expected Actionlint to default to false") + } +} + +// TestCompilationStats_DefaultValues tests default stats values +func TestCompilationStats_DefaultValues(t *testing.T) { + stats := &CompilationStats{} + + if stats.Total != 0 { + t.Errorf("Expected Total to default to 0, got %d", stats.Total) + } + if stats.Errors != 0 { + t.Errorf("Expected Errors to default to 0, got %d", stats.Errors) + } + if stats.Warnings != 0 { + t.Errorf("Expected Warnings to default to 0, got %d", stats.Warnings) + } + if len(stats.FailedWorkflows) != 0 { + t.Errorf("Expected FailedWorkflows to default to empty, got %d items", len(stats.FailedWorkflows)) + } +} + +// TestCompileWorkflows_EmptyMarkdownFiles tests compilation with no files specified +func TestCompileWorkflows_EmptyMarkdownFiles(t *testing.T) { + // This test requires being in a git repository + // We'll skip if not in a git repo + _, err := findGitRoot() + if err != nil { + t.Skip("Not in a git repository, skipping test") + } + + config := CompileConfig{ + MarkdownFiles: []string{}, + WorkflowDir: ".github/workflows", + } + + // This will try to compile all files in .github/workflows + // It may fail if the directory doesn't exist, which is expected + CompileWorkflows(config) + + // We don't check for specific error here as it depends on the repository state + // The test just ensures the function handles empty MarkdownFiles correctly +} + +// TestCompileWorkflows_TrialMode tests trial mode configuration +func TestCompileWorkflows_TrialMode(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.md") + + // Create a simple test workflow + workflowContent := `--- +on: push +permissions: + contents: read +engine: copilot +--- + +# Test Workflow + +This is a test workflow. +` + if err := os.WriteFile(testFile, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + config := CompileConfig{ + MarkdownFiles: []string{testFile}, + TrialMode: true, + TrialLogicalRepoSlug: "owner/trial-repo", + } + + _, err := CompileWorkflows(config) + + // The compilation may fail for various reasons in a test environment, + // but it should not panic and should handle trial mode settings + _ = err // We're just testing that the config is processed +} diff --git a/pkg/workflow/scripts_test.go b/pkg/workflow/scripts_test.go new file mode 100644 index 00000000000..60df2151b3d --- /dev/null +++ b/pkg/workflow/scripts_test.go @@ -0,0 +1,325 @@ +package workflow + +import ( + "strings" + "testing" +) + +// TestGetScriptFunctions tests that all script getter functions return non-empty scripts +func TestGetScriptFunctions(t *testing.T) { + tests := []struct { + name string + getFunc func() string + minSize int // Minimum expected size in bytes + }{ + { + name: "getCollectJSONLOutputScript", + getFunc: getCollectJSONLOutputScript, + minSize: 100, + }, + { + name: "getComputeTextScript", + getFunc: getComputeTextScript, + minSize: 100, + }, + { + name: "getSanitizeOutputScript", + getFunc: getSanitizeOutputScript, + minSize: 100, + }, + { + name: "getCreateIssueScript", + getFunc: getCreateIssueScript, + minSize: 100, + }, + { + name: "getAddLabelsScript", + getFunc: getAddLabelsScript, + minSize: 100, + }, + { + name: "getParseFirewallLogsScript", + getFunc: getParseFirewallLogsScript, + minSize: 100, + }, + { + name: "getCreateDiscussionScript", + getFunc: getCreateDiscussionScript, + minSize: 100, + }, + { + name: "getUpdateIssueScript", + getFunc: getUpdateIssueScript, + minSize: 100, + }, + { + name: "getCreateCodeScanningAlertScript", + getFunc: getCreateCodeScanningAlertScript, + minSize: 100, + }, + { + name: "getCreatePRReviewCommentScript", + getFunc: getCreatePRReviewCommentScript, + minSize: 100, + }, + { + name: "getAddCommentScript", + getFunc: getAddCommentScript, + minSize: 100, + }, + { + name: "getUploadAssetsScript", + getFunc: getUploadAssetsScript, + minSize: 100, + }, + { + name: "getPushToPullRequestBranchScript", + getFunc: getPushToPullRequestBranchScript, + minSize: 100, + }, + { + name: "getCreatePullRequestScript", + getFunc: getCreatePullRequestScript, + minSize: 100, + }, + { + name: "getInterpolatePromptScript", + getFunc: getInterpolatePromptScript, + minSize: 100, + }, + { + name: "getParseClaudeLogScript", + getFunc: getParseClaudeLogScript, + minSize: 100, + }, + { + name: "getParseCodexLogScript", + getFunc: getParseCodexLogScript, + minSize: 100, + }, + { + name: "getParseCopilotLogScript", + getFunc: getParseCopilotLogScript, + minSize: 100, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Call the function + script := tt.getFunc() + + // Verify the script is not empty + if script == "" { + t.Errorf("%s returned empty script", tt.name) + } + + // Verify the script meets minimum size requirements + if len(script) < tt.minSize { + t.Errorf("%s returned script smaller than expected: got %d bytes, want at least %d bytes", + tt.name, len(script), tt.minSize) + } + + // Verify the script is valid JavaScript (basic check) + // All our scripts should have some common JavaScript patterns + hasValidJSPattern := strings.Contains(script, "function") || + strings.Contains(script, "const") || + strings.Contains(script, "var") || + strings.Contains(script, "let") || + strings.Contains(script, "async") || + strings.Contains(script, "require") + + if !hasValidJSPattern { + t.Errorf("%s returned script that doesn't look like JavaScript", tt.name) + } + }) + } +} + +// TestScriptBundlingIdempotency tests that calling script functions multiple times returns the same result +func TestScriptBundlingIdempotency(t *testing.T) { + tests := []struct { + name string + getFunc func() string + }{ + {"getCollectJSONLOutputScript", getCollectJSONLOutputScript}, + {"getComputeTextScript", getComputeTextScript}, + {"getSanitizeOutputScript", getSanitizeOutputScript}, + {"getCreateIssueScript", getCreateIssueScript}, + {"getParseClaudeLogScript", getParseClaudeLogScript}, + {"getParseCodexLogScript", getParseCodexLogScript}, + {"getParseCopilotLogScript", getParseCopilotLogScript}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Call the function twice + first := tt.getFunc() + second := tt.getFunc() + + // Verify both calls return the same script + if first != second { + t.Errorf("%s returned different scripts on consecutive calls", tt.name) + } + + // Verify the result is cached (same memory address would be ideal, but we can't test that easily) + // Instead, we verify the content is exactly identical + if len(first) != len(second) { + t.Errorf("%s returned scripts with different lengths: first=%d, second=%d", + tt.name, len(first), len(second)) + } + }) + } +} + +// TestScriptContainsExpectedPatterns tests that scripts contain expected patterns +func TestScriptContainsExpectedPatterns(t *testing.T) { + tests := []struct { + name string + getFunc func() string + expectedPattern string + description string + }{ + { + name: "create_issue contains create logic", + getFunc: getCreateIssueScript, + expectedPattern: "create", + description: "create issue script should contain 'create'", + }, + { + name: "add_labels contains label logic", + getFunc: getAddLabelsScript, + expectedPattern: "label", + description: "add labels script should contain 'label'", + }, + { + name: "create_discussion contains discussion logic", + getFunc: getCreateDiscussionScript, + expectedPattern: "discussion", + description: "create discussion script should contain 'discussion'", + }, + { + name: "update_issue contains update logic", + getFunc: getUpdateIssueScript, + expectedPattern: "update", + description: "update issue script should contain 'update'", + }, + { + name: "add_comment contains comment logic", + getFunc: getAddCommentScript, + expectedPattern: "comment", + description: "add comment script should contain 'comment'", + }, + { + name: "create_pull_request contains PR logic", + getFunc: getCreatePullRequestScript, + expectedPattern: "pull", + description: "create pull request script should contain 'pull'", + }, + { + name: "parse_claude_log contains parsing logic", + getFunc: getParseClaudeLogScript, + expectedPattern: "parse", + description: "parse claude log script should contain 'parse'", + }, + { + name: "parse_codex_log contains parsing logic", + getFunc: getParseCodexLogScript, + expectedPattern: "parse", + description: "parse codex log script should contain 'parse'", + }, + { + name: "parse_copilot_log contains parsing logic", + getFunc: getParseCopilotLogScript, + expectedPattern: "parse", + description: "parse copilot log script should contain 'parse'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + script := tt.getFunc() + scriptLower := strings.ToLower(script) + + if !strings.Contains(scriptLower, tt.expectedPattern) { + t.Errorf("%s: expected pattern %q not found in script", tt.description, tt.expectedPattern) + } + }) + } +} + +// TestScriptNonEmpty tests that embedded source scripts are not empty +func TestScriptNonEmpty(t *testing.T) { + tests := []struct { + name string + script string + }{ + {"collectJSONLOutputScriptSource", collectJSONLOutputScriptSource}, + {"computeTextScriptSource", computeTextScriptSource}, + {"sanitizeOutputScriptSource", sanitizeOutputScriptSource}, + {"createIssueScriptSource", createIssueScriptSource}, + {"addLabelsScriptSource", addLabelsScriptSource}, + {"createDiscussionScriptSource", createDiscussionScriptSource}, + {"updateIssueScriptSource", updateIssueScriptSource}, + {"createCodeScanningAlertScriptSource", createCodeScanningAlertScriptSource}, + {"createPRReviewCommentScriptSource", createPRReviewCommentScriptSource}, + {"addCommentScriptSource", addCommentScriptSource}, + {"uploadAssetsScriptSource", uploadAssetsScriptSource}, + {"parseFirewallLogsScriptSource", parseFirewallLogsScriptSource}, + {"pushToPullRequestBranchScriptSource", pushToPullRequestBranchScriptSource}, + {"createPullRequestScriptSource", createPullRequestScriptSource}, + {"parseClaudeLogScriptSource", parseClaudeLogScriptSource}, + {"parseCodexLogScriptSource", parseCodexLogScriptSource}, + {"parseCopilotLogScriptSource", parseCopilotLogScriptSource}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.script == "" { + t.Errorf("%s is empty (go:embed failed)", tt.name) + } + + if len(tt.script) < 50 { + t.Errorf("%s is too short (%d bytes), go:embed may have failed", tt.name, len(tt.script)) + } + }) + } +} + +// TestScriptBundlingDoesNotFail tests that bundling never returns empty strings +func TestScriptBundlingDoesNotFail(t *testing.T) { + // This test ensures that even if bundling fails, we fallback to source + // by calling all getter functions and verifying they return non-empty strings + tests := []struct { + name string + getFunc func() string + }{ + {"getCollectJSONLOutputScript", getCollectJSONLOutputScript}, + {"getComputeTextScript", getComputeTextScript}, + {"getSanitizeOutputScript", getSanitizeOutputScript}, + {"getCreateIssueScript", getCreateIssueScript}, + {"getAddLabelsScript", getAddLabelsScript}, + {"getParseFirewallLogsScript", getParseFirewallLogsScript}, + {"getCreateDiscussionScript", getCreateDiscussionScript}, + {"getUpdateIssueScript", getUpdateIssueScript}, + {"getCreateCodeScanningAlertScript", getCreateCodeScanningAlertScript}, + {"getCreatePRReviewCommentScript", getCreatePRReviewCommentScript}, + {"getAddCommentScript", getAddCommentScript}, + {"getUploadAssetsScript", getUploadAssetsScript}, + {"getPushToPullRequestBranchScript", getPushToPullRequestBranchScript}, + {"getCreatePullRequestScript", getCreatePullRequestScript}, + {"getInterpolatePromptScript", getInterpolatePromptScript}, + {"getParseClaudeLogScript", getParseClaudeLogScript}, + {"getParseCodexLogScript", getParseCodexLogScript}, + {"getParseCopilotLogScript", getParseCopilotLogScript}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + script := tt.getFunc() + if script == "" { + t.Errorf("%s returned empty string - bundling failed and no fallback", tt.name) + } + }) + } +}