diff --git a/tools/flakeguard/cmd/aggregate_results.go b/tools/flakeguard/cmd/aggregate_results.go index 2133af071..48a2972c5 100644 --- a/tools/flakeguard/cmd/aggregate_results.go +++ b/tools/flakeguard/cmd/aggregate_results.go @@ -3,6 +3,7 @@ package cmd import ( "encoding/json" "fmt" + "os" "path/filepath" "time" @@ -15,7 +16,7 @@ import ( var AggregateResultsCmd = &cobra.Command{ Use: "aggregate-results", Short: "Aggregate test results into a single JSON report", - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { fs := reports.OSFileSystem{} // Get flag values @@ -37,7 +38,8 @@ var AggregateResultsCmd = &cobra.Command{ // Ensure the output directory exists if err := fs.MkdirAll(outputDir, 0755); err != nil { - return fmt.Errorf("error creating output directory: %w", err) + log.Error().Err(err).Str("path", outputDir).Msg("Error creating output directory") + os.Exit(ErrorExitCode) } // Start spinner for loading test reports @@ -59,16 +61,11 @@ var AggregateResultsCmd = &cobra.Command{ if err != nil { s.Stop() fmt.Println() - return fmt.Errorf("error loading test reports: %w", err) + log.Error().Err(err).Msg("Error aggregating test reports") + os.Exit(ErrorExitCode) } s.Stop() fmt.Println() - - if err != nil { - s.Stop() - return fmt.Errorf("error aggregating test reports: %w", err) - } - s.Stop() log.Debug().Msg("Successfully loaded and aggregated test reports") // Start spinner for mapping test results to paths @@ -80,9 +77,12 @@ var AggregateResultsCmd = &cobra.Command{ err = reports.MapTestResultsToPaths(aggregatedReport, repoPath) if err != nil { s.Stop() - return fmt.Errorf("error mapping test results to paths: %w", err) + fmt.Println() + log.Error().Err(err).Msg("Error mapping test results to paths") + os.Exit(ErrorExitCode) } s.Stop() + fmt.Println() log.Debug().Msg("Successfully mapped paths to test results") // Map test results to code owners if codeOwnersPath is provided @@ -94,9 +94,12 @@ var AggregateResultsCmd = &cobra.Command{ err = reports.MapTestResultsToOwners(aggregatedReport, codeOwnersPath) if err != nil { s.Stop() - return fmt.Errorf("error mapping test results to code owners: %w", err) + fmt.Println() + log.Error().Err(err).Msg("Error mapping test results to code owners") + os.Exit(ErrorExitCode) } s.Stop() + fmt.Println() log.Debug().Msg("Successfully mapped code owners to test results") } @@ -104,6 +107,7 @@ var AggregateResultsCmd = &cobra.Command{ return !tr.Skipped && tr.PassRatio < maxPassRatio }) s.Stop() + fmt.Println() // Check if there are any failed tests if len(failedTests) > 0 { @@ -125,7 +129,8 @@ var AggregateResultsCmd = &cobra.Command{ // Save the failed tests report with logs failedTestsReportWithLogsPath := filepath.Join(outputDir, "failed-test-results-with-logs.json") if err := reports.SaveReport(fs, failedTestsReportWithLogsPath, *failedReportWithLogs); err != nil { - return fmt.Errorf("error saving failed tests report with logs: %w", err) + log.Error().Err(err).Msg("Error saving failed tests report with logs") + os.Exit(ErrorExitCode) } log.Debug().Str("path", failedTestsReportWithLogsPath).Msg("Failed tests report with logs saved") @@ -139,7 +144,8 @@ var AggregateResultsCmd = &cobra.Command{ // Save the failed tests report without logs failedTestsReportNoLogsPath := filepath.Join(outputDir, "failed-test-results.json") if err := reports.SaveReport(fs, failedTestsReportNoLogsPath, *failedReportWithLogs); err != nil { - return fmt.Errorf("error saving failed tests report without logs: %w", err) + log.Error().Err(err).Msg("Error saving failed tests report without logs") + os.Exit(ErrorExitCode) } log.Debug().Str("path", failedTestsReportNoLogsPath).Msg("Failed tests report without logs saved") } else { @@ -156,7 +162,8 @@ var AggregateResultsCmd = &cobra.Command{ // Save the aggregated report to the output directory aggregatedReportPath := filepath.Join(outputDir, "all-test-results.json") if err := reports.SaveReport(fs, aggregatedReportPath, *aggregatedReport); err != nil { - return fmt.Errorf("error saving aggregated test report: %w", err) + log.Error().Err(err).Msg("Error saving aggregated test report") + os.Exit(ErrorExitCode) } log.Debug().Str("path", aggregatedReportPath).Msg("Aggregated test report saved") @@ -171,14 +178,16 @@ var AggregateResultsCmd = &cobra.Command{ err = generateAllTestsSummaryJSON(aggregatedReport, summaryFilePath, maxPassRatio) if err != nil { s.Stop() - return fmt.Errorf("error generating summary json: %w", err) + fmt.Println() + log.Error().Err(err).Msg("Error generating summary json") + os.Exit(ErrorExitCode) } s.Stop() + fmt.Println() log.Debug().Str("path", summaryFilePath).Msg("Summary generated") } log.Info().Str("summary", summaryFilePath).Str("report", aggregatedReportPath).Msg("Aggregation complete") - return nil }, } diff --git a/tools/flakeguard/cmd/check_test_owners.go b/tools/flakeguard/cmd/check_test_owners.go index cf838466d..4a4caeea0 100644 --- a/tools/flakeguard/cmd/check_test_owners.go +++ b/tools/flakeguard/cmd/check_test_owners.go @@ -1,6 +1,7 @@ package cmd import ( + "os" "path/filepath" "github.com/rs/zerolog/log" @@ -18,19 +19,21 @@ var ( var CheckTestOwnersCmd = &cobra.Command{ Use: "check-test-owners", Short: "Check which tests in the project do not have code owners", - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { projectPath, _ := cmd.Flags().GetString("project-path") // Scan project for test functions testFileMap, err := reports.ScanTestFiles(projectPath) if err != nil { - log.Fatal().Err(err).Msg("Error scanning test files") + log.Error().Err(err).Msg("Error scanning test files") + os.Exit(ErrorExitCode) } // Parse CODEOWNERS file codeOwnerPatterns, err := codeowners.Parse(codeownersPath) if err != nil { - log.Fatal().Err(err).Msg("Error parsing CODEOWNERS file") + log.Error().Err(err).Msg("Error parsing CODEOWNERS file") + os.Exit(ErrorExitCode) } // Check for tests without code owners @@ -73,8 +76,6 @@ var CheckTestOwnersCmd = &cobra.Command{ log.Debug().Msg("All Test functions have code owners!") } } - - return nil }, } diff --git a/tools/flakeguard/cmd/find.go b/tools/flakeguard/cmd/find.go index 0498d6d18..27fea7d1d 100644 --- a/tools/flakeguard/cmd/find.go +++ b/tools/flakeguard/cmd/find.go @@ -3,6 +3,7 @@ package cmd import ( "encoding/json" "fmt" + "os" "github.com/rs/zerolog/log" "github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard/git" @@ -32,7 +33,8 @@ var FindTestsCmd = &cobra.Command{ if findByTestFilesDiff { changedTestFiles, err := git.FindChangedFiles(projectPath, baseRef, "grep '_test\\.go$'") if err != nil { - log.Fatal().Err(err).Msg("Error finding changed test files") + log.Error().Err(err).Msg("Error finding changed test files") + os.Exit(ErrorExitCode) } if onlyShowChangedTestFiles { outputResults(changedTestFiles, jsonOutput) @@ -43,7 +45,8 @@ var FindTestsCmd = &cobra.Command{ } changedTestPkgs, err = golang.GetFilePackages(changedTestFiles) if err != nil { - log.Fatal().Err(err).Msg("Error getting package names for test files") + log.Error().Err(err).Msg("Error getting package names for test files") + os.Exit(ErrorExitCode) } } @@ -85,27 +88,32 @@ func init() { FindTestsCmd.Flags().Bool("only-show-changed-test-files", false, "Only show the changed test files and exit") if err := FindTestsCmd.MarkFlagRequired("base-ref"); err != nil { - log.Fatal().Err(err).Msg("Error marking base-ref as required") + log.Error().Err(err).Msg("Error marking base-ref as required") + os.Exit(ErrorExitCode) } } func findAffectedPackages(baseRef, projectPath string, excludes []string, levels int) []string { goList, err := golang.GoList() if err != nil { - log.Fatal().Err(err).Msg("Error getting go list") + log.Error().Err(err).Msg("Error getting go list") + os.Exit(ErrorExitCode) } gitDiff, err := git.Diff(baseRef) if err != nil { - log.Fatal().Err(err).Msg("Error getting the git diff") + log.Error().Err(err).Msg("Error getting the git diff") + os.Exit(ErrorExitCode) } gitModDiff, err := git.ModDiff(baseRef, projectPath) if err != nil { - log.Fatal().Err(err).Msg("Error getting the git mod diff") + log.Error().Err(err).Msg("Error getting the git mod diff") + os.Exit(ErrorExitCode) } packages, err := golang.ParsePackages(goList.Stdout) if err != nil { - log.Fatal().Err(err).Msg("Error parsing packages") + log.Error().Err(err).Msg("Error parsing packages") + os.Exit(ErrorExitCode) } fileMap := golang.GetGoFileMap(packages, true) @@ -113,12 +121,14 @@ func findAffectedPackages(baseRef, projectPath string, excludes []string, levels var changedPackages []string changedPackages, err = git.GetChangedGoPackagesFromDiff(gitDiff.Stdout, projectPath, excludes, fileMap) if err != nil { - log.Fatal().Err(err).Msg("Error getting changed packages") + log.Error().Err(err).Msg("Error getting changed packages") + os.Exit(ErrorExitCode) } changedModPackages, err := git.GetGoModChangesFromDiff(gitModDiff.Stdout) if err != nil { - log.Fatal().Err(err).Msg("Error getting go.mod changes") + log.Error().Err(err).Msg("Error getting go.mod changes") + os.Exit(ErrorExitCode) } depMap := golang.GetGoDepMap(packages) @@ -156,7 +166,8 @@ func outputResults(packages []string, jsonOutput bool) { if jsonOutput { data, err := json.Marshal(packages) if err != nil { - log.Fatal().Err(err).Msg("Error marshaling test packages to JSON") + log.Error().Err(err).Msg("Error marshaling test packages to JSON") + os.Exit(ErrorExitCode) } log.Debug().Str("output", string(data)).Msg("JSON") } else { diff --git a/tools/flakeguard/cmd/generate_report.go b/tools/flakeguard/cmd/generate_report.go index 9242fd4eb..d8ceebd2b 100644 --- a/tools/flakeguard/cmd/generate_report.go +++ b/tools/flakeguard/cmd/generate_report.go @@ -34,7 +34,7 @@ type SummaryData struct { var GenerateReportCmd = &cobra.Command{ Use: "generate-report", Short: "Generate reports from an aggregated test results", - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { fs := reports.OSFileSystem{} // Get flag values @@ -50,7 +50,8 @@ var GenerateReportCmd = &cobra.Command{ // Get the GitHub token from environment variable githubToken := os.Getenv("GITHUB_TOKEN") if githubToken == "" { - return fmt.Errorf("GITHUB_TOKEN environment variable is not set") + log.Error().Msg("GITHUB_TOKEN environment variable is not set") + os.Exit(ErrorExitCode) } // Load the aggregated report @@ -62,32 +63,40 @@ var GenerateReportCmd = &cobra.Command{ reportFile, err := os.Open(aggregatedResultsPath) if err != nil { s.Stop() - return fmt.Errorf("error opening aggregated test report: %w", err) + fmt.Println() + log.Error().Err(err).Msg("Error opening aggregated test report") + os.Exit(ErrorExitCode) } defer reportFile.Close() if err := json.NewDecoder(reportFile).Decode(aggregatedReport); err != nil { s.Stop() - return fmt.Errorf("error decoding aggregated test report: %w", err) + fmt.Println() + log.Error().Err(err).Msg("Error decoding aggregated test report") + os.Exit(ErrorExitCode) } s.Stop() + fmt.Println() log.Info().Msg("Successfully loaded aggregated test report") // Load the summary data to check for failed tests var summaryData SummaryData if summaryPath == "" { - return fmt.Errorf("--summary-path is required") + log.Error().Msg("Summary path is required") + os.Exit(ErrorExitCode) } summaryFile, err := os.Open(summaryPath) if err != nil { - return fmt.Errorf("error opening summary JSON file: %w", err) + log.Error().Err(err).Msg("Error opening summary JSON file") + os.Exit(ErrorExitCode) } defer summaryFile.Close() if err := json.NewDecoder(summaryFile).Decode(&summaryData); err != nil { - return fmt.Errorf("error decoding summary JSON file: %w", err) + log.Error().Err(err).Msg("Error decoding summary JSON file") + os.Exit(ErrorExitCode) } // Check if there are failed tests @@ -98,7 +107,8 @@ var GenerateReportCmd = &cobra.Command{ // Fetch artifact link from GitHub API artifactLink, err = fetchArtifactLink(githubToken, githubRepo, githubRunID, artifactName) if err != nil { - return fmt.Errorf("error fetching artifact link: %w", err) + log.Error().Err(err).Msg("Error fetching artifact link") + os.Exit(ErrorExitCode) } } else { // No failed tests, set artifactLink to empty string @@ -108,7 +118,8 @@ var GenerateReportCmd = &cobra.Command{ // Create output directory if it doesn't exist if err := fs.MkdirAll(outputDir, 0755); err != nil { - return fmt.Errorf("error creating output directory: %w", err) + log.Error().Err(err).Msg("Error creating output directory") + os.Exit(ErrorExitCode) } // Generate GitHub summary markdown @@ -119,9 +130,12 @@ var GenerateReportCmd = &cobra.Command{ err = generateGitHubSummaryMarkdown(aggregatedReport, filepath.Join(outputDir, "all-test"), artifactLink, artifactName) if err != nil { s.Stop() - return fmt.Errorf("error generating GitHub summary markdown: %w", err) + fmt.Println() + log.Error().Err(err).Msg("Error generating GitHub summary markdown") + os.Exit(ErrorExitCode) } s.Stop() + fmt.Println() log.Info().Msg("GitHub summary markdown generated successfully") if generatePRComment { @@ -147,7 +161,8 @@ var GenerateReportCmd = &cobra.Command{ missingFlags = append(missingFlags, "--action-run-id") } if len(missingFlags) > 0 { - return fmt.Errorf("the following flags are required when --generate-pr-comment is set: %s", strings.Join(missingFlags, ", ")) + log.Error().Strs("missing flags", missingFlags).Msg("Not all required flags are provided for --generate-pr-comment") + os.Exit(ErrorExitCode) } // Generate PR comment markdown @@ -169,15 +184,16 @@ var GenerateReportCmd = &cobra.Command{ ) if err != nil { s.Stop() - return fmt.Errorf("error generating PR comment markdown: %w", err) + fmt.Println() + log.Error().Err(err).Msg("Error generating PR comment markdown") + os.Exit(ErrorExitCode) } s.Stop() + fmt.Println() log.Info().Msg("PR comment markdown generated successfully") } log.Info().Str("output", outputDir).Msg("Reports generated successfully") - - return nil }, } @@ -197,16 +213,20 @@ func init() { GenerateReportCmd.Flags().String("failed-tests-artifact-name", "failed-test-results-with-logs.json", "The name of the failed tests artifact (default 'failed-test-results-with-logs.json')") if err := GenerateReportCmd.MarkFlagRequired("aggregated-results-path"); err != nil { - log.Fatal().Err(err).Msg("Error marking flag as required") + log.Error().Err(err).Msg("Error marking flag as required") + os.Exit(ErrorExitCode) } if err := GenerateReportCmd.MarkFlagRequired("summary-path"); err != nil { - log.Fatal().Err(err).Msg("Error marking flag as required") + log.Error().Err(err).Msg("Error marking flag as required") + os.Exit(ErrorExitCode) } if err := GenerateReportCmd.MarkFlagRequired("github-repository"); err != nil { - log.Fatal().Err(err).Msg("Error marking flag as required") + log.Error().Err(err).Msg("Error marking flag as required") + os.Exit(ErrorExitCode) } if err := GenerateReportCmd.MarkFlagRequired("github-run-id"); err != nil { - log.Fatal().Err(err).Msg("Error marking flag as required") + log.Error().Err(err).Msg("Error marking flag as required") + os.Exit(ErrorExitCode) } } diff --git a/tools/flakeguard/cmd/run.go b/tools/flakeguard/cmd/run.go index 0bdf9756b..3332bbe32 100644 --- a/tools/flakeguard/cmd/run.go +++ b/tools/flakeguard/cmd/run.go @@ -13,6 +13,13 @@ import ( "github.com/spf13/cobra" ) +const ( + // FlakyTestsExitCode indicates that Flakeguard ran correctly and was able to identify flaky tests + FlakyTestsExitCode = 1 + // ErrorExitCode indicates that Flakeguard ran into an error and was not able to complete operation + ErrorExitCode = 2 +) + var RunTestsCmd = &cobra.Command{ Use: "run", Short: "Run tests to check if they are flaky", @@ -35,19 +42,22 @@ var RunTestsCmd = &cobra.Command{ // Check if project dependencies are correctly set up if err := checkDependencies(projectPath); err != nil { - log.Fatal().Err(err).Msg("Error checking project dependencies") + log.Error().Err(err).Msg("Error checking project dependencies") + os.Exit(ErrorExitCode) } // Determine test packages var testPackages []string if testPackagesJson != "" { if err := json.Unmarshal([]byte(testPackagesJson), &testPackages); err != nil { - log.Fatal().Err(err).Msg("Error decoding test packages JSON") + log.Error().Err(err).Msg("Error decoding test packages JSON") + os.Exit(ErrorExitCode) } } else if len(testPackagesArg) > 0 { testPackages = testPackagesArg } else { - log.Fatal().Msg("Error: must specify either --test-packages-json or --test-packages") + log.Error().Msg("Error: must specify either --test-packages-json or --test-packages") + os.Exit(ErrorExitCode) } // Initialize the runner @@ -69,17 +79,20 @@ var RunTestsCmd = &cobra.Command{ // Run the tests testReport, err := testRunner.RunTests() if err != nil { - log.Fatal().Err(err).Msg("Error running tests") + log.Error().Err(err).Msg("Error running tests") + os.Exit(ErrorExitCode) } // Save the test results in JSON format if outputPath != "" && len(testReport.Results) > 0 { jsonData, err := json.MarshalIndent(testReport, "", " ") if err != nil { - log.Fatal().Err(err).Msg("Error marshaling test results to JSON") + log.Error().Err(err).Msg("Error marshaling test results to JSON") + os.Exit(ErrorExitCode) } if err := os.WriteFile(outputPath, jsonData, 0600); err != nil { - log.Fatal().Err(err).Msg("Error writing test results to file") + log.Error().Err(err).Msg("Error writing test results to file") + os.Exit(ErrorExitCode) } log.Info().Str("path", outputPath).Msg("Test results saved") } @@ -99,7 +112,7 @@ var RunTestsCmd = &cobra.Command{ fmt.Printf("\nFlakeguard Summary\n") reports.RenderResults(os.Stdout, flakyTests, maxPassRatio, false, false) // Exit with error code if there are flaky tests - os.Exit(1) + os.Exit(FlakyTestsExitCode) } }, } diff --git a/tools/flakeguard/reports/presentation.go b/tools/flakeguard/reports/presentation.go index 0a0babfa9..831305629 100644 --- a/tools/flakeguard/reports/presentation.go +++ b/tools/flakeguard/reports/presentation.go @@ -176,6 +176,13 @@ func buildSettingsTable(testReport *TestReport, maxPassRatio float64) [][]string return rows } +func RenderError( + w io.Writer, + err error, +) { + fmt.Fprintln(w, ":x: Error Running Flakeguard :x:") +} + // RenderResults renders the test results into a console or markdown format. // If in markdown mode, the table results can also be made collapsible. func RenderResults( diff --git a/tools/flakeguard/runner/runner_test.go b/tools/flakeguard/runner/runner_test.go index 84fc5612c..daa011090 100644 --- a/tools/flakeguard/runner/runner_test.go +++ b/tools/flakeguard/runner/runner_test.go @@ -497,7 +497,7 @@ func TestAttributeRaceToTest(t *testing.T) { } // TODO: Running the failing test here fools tools like gotestfmt into thinking we actually ran a failing test -// as the output gets piped out to the console. This is a minor annoyance we should fix. +// as the output gets piped out to the console. This a confusing annoyance that I'd like to fix, but it's not crucial.p\\\\\\\\\\- func TestFailedOutputs(t *testing.T) { t.Parallel()