From f5c67fa8a143ed5c233bce34d7bfd00e7973eb43 Mon Sep 17 00:00:00 2001 From: Suryansh Garg Date: Fri, 28 Aug 2026 00:14:09 +0000 Subject: [PATCH 1/3] fix(errorutil): ignore pre-existing duplicate codes Signed-off-by: Suryansh Garg --- cmd/errorutil/internal/coder/commands.go | 126 ++++++++- cmd/errorutil/internal/coder/commands_test.go | 250 ++++++++++++++++-- 2 files changed, 337 insertions(+), 39 deletions(-) diff --git a/cmd/errorutil/internal/coder/commands.go b/cmd/errorutil/internal/coder/commands.go index 4f6ba609..13417b0c 100644 --- a/cmd/errorutil/internal/coder/commands.go +++ b/cmd/errorutil/internal/coder/commands.go @@ -6,6 +6,8 @@ import ( "io" "os" "path/filepath" + "sort" + "strconv" "github.com/meshery/meshkit/cmd/errorutil/internal/component" @@ -176,7 +178,7 @@ This tool produces three files: - errorutil_errors_export.json: export of errors which can be used to create the error code reference on the Meshery website Typically, the 'analyze' command of the tool is used by the developer to verify errors, i.e. that there are no duplicate names or details. -A CI workflow is used to replace the placeholder code strings with integer code, and export errors. Using this export, the workflow updates +A CI workflow is used to replace the placeholder code strings with integer code, and export errors. Using this export, the workflow updates the error code reference documentation in the Meshery repository. Meshery components and this tool: @@ -191,37 +193,99 @@ Meshery components and this tool: "next_error_code": 1014 } - next_error_code is the value used by the tool to replace the error code placeholder string with the next integer. -- The tool updates next_error_code. +- The tool updates next_error_code. `) }, } } -func ValidateNewErrors(baseline, current mesherr.InfoAll, out io.Writer) error { - baselineErrors := make(map[string]bool) +func ValidateNewErrors(baseline, current mesherr.InfoAll, baselineNext, currentNext int, out io.Writer) error { + baselineCodes := make(map[string]bool) + baselineCounts := make(map[string]int) + baselineDup := make(map[string]bool) for _, entry := range baseline.Entries { - baselineErrors[entry.Name] = true + if entry.CodeIsInt { + baselineCodes[entry.Code] = true + baselineCounts[entry.Code]++ + if baselineCounts[entry.Code] > 1 { + baselineDup[entry.Code] = true + } + } } hasError := false + + // R1: Duplicate codes in current state (ignoring inherited baseline duplicates) + seen := make(map[string][]string) + for _, entry := range current.Entries { + if entry.CodeIsInt { + seen[entry.Code] = append(seen[entry.Code], entry.Name) + } + } + + var duplicateCodes []string + for code, names := range seen { + if len(names) > 1 && !baselineDup[code] { + duplicateCodes = append(duplicateCodes, code) + } + } + sort.Strings(duplicateCodes) + + for _, code := range duplicateCodes { + fmt.Fprintf(out, "Error: duplicate error code \"%s\" used by %v\n", code, seen[code]) + hasError = true + } + + // R2: Counter regression + if baselineNext > 0 && currentNext > 0 && currentNext < baselineNext { + fmt.Fprintf(out, "Error: Allocation counter regression detected (baseline: %d, current: %d)\n", baselineNext, currentNext) + hasError = true + } + for _, entry := range current.Entries { - if _, exists := baselineErrors[entry.Name]; !exists && entry.CodeIsInt { - fmt.Fprintf(out, "Error: New error %s uses a manually assigned code \"%s\"; use \"replace_me\" and let errorutil allocate the code\n", entry.Name, entry.Code) + if !entry.CodeIsInt { + continue + } + + // R3: Identify newly introduced integer codes BY CODE, NOT NAME + if baselineCodes[entry.Code] { + continue // Legitimate rename or no-op + } + + // R4: Validate genuinely new integer codes + validLocalAllocation := false + if baselineNext > 0 && currentNext > 0 { + codeInt, err := strconv.Atoi(entry.Code) + if err == nil { + if codeInt >= baselineNext && codeInt < currentNext { + validLocalAllocation = true + } + } + } + + if !validLocalAllocation { + if baselineNext > 0 && currentNext > 0 { + fmt.Fprintf(out, "Error: New error %s uses manually assigned code \"%s\"; must use \"replace_me\" or be in valid local allocation range [%d, %d)\n", entry.Name, entry.Code, baselineNext, currentNext) + } else { + fmt.Fprintf(out, "Error: New error %s uses manually assigned code \"%s\"; use \"replace_me\" and let errorutil allocate the code\n", entry.Name, entry.Code) + } hasError = true } } if hasError { - return fmt.Errorf("newly introduced error codes must use placeholder 'replace_me'") + return fmt.Errorf("error validation failed") } return nil } func commandCheck() *cobra.Command { - return &cobra.Command{ + var baselineSummaryPath, currentSummaryPath string + + cmd := &cobra.Command{ Use: "check [baseline JSON] [current JSON]", Short: "Checks that newly introduced error codes use a placeholder (e.g. replace_me)", - Long: `check compares the errors from the two provided JSON files (baseline and current) and ensures that any newly introduced error code does not use a manually assigned integer code, but rather a placeholder string.`, + Long: `check compares the errors from the two provided JSON files (baseline and current) and ensures that any newly introduced error code does not use a manually assigned integer code, but rather a placeholder string. It validates local allocations if summary files are provided.`, Args: cobra.ExactArgs(2), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { @@ -242,9 +306,49 @@ func commandCheck() *cobra.Command { return err } - return ValidateNewErrors(baseline, current, cmd.OutOrStdout()) + baselineNext := -1 + currentNext := -1 + + if baselineSummaryPath != "" && currentSummaryPath != "" { + type analysisSummary struct { + NextCode int `json:"next_code"` + } + + bSumBytes, err := os.ReadFile(baselineSummaryPath) + if err != nil { + return fmt.Errorf("failed to read baseline summary: %w", err) + } + var bSum analysisSummary + if err := json.Unmarshal(bSumBytes, &bSum); err != nil { + return fmt.Errorf("failed to parse baseline summary: %w", err) + } + if bSum.NextCode <= 0 { + return fmt.Errorf("%s: next_code missing or zero — expected an errorutil_analyze_summary.json", baselineSummaryPath) + } + baselineNext = bSum.NextCode + + cSumBytes, err := os.ReadFile(currentSummaryPath) + if err != nil { + return fmt.Errorf("failed to read current summary: %w", err) + } + var cSum analysisSummary + if err := json.Unmarshal(cSumBytes, &cSum); err != nil { + return fmt.Errorf("failed to parse current summary: %w", err) + } + if cSum.NextCode <= 0 { + return fmt.Errorf("%s: next_code missing or zero — expected an errorutil_analyze_summary.json", currentSummaryPath) + } + currentNext = cSum.NextCode + } else if baselineSummaryPath != "" || currentSummaryPath != "" { + return fmt.Errorf("both --baseline-summary and --current-summary must be provided if one is provided") + } + + return ValidateNewErrors(baseline, current, baselineNext, currentNext, cmd.OutOrStdout()) }, } + cmd.Flags().StringVar(&baselineSummaryPath, "baseline-summary", "", "path to baseline errorutil_analyze_summary.json") + cmd.Flags().StringVar(¤tSummaryPath, "current-summary", "", "path to current errorutil_analyze_summary.json") + return cmd } func RootCommand() *cobra.Command { diff --git a/cmd/errorutil/internal/coder/commands_test.go b/cmd/errorutil/internal/coder/commands_test.go index 9801fa23..99feb1db 100644 --- a/cmd/errorutil/internal/coder/commands_test.go +++ b/cmd/errorutil/internal/coder/commands_test.go @@ -9,11 +9,14 @@ import ( func TestCheckLogic(t *testing.T) { tests := []struct { - name string - baseline mesherr.InfoAll - current mesherr.InfoAll - wantErrors bool + name string + baseline mesherr.InfoAll + current mesherr.InfoAll + baselineNext int + currentNext int + wantErrors bool }{ + // 1. new + replace_me -> PASS { name: "New placeholder passes", baseline: mesherr.InfoAll{ @@ -27,10 +30,13 @@ func TestCheckLogic(t *testing.T) { {Name: "ErrNew", Code: "replace_me", CodeIsInt: false}, }, }, - wantErrors: false, + baselineNext: -1, + currentNext: -1, + wantErrors: false, }, + // 2. single legitimate local allocation -> PASS { - name: "New unique manual code fails", + name: "Single legitimate local allocation passes", baseline: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, @@ -42,10 +48,13 @@ func TestCheckLogic(t *testing.T) { {Name: "ErrNew", Code: "1001", CodeIsInt: true}, }, }, - wantErrors: true, + baselineNext: 1001, + currentNext: 1002, + wantErrors: false, }, + // 3. multiple legitimate local allocations -> PASS { - name: "New manual code reusing existing code fails", + name: "Multiple legitimate local allocations pass", baseline: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, @@ -54,11 +63,105 @@ func TestCheckLogic(t *testing.T) { current: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, - {Name: "ErrNew", Code: "1000", CodeIsInt: true}, // Old name still exists, reuse fails + {Name: "ErrNew1", Code: "1001", CodeIsInt: true}, + {Name: "ErrNew2", Code: "1002", CodeIsInt: true}, }, }, - wantErrors: true, + baselineNext: 1001, + currentNext: 1003, + wantErrors: false, }, + // 4. manually hardcoded integer -> FAIL + { + name: "Manually hardcoded integer without metadata fails", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + {Name: "ErrNew", Code: "1001", CodeIsInt: true}, + }, + }, + baselineNext: -1, + currentNext: -1, + wantErrors: true, + }, + // 5. vanity/out-of-range integer -> FAIL + { + name: "Vanity or out-of-range integer fails", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + {Name: "ErrNew", Code: "2000", CodeIsInt: true}, + }, + }, + baselineNext: 1001, + currentNext: 1002, + wantErrors: true, // Code 2000 is not in [1001, 1002) + }, + // 6. counter regression -> FAIL + { + name: "Counter regression fails", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + }, + }, + baselineNext: 1001, + currentNext: 999, // Regression! + wantErrors: true, + }, + // 7. duplicate code in current state (NOT in baseline) -> FAIL + { + name: "Duplicate code introduced by PR fails", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, + {Name: "ErrNew", Code: "1000", CodeIsInt: true}, // Duplicate + }, + }, + baselineNext: -1, + currentNext: -1, + wantErrors: true, + }, + // 7.5. duplicate code inherited from baseline -> PASS + { + name: "Existing duplicate inherited from baseline passes", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrA", Code: "1000", CodeIsInt: true}, + {Name: "ErrB", Code: "1000", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrA", Code: "1000", CodeIsInt: true}, + {Name: "ErrB", Code: "1000", CodeIsInt: true}, + }, + }, + baselineNext: -1, + currentNext: -1, + wantErrors: false, + }, + // 8. no change -> PASS { name: "Unchanged existing code passes", baseline: mesherr.InfoAll{ @@ -71,10 +174,13 @@ func TestCheckLogic(t *testing.T) { {Name: "ErrOld", Code: "1000", CodeIsInt: true}, }, }, - wantErrors: false, + baselineNext: -1, + currentNext: -1, + wantErrors: false, }, + // 9. pure rename preserving existing code -> PASS { - name: "Rename existing error fails", + name: "Rename existing error preserving code passes", baseline: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, @@ -82,27 +188,52 @@ func TestCheckLogic(t *testing.T) { }, current: mesherr.InfoAll{ Entries: []mesherr.Info{ - {Name: "ErrRenamed", Code: "1000", CodeIsInt: true}, // Old name removed, but new name has manual code -> fails + {Name: "ErrRenamed", Code: "1000", CodeIsInt: true}, }, }, - wantErrors: true, + baselineNext: -1, + currentNext: -1, + wantErrors: false, }, + // 10. rename + one genuinely new allocation -> PASS { - name: "Move existing error passes", + name: "Rename + genuinely new allocation passes", baseline: mesherr.InfoAll{ Entries: []mesherr.Info{ - {Name: "ErrOld", Code: "1000", CodeIsInt: true, Path: "pkg/old/error.go"}, + {Name: "ErrOld", Code: "1000", CodeIsInt: true}, }, }, current: mesherr.InfoAll{ Entries: []mesherr.Info{ - {Name: "ErrOld", Code: "1000", CodeIsInt: true, Path: "pkg/new/error.go"}, + {Name: "ErrRenamed", Code: "1000", CodeIsInt: true}, + {Name: "ErrNew", Code: "1001", CodeIsInt: true}, }, }, - wantErrors: false, + baselineNext: 1001, + currentNext: 1002, + wantErrors: false, }, + // 10.5. delete existing error + reuse its code for a new error -> PASS { - name: "Multiple additions", + name: "Delete old error and reuse its code passes (indistinguishable from rename)", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOldCode", Code: "1463", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrNewCode", Code: "1463", CodeIsInt: true}, + }, + }, + baselineNext: -1, + currentNext: -1, + wantErrors: false, + }, + // 11. allocate-then-delete -> PASS + // 12. over-advanced counter -> PASS with explanatory comment + { + name: "Allocate-then-delete / Over-advanced counter passes", baseline: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, @@ -111,14 +242,21 @@ func TestCheckLogic(t *testing.T) { current: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, - {Name: "ErrNew1", Code: "replace_me", CodeIsInt: false}, - {Name: "ErrNew2", Code: "replace_me", CodeIsInt: false}, + {Name: "ErrNew", Code: "1002", CodeIsInt: true}, }, }, - wantErrors: false, + baselineNext: 1001, + currentNext: 1005, + // Explanation: The counter has advanced from 1001 to 1005. + // Code 1002 falls within the [1001, 1005) range. + // This represents a legitimate workflow where an error code (e.g. 1001) + // was allocated, but later deleted from the PR. We shouldn't enforce + // strict equality between the count of new errors and currentNext - baselineNext. + wantErrors: false, }, + // 13. unrelated integer/string constant -> unaffected { - name: "Mixed additions with one invalid", + name: "Unrelated non-int or placeholder code passes", baseline: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, @@ -127,20 +265,76 @@ func TestCheckLogic(t *testing.T) { current: mesherr.InfoAll{ Entries: []mesherr.Info{ {Name: "ErrOld", Code: "1000", CodeIsInt: true}, - {Name: "ErrNew1", Code: "replace_me", CodeIsInt: false}, - {Name: "ErrNew2", Code: "1001", CodeIsInt: true}, + {Name: "ErrString", Code: "some_string_val", CodeIsInt: false}, + }, + }, + baselineNext: 1001, + currentNext: 1001, + wantErrors: false, + }, + // 14. existing code moved between files/packages -> PASS + { + name: "Move existing error passes", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true, Path: "pkg/old/error.go"}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrOld", Code: "1000", CodeIsInt: true, Path: "pkg/new/error.go"}, + }, + }, + baselineNext: -1, + currentNext: -1, + wantErrors: false, + }, + // 15. Two new errors both left as replace_me (simulating the 'place_' normalization collision) + { + name: "Multiple new placeholders simulating normalization collision passes", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrExistingCode", Code: "1463", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrExistingCode", Code: "1463", CodeIsInt: true}, + {Name: "ErrNewOneCode", Code: "place_", CodeIsInt: false}, + {Name: "ErrNewTwoCode", Code: "place_", CodeIsInt: false}, + }, + }, + baselineNext: 1464, + currentNext: 1464, + wantErrors: false, + }, + // 16. Two new errors both hardcoded to the same existing integer + { + name: "Two new errors hardcoded to the same integer fail", + baseline: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrExistingCode", Code: "1463", CodeIsInt: true}, + }, + }, + current: mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrExistingCode", Code: "1463", CodeIsInt: true}, + {Name: "ErrNewOneCode", Code: "1463", CodeIsInt: true}, + {Name: "ErrNewTwoCode", Code: "1463", CodeIsInt: true}, }, }, - wantErrors: true, + baselineNext: -1, + currentNext: -1, + wantErrors: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var buf bytes.Buffer - err := ValidateNewErrors(tt.baseline, tt.current, &buf) + err := ValidateNewErrors(tt.baseline, tt.current, tt.baselineNext, tt.currentNext, &buf) if (err != nil) != tt.wantErrors { - t.Errorf("ValidateNewErrors() error = %v, wantErrors %v", err, tt.wantErrors) + t.Errorf("ValidateNewErrors() error = %v, wantErrors %v. Output: %s", err, tt.wantErrors, buf.String()) } }) } From a3b52929981006b0c8506d5546c12eab203d23ca Mon Sep 17 00:00:00 2001 From: Suryansh Garg Date: Fri, 28 Aug 2026 00:56:59 +0000 Subject: [PATCH 2/3] test(errorutil): cover check command Signed-off-by: Suryansh Garg --- cmd/errorutil/internal/coder/commands_test.go | 282 ++++++++++++++++++ 1 file changed, 282 insertions(+) diff --git a/cmd/errorutil/internal/coder/commands_test.go b/cmd/errorutil/internal/coder/commands_test.go index 99feb1db..d232d43c 100644 --- a/cmd/errorutil/internal/coder/commands_test.go +++ b/cmd/errorutil/internal/coder/commands_test.go @@ -2,6 +2,10 @@ package coder import ( "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" "testing" mesherr "github.com/meshery/meshkit/cmd/errorutil/internal/error" @@ -339,3 +343,281 @@ func TestCheckLogic(t *testing.T) { }) } } + +func TestCommandCheck(t *testing.T) { + // Helper to write JSON files + writeJSON := func(t *testing.T, dir, filename string, v interface{}) string { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, filename) + if err := os.WriteFile(path, b, 0644); err != nil { + t.Fatal(err) + } + return path + } + + // Helpers for standard payloads + makeErrors := func(code string, codeIsInt bool) mesherr.InfoAll { + return mesherr.InfoAll{ + Entries: []mesherr.Info{ + {Name: "ErrTest", Code: code, CodeIsInt: codeIsInt}, + }, + } + } + + type analysisSummary struct { + NextCode int `json:"next_code"` + } + + tests := []struct { + name string + setup func(t *testing.T, dir string) (args []string) + wantError bool + wantErrorMatch string + }{ + // 1. Both summary flags with valid local allocation + { + name: "Both summary flags with valid local allocation", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + bSum := writeJSON(t, dir, "b_sum.json", analysisSummary{NextCode: 1001}) + cSum := writeJSON(t, dir, "c_sum.json", analysisSummary{NextCode: 1002}) + + return []string{ + "--baseline-summary", bSum, + "--current-summary", cSum, + bErr, cErr, + } + }, + wantError: false, + }, + // 2. Both summary flags with invalid/manual integer + { + name: "Both summary flags with invalid manual integer", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("2000", true)) // Outside 1001..1002 + bSum := writeJSON(t, dir, "b_sum.json", analysisSummary{NextCode: 1001}) + cSum := writeJSON(t, dir, "c_sum.json", analysisSummary{NextCode: 1002}) + + return []string{ + "--baseline-summary", bSum, + "--current-summary", cSum, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "error validation failed", + }, + // 3. No summary flags (legacy strict behavior) + { + name: "No summary flags", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) // Valid if local, but fails in strict + + return []string{bErr, cErr} + }, + wantError: true, + wantErrorMatch: "error validation failed", + }, + // 4. Only baseline summary supplied + { + name: "Only baseline summary supplied", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + bSum := writeJSON(t, dir, "b_sum.json", analysisSummary{NextCode: 1001}) + + return []string{ + "--baseline-summary", bSum, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "both --baseline-summary and --current-summary must be provided", + }, + // 5. Only current summary supplied + { + name: "Only current summary supplied", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + cSum := writeJSON(t, dir, "c_sum.json", analysisSummary{NextCode: 1002}) + + return []string{ + "--current-summary", cSum, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "both --baseline-summary and --current-summary must be provided", + }, + // 6. Wrong file supplied as summary + { + name: "Wrong file supplied as summary", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + + return []string{ + "--baseline-summary", bErr, // Passing error JSON instead of summary + "--current-summary", cErr, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "next_code", + }, + // 7. Missing baseline summary file + { + name: "Missing baseline summary file", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + cSum := writeJSON(t, dir, "c_sum.json", analysisSummary{NextCode: 1002}) + + return []string{ + "--baseline-summary", filepath.Join(dir, "nonexistent.json"), + "--current-summary", cSum, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "failed to read baseline summary", + }, + // 8. Missing current summary file + { + name: "Missing current summary file", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + bSum := writeJSON(t, dir, "b_sum.json", analysisSummary{NextCode: 1001}) + + return []string{ + "--baseline-summary", bSum, + "--current-summary", filepath.Join(dir, "nonexistent.json"), + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "failed to read current summary", + }, + // 9. Malformed baseline summary JSON + { + name: "Malformed baseline summary JSON", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + cSum := writeJSON(t, dir, "c_sum.json", analysisSummary{NextCode: 1002}) + + bSum := filepath.Join(dir, "b_sum.json") + os.WriteFile(bSum, []byte("not valid json"), 0644) + + return []string{ + "--baseline-summary", bSum, + "--current-summary", cSum, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "failed to parse baseline summary", + }, + // 10. Malformed current summary JSON + { + name: "Malformed current summary JSON", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + bSum := writeJSON(t, dir, "b_sum.json", analysisSummary{NextCode: 1001}) + + cSum := filepath.Join(dir, "c_sum.json") + os.WriteFile(cSum, []byte("not valid json"), 0644) + + return []string{ + "--baseline-summary", bSum, + "--current-summary", cSum, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "failed to parse current summary", + }, + // 11. Summary with missing/zero next_code + { + name: "Summary with missing/zero next_code", + setup: func(t *testing.T, dir string) []string { + bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) + cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) + bSum := writeJSON(t, dir, "b_sum.json", analysisSummary{NextCode: 0}) + cSum := writeJSON(t, dir, "c_sum.json", analysisSummary{NextCode: 1002}) + + return []string{ + "--baseline-summary", bSum, + "--current-summary", cSum, + bErr, cErr, + } + }, + wantError: true, + wantErrorMatch: "next_code", + }, + // 12. Positional argument validation + { + name: "Positional argument validation (no args)", + setup: func(t *testing.T, dir string) []string { + return []string{} + }, + wantError: true, + wantErrorMatch: "accepts 2 arg(s), received 0", + }, + { + name: "Positional argument validation (1 arg)", + setup: func(t *testing.T, dir string) []string { + return []string{"one"} + }, + wantError: true, + wantErrorMatch: "accepts 2 arg(s), received 1", + }, + { + name: "Positional argument validation (3 args)", + setup: func(t *testing.T, dir string) []string { + return []string{"one", "two", "three"} + }, + wantError: true, + wantErrorMatch: "accepts 2 arg(s), received 3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + args := tt.setup(t, dir) + + cmd := commandCheck() + + // We only want to capture out/err, not pollute real stdout + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs(args) + + err := cmd.Execute() + if (err != nil) != tt.wantError { + t.Fatalf("commandCheck().Execute() error = %v, wantError %v", err, tt.wantError) + } + + if tt.wantError && tt.wantErrorMatch != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErrorMatch) + } + if !strings.Contains(err.Error(), tt.wantErrorMatch) { + t.Errorf("error %q does not contain %q", err.Error(), tt.wantErrorMatch) + } + } + }) + } +} From 132b46623fa6e7ccbdbdf358c11048ad6fe2b170 Mon Sep 17 00:00:00 2001 From: Suryansh Garg Date: Fri, 28 Aug 2026 01:03:19 +0000 Subject: [PATCH 3/3] test(errorutil): cover check command Signed-off-by: Suryansh Garg --- cmd/errorutil/internal/coder/commands_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/errorutil/internal/coder/commands_test.go b/cmd/errorutil/internal/coder/commands_test.go index d232d43c..2839ea6c 100644 --- a/cmd/errorutil/internal/coder/commands_test.go +++ b/cmd/errorutil/internal/coder/commands_test.go @@ -514,7 +514,7 @@ func TestCommandCheck(t *testing.T) { bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) cSum := writeJSON(t, dir, "c_sum.json", analysisSummary{NextCode: 1002}) - + bSum := filepath.Join(dir, "b_sum.json") os.WriteFile(bSum, []byte("not valid json"), 0644) @@ -534,7 +534,7 @@ func TestCommandCheck(t *testing.T) { bErr := writeJSON(t, dir, "b_err.json", makeErrors("1000", true)) cErr := writeJSON(t, dir, "c_err.json", makeErrors("1001", true)) bSum := writeJSON(t, dir, "b_sum.json", analysisSummary{NextCode: 1001}) - + cSum := filepath.Join(dir, "c_sum.json") os.WriteFile(cSum, []byte("not valid json"), 0644) @@ -598,7 +598,7 @@ func TestCommandCheck(t *testing.T) { args := tt.setup(t, dir) cmd := commandCheck() - + // We only want to capture out/err, not pollute real stdout var stdout, stderr bytes.Buffer cmd.SetOut(&stdout) @@ -609,7 +609,7 @@ func TestCommandCheck(t *testing.T) { if (err != nil) != tt.wantError { t.Fatalf("commandCheck().Execute() error = %v, wantError %v", err, tt.wantError) } - + if tt.wantError && tt.wantErrorMatch != "" { if err == nil { t.Fatalf("expected error containing %q, got nil", tt.wantErrorMatch)