diff --git a/shortcuts/sheets/batch_op_contract_test.go b/shortcuts/sheets/batch_op_contract_test.go index 88715cfc4..455b4782c 100644 --- a/shortcuts/sheets/batch_op_contract_test.go +++ b/shortcuts/sheets/batch_op_contract_test.go @@ -444,12 +444,7 @@ func TestBatchOp_ErrorEquivalence(t *testing.T) { t, tc.shortcut, append([]string{"--url", testURL, "--dry-run"}, tc.args...), ) - if standaloneErr == nil { - t.Fatalf("standalone Validate accepted bad input — expected error containing %q", tc.wantContains) - } - if !strings.Contains(standaloneErr.Error(), tc.wantContains) { - t.Errorf("standalone error = %q, want substring %q", standaloneErr.Error(), tc.wantContains) - } + requireValidation(t, standaloneErr, tc.wantContains) // Batch path: translate the matching sub-op. The translator wraps // the inner error with "operations[i] (): " — assert the @@ -463,17 +458,12 @@ func TestBatchOp_ErrorEquivalence(t *testing.T) { "input": subInput, } _, batchErr := translateBatchOp(rawOp, testToken, 0) - if batchErr == nil { - t.Fatalf("batch translator accepted bad input — expected error containing %q", tc.wantContains) - } - if !strings.Contains(batchErr.Error(), tc.wantContains) { - t.Errorf("batch error = %q, want substring %q (operations[i] prefix is fine)", batchErr.Error(), tc.wantContains) - } + batchVE := requireValidation(t, batchErr, tc.wantContains) // And the wrap context must include the sub-op index + shortcut // name so error reports stay actionable in multi-op batches. wrapHint := "operations[0] (" + tc.subShortcut + "):" - if !strings.Contains(batchErr.Error(), wrapHint) { - t.Errorf("batch error %q missing context prefix %q", batchErr.Error(), wrapHint) + if !strings.Contains(batchVE.Message, wrapHint) { + t.Errorf("batch error %q missing context prefix %q", batchVE.Message, wrapHint) } }) } @@ -529,12 +519,7 @@ func TestBatchOp_RejectsWrongScalarType(t *testing.T) { } rawOp := map[string]interface{}{"shortcut": tc.subShortcut, "input": subInput} _, err := translateBatchOp(rawOp, testToken, 0) - if err == nil { - t.Fatalf("translateBatchOp accepted wrong-typed field; want error containing %q", tc.wantContains) - } - if !strings.Contains(err.Error(), tc.wantContains) { - t.Errorf("error = %q, want substring %q", err.Error(), tc.wantContains) - } + requireValidation(t, err, tc.wantContains) }) } } @@ -592,12 +577,7 @@ func TestBatchOp_GuardsBeyondCobra(t *testing.T) { } rawOp := map[string]interface{}{"shortcut": tc.subShortcut, "input": subInput} _, err := translateBatchOp(rawOp, testToken, 0) - if err == nil { - t.Fatalf("translateBatchOp accepted bad input; want error containing %q", tc.wantContains) - } - if !strings.Contains(err.Error(), tc.wantContains) { - t.Errorf("error = %q, want substring %q", err.Error(), tc.wantContains) - } + requireValidation(t, err, tc.wantContains) }) } } @@ -728,12 +708,7 @@ func TestBatchOp_RejectsBadSubOpInput(t *testing.T) { "input": subInput, } _, err := translateBatchOp(rawOp, testToken, 0) - if err == nil { - t.Fatalf("translator accepted bad input — expected error containing %q", tc.wantContains) - } - if !strings.Contains(err.Error(), tc.wantContains) { - t.Errorf("error = %q, want substring %q", err.Error(), tc.wantContains) - } + requireValidation(t, err, tc.wantContains) }) } } @@ -794,12 +769,7 @@ func TestBatchOp_SchemaValidatesSubOps(t *testing.T) { "input": subInput, } _, err := translateBatchOp(rawOp, testToken, 0) - if err == nil { - t.Fatalf("translator accepted schema-violating sub-op — expected error containing %q", tc.wantContains) - } - if !strings.Contains(err.Error(), tc.wantContains) { - t.Errorf("error = %q, want substring %q", err.Error(), tc.wantContains) - } + requireValidation(t, err, tc.wantContains) }) } } diff --git a/shortcuts/sheets/csv_put_guard_test.go b/shortcuts/sheets/csv_put_guard_test.go index 29a47815c..583aec823 100644 --- a/shortcuts/sheets/csv_put_guard_test.go +++ b/shortcuts/sheets/csv_put_guard_test.go @@ -4,12 +4,10 @@ package sheets import ( - "errors" "os" "strings" "testing" - "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" _ "github.com/larksuite/cli/internal/vfs/localfileio" "github.com/larksuite/cli/shortcuts/common" @@ -37,18 +35,9 @@ func TestGuardCSVValueIsNotFilePath(t *testing.T) { // Bare value naming an existing file → guarded with a fix-it hint. err := guardCSVValueIsNotFilePath(newCSVGuardRuntime("data.csv")) - if err == nil { - t.Fatal("expected guard error when --csv names an existing file") - } - if !strings.Contains(err.Error(), "existing file") || !strings.Contains(err.Error(), "@data.csv") { - t.Errorf("error should flag the file and suggest @data.csv, got: %v", err) - } - if p, ok := errs.ProblemOf(err); !ok || p.Category != errs.CategoryValidation || p.Subtype != errs.SubtypeInvalidArgument { - t.Errorf("problem = %+v, want validation/invalid_argument", p) - } - var ve *errs.ValidationError - if !errors.As(err, &ve) { - t.Fatalf("guard error = %T, want *errs.ValidationError", err) + ve := requireValidation(t, err, "existing file") + if !strings.Contains(ve.Message, "@data.csv") { + t.Errorf("message should suggest @data.csv, got: %q", ve.Message) } if ve.Param != "--csv" { t.Errorf("param = %q, want --csv", ve.Param) diff --git a/shortcuts/sheets/csv_put_range_alias_test.go b/shortcuts/sheets/csv_put_range_alias_test.go index e4157c22b..114aa1eb7 100644 --- a/shortcuts/sheets/csv_put_range_alias_test.go +++ b/shortcuts/sheets/csv_put_range_alias_test.go @@ -4,7 +4,6 @@ package sheets import ( - "strings" "testing" ) @@ -44,12 +43,7 @@ func TestCsvPutInput_RejectsStartCellAndRangeTogether(t *testing.T) { "range": "A1:H17", }) _, err := csvPutInput(fv, "tok", "sid", "") - if err == nil { - t.Fatal("csvPutInput accepted both start-cell and range; want mutual-exclusion error") - } - if !strings.Contains(err.Error(), "--start-cell and --range are mutually exclusive") { - t.Errorf("error = %q, want it to mention start-cell/range mutual exclusion", err.Error()) - } + requireValidation(t, err, "--start-cell and --range are mutually exclusive") } // With neither --start-cell nor --range explicitly set, csvPutInput rejects the @@ -61,12 +55,7 @@ func TestCsvPutInput_RejectsStartCellAndRangeTogether(t *testing.T) { func TestCsvPutInput_RequiresStartCellOrRange(t *testing.T) { fv := newMapFlagViewForCommand("+csv-put", map[string]interface{}{"csv": "a,b"}) _, err := csvPutInput(fv, "tok", "sid", "") - if err == nil { - t.Fatal("csvPutInput accepted missing start-cell/range; want a required-flag error") - } - if !strings.Contains(err.Error(), "--start-cell or --range is required") { - t.Errorf("error = %q, want it to mention '--start-cell or --range is required'", err.Error()) - } + requireValidation(t, err, "--start-cell or --range is required") } // csvPutWriteRangeFromInput surfaces the real paste footprint so agents can see diff --git a/shortcuts/sheets/execute_paths_test.go b/shortcuts/sheets/execute_paths_test.go index f8193bb27..2147f7bec 100644 --- a/shortcuts/sheets/execute_paths_test.go +++ b/shortcuts/sheets/execute_paths_test.go @@ -47,19 +47,16 @@ func TestExecute_WorkbookInfo_ToolError(t *testing.T) { "data": map[string]interface{}{}, }, } - stdout, stderr, err := func() (string, string, error) { + _, _, err := func() (string, string, error) { parent, stdout, stderr, reg := newTestRig(t, WorkbookInfo) reg.Register(stub) parent.SetArgs([]string{"+workbook-info", "--url", testURL}) err := parent.Execute() return stdout.String(), stderr.String(), err }() - if err == nil { - t.Fatalf("expected non-zero code to surface as error; stdout=%s stderr=%s", stdout, stderr) - } - combined := stdout + stderr + err.Error() - if !strings.Contains(combined, "1310201") && !strings.Contains(combined, "not found") { - t.Errorf("expected error code in envelope; got=%s|%s|%v", stdout, stderr, err) + p := requireProblem(t, err, errs.CategoryAPI, errs.SubtypeServerError, "") + if !strings.Contains(p.Message, "1310201") && !strings.Contains(p.Message, "not found") { + t.Errorf("expected error code or message in problem; got message=%q", p.Message) } } @@ -113,18 +110,9 @@ func TestExecute_WikiURLWrongObjType(t *testing.T) { }, }, } - out, err := runShortcutWithStubs(t, WorkbookInfo, + _, err := runShortcutWithStubs(t, WorkbookInfo, []string{"--url", "https://example.feishu.cn/wiki/wikTestNODE"}, getNode) - if err == nil { - t.Fatalf("want error for non-sheet wiki node; out=%s", out) - } - if !strings.Contains(err.Error(), "obj_type") { - t.Fatalf("error = %v, want mention of obj_type", err) - } - var ve *errs.ValidationError - if !errors.As(err, &ve) { - t.Fatalf("wrong-obj_type error = %T, want *errs.ValidationError", err) - } + requireValidation(t, err, "obj_type") } // TestExecute_WikiURLIncompleteNode treats an incomplete get_node response diff --git a/shortcuts/sheets/flag_schema.go b/shortcuts/sheets/flag_schema.go index 4fce37408..ccdbc7b08 100644 --- a/shortcuts/sheets/flag_schema.go +++ b/shortcuts/sheets/flag_schema.go @@ -9,6 +9,8 @@ import ( "fmt" "sort" "sync" + + "github.com/larksuite/cli/errs" ) // ─── --print-schema runtime introspection ───────────────────────────── @@ -91,7 +93,7 @@ func printFlagSchemaFor(command string) func(flagName string) ([]byte, error) { } entry, ok := idx.Flags[command] if !ok || len(entry) == 0 { - return nil, fmt.Errorf("no JSON Schema registered for %s", command) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "no JSON Schema registered for %s", command) } if flagName == "" { flags := make([]string, 0, len(entry)) @@ -112,7 +114,9 @@ func printFlagSchemaFor(command string) func(flagName string) ([]byte, error) { flags = append(flags, f) } sort.Strings(flags) - return nil, fmt.Errorf("no JSON Schema registered for %s --%s; available: %v", command, flagName, flags) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "no JSON Schema registered for %s --%s; available: %v", command, flagName, flags). + WithParam("--flag-name") } // Reformat for readability — schema files store compact JSON. var pretty interface{} diff --git a/shortcuts/sheets/flag_schema_test.go b/shortcuts/sheets/flag_schema_test.go index 0310e0374..433320322 100644 --- a/shortcuts/sheets/flag_schema_test.go +++ b/shortcuts/sheets/flag_schema_test.go @@ -84,12 +84,12 @@ func TestPrintFlagSchema_NamedFlagReturnsSchemaSubtree(t *testing.T) { func TestPrintFlagSchema_UnknownFlagListsAvailable(t *testing.T) { t.Parallel() _, err := printFlagSchemaFor("+chart-create")("does-not-exist") - if err == nil { - t.Fatal("expected error for unknown flag, got nil") + ve := requireValidation(t, err, "+chart-create") + if !strings.Contains(ve.Message, "properties") { + t.Errorf("message should list available flags; got %q", ve.Message) } - msg := err.Error() - if !strings.Contains(msg, "+chart-create") || !strings.Contains(msg, "properties") { - t.Errorf("error should mention shortcut + available flags; got %q", msg) + if ve.Param != "--flag-name" { + t.Errorf("param = %q, want --flag-name", ve.Param) } } diff --git a/shortcuts/sheets/flag_schema_validate_test.go b/shortcuts/sheets/flag_schema_validate_test.go index 79e38991a..388b949e1 100644 --- a/shortcuts/sheets/flag_schema_validate_test.go +++ b/shortcuts/sheets/flag_schema_validate_test.go @@ -478,11 +478,9 @@ func TestValidateInputAgainstSchema_RealSchema(t *testing.T) { }, } err := validateInputAgainstSchema(fv, bad) - if err == nil { - t.Fatal("expected enum violation, got nil") - } - if !strings.Contains(err.Error(), "summarize_by") || !strings.Contains(err.Error(), "not in enum") { - t.Errorf("error = %q, want summarize_by + enum hint", err.Error()) + ve := requireValidation(t, err, "summarize_by") + if !strings.Contains(ve.Message, "not in enum") { + t.Errorf("error = %q, want enum hint", ve.Message) } } @@ -499,11 +497,9 @@ func TestValidateInputAgainstSchema_RealMinItems(t *testing.T) { }, } err := validateInputAgainstSchema(fv, bad) - if err == nil { - t.Fatal("expected minItems violation for empty values, got nil") - } - if !strings.Contains(err.Error(), "values") || !strings.Contains(err.Error(), "minimum is 1") { - t.Errorf("error = %q, want values + minimum-is-1 hint", err.Error()) + ve := requireValidation(t, err, "values") + if !strings.Contains(ve.Message, "minimum is 1") { + t.Errorf("error = %q, want minimum-is-1 hint", ve.Message) } } @@ -520,11 +516,9 @@ func TestValidateInputAgainstSchema_RealMinimum(t *testing.T) { }, } err := validateInputAgainstSchema(fv, bad) - if err == nil { - t.Fatal("expected minimum violation for row:-1, got nil") - } - if !strings.Contains(err.Error(), "row") || !strings.Contains(err.Error(), "below minimum") { - t.Errorf("error = %q, want row + below-minimum hint", err.Error()) + ve := requireValidation(t, err, "row") + if !strings.Contains(ve.Message, "below minimum") { + t.Errorf("error = %q, want below-minimum hint", ve.Message) } } @@ -554,11 +548,9 @@ func TestValidateInputAgainstSchema_RealAdditionalProperties(t *testing.T) { }, } err := validateInputAgainstSchema(fv, bad) - if err == nil { - t.Fatal("expected additionalProperties violation, got nil") - } - if !strings.Contains(err.Error(), "collapse") || !strings.Contains(err.Error(), `expected type "string"`) { - t.Errorf("error = %q, want collapse + string-type hint", err.Error()) + ve := requireValidation(t, err, "collapse") + if !strings.Contains(ve.Message, `expected type "string"`) { + t.Errorf("error = %q, want string-type hint", ve.Message) } } @@ -597,17 +589,14 @@ func TestValidateValueAgainstSchema_PrintSchemaHint(t *testing.T) { t.Parallel() fv := mapFlagView{command: "+cells-set"} err := validateValueAgainstSchema(fv, "cells", "not-an-array") - if err == nil { - t.Fatal("expected schema validation error for wrong --cells shape") - } - msg := err.Error() // Underlying shape error is preserved (substring callers still match). - if !strings.Contains(msg, `expected type "array"`) { - t.Errorf("want underlying shape error preserved; got %q", msg) - } + ve := requireValidation(t, err, `expected type "array"`) // And the actionable --print-schema hint is appended with the exact // command + flag, so a copy-paste fetches the schema for this pair. - if !strings.Contains(msg, "lark-cli sheets +cells-set --print-schema --flag-name cells") { - t.Errorf("want --print-schema hint with command+flag; got %q", msg) + if !strings.Contains(ve.Message, "lark-cli sheets +cells-set --print-schema --flag-name cells") { + t.Errorf("want --print-schema hint with command+flag; got %q", ve.Message) + } + if ve.Param != "--cells" { + t.Errorf("param = %q, want --cells", ve.Param) } } diff --git a/shortcuts/sheets/helpers_test.go b/shortcuts/sheets/helpers_test.go index 5c3dbd0dd..e17149a21 100644 --- a/shortcuts/sheets/helpers_test.go +++ b/shortcuts/sheets/helpers_test.go @@ -81,6 +81,53 @@ func runShortcutWithStubs(t *testing.T, sc common.Shortcut, args []string, stubs return stdout.String(), err } +// requireProblem asserts err carries a typed errs.Problem with the given +// category and (optional) subtype, and that its message contains msgContains +// (skip the message check by passing ""). Returns the Problem so callers can +// drill into the typed envelope's category-specific fields (e.g. cast to +// *errs.ValidationError to read .Param / .Params / .Cause). +// +// Replaces the older "strings.Contains(stdout+stderr+err.Error(), ...)" pattern +// across sheets tests: substring on a rendered envelope was brittle (any +// message tweak silently broke it) and didn't verify that the typed contract — +// category / subtype / cause preservation — held. Per coding guideline +// "Error-path tests must assert typed metadata via errs.ProblemOf +// (category / subtype / param) and cause preservation, not message substrings +// alone." +func requireProblem(t *testing.T, err error, wantCategory errs.Category, wantSubtype errs.Subtype, msgContains string) *errs.Problem { + t.Helper() + if err == nil { + t.Fatal("expected error, got nil") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error carrying errs.Problem, got %T: %v", err, err) + } + if p.Category != wantCategory { + t.Errorf("category = %q, want %q (err=%v)", p.Category, wantCategory, err) + } + if wantSubtype != "" && p.Subtype != wantSubtype { + t.Errorf("subtype = %q, want %q (err=%v)", p.Subtype, wantSubtype, err) + } + if msgContains != "" && !strings.Contains(p.Message, msgContains) { + t.Errorf("message = %q, want containing %q", p.Message, msgContains) + } + return p +} + +// requireValidation is shorthand for the most common case: a typed +// CategoryValidation error with SubtypeInvalidArgument. Returns the +// *errs.ValidationError so callers can also assert on .Param / .Params / .Cause. +func requireValidation(t *testing.T, err error, msgContains string) *errs.ValidationError { + t.Helper() + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, msgContains) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + return ve +} + func TestSheetHelpersValidationMetadata(t *testing.T) { t.Parallel() diff --git a/shortcuts/sheets/lark_sheet_batch_update_test.go b/shortcuts/sheets/lark_sheet_batch_update_test.go index e122c1355..56e2c5e2b 100644 --- a/shortcuts/sheets/lark_sheet_batch_update_test.go +++ b/shortcuts/sheets/lark_sheet_batch_update_test.go @@ -5,7 +5,6 @@ package sheets import ( "encoding/json" - "strings" "testing" ) @@ -166,18 +165,16 @@ func TestCellsBatchClear_Guards(t *testing.T) { t.Parallel() // sheetless range → prefix guard (shared with the dropdown fan-outs). - stdout, stderr, err := runShortcutCapturingErr(t, CellsBatchClear, []string{ + _, _, err := runShortcutCapturingErr(t, CellsBatchClear, []string{ "--url", testURL, "--ranges", `["A1:A10"]`, "--yes", "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "must include a sheet prefix") { - t.Errorf("expected sheet-prefix guard; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "must include a sheet prefix") // missing --yes → confirmation_required (high-risk-write). - stdout, stderr, err = runShortcutCapturingErr(t, CellsBatchClear, []string{ + stdout, stderr, err := runShortcutCapturingErr(t, CellsBatchClear, []string{ "--url", testURL, "--ranges", `["sheet1!A1:A10"]`, }) @@ -268,38 +265,32 @@ func TestBatchUpdate_ValidationGuards(t *testing.T) { t.Parallel() // dropdown-update with sheetless range - stdout, stderr, err := runShortcutCapturingErr(t, DropdownUpdate, []string{ + _, _, err := runShortcutCapturingErr(t, DropdownUpdate, []string{ "--url", testURL, "--ranges", `["A2:A5"]`, "--options", `["a"]`, "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "must include a sheet prefix") { - t.Errorf("expected sheet-prefix guard for +dropdown-update; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "must include a sheet prefix") // batch-update with empty operations - stdout, stderr, err = runShortcutCapturingErr(t, BatchUpdate, []string{ + _, _, err = runShortcutCapturingErr(t, BatchUpdate, []string{ "--url", testURL, "--operations", `[]`, "--yes", "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "non-empty JSON array") { - t.Errorf("expected empty-operations guard; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "non-empty JSON array") // dropdown-update with non-array --options (object instead) → array guard // (now via schema validator at parseJSONFlag time) - stdout, stderr, err = runShortcutCapturingErr(t, DropdownUpdate, []string{ + _, _, err = runShortcutCapturingErr(t, DropdownUpdate, []string{ "--url", testURL, "--ranges", `["sheet1!A1:A2"]`, "--options", `{"not":"array"}`, "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), `expected type "array"`) { - t.Errorf("expected JSON array guard; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, `expected type "array"`) } // TestValidateDropdownRanges_RejectsMalformedRange locks the up-front sheet!range @@ -322,15 +313,13 @@ func TestValidateDropdownRanges_RejectsMalformedRange(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, DropdownUpdate, []string{ + _, _, err := runShortcutCapturingErr(t, DropdownUpdate, []string{ "--url", testURL, "--ranges", tc.ranges, "--options", `["a"]`, "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), tc.want) { - t.Errorf("ranges=%s: expected error containing %q; got=%s|%s|%v", tc.ranges, tc.want, stdout, stderr, err) - } + requireValidation(t, err, tc.want) }) } } @@ -419,18 +408,13 @@ func TestBatchUpdate_TranslatorRejects(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, BatchUpdate, []string{ + _, _, err := runShortcutCapturingErr(t, BatchUpdate, []string{ "--url", testURL, "--operations", tc.opsJSON, "--yes", "--dry-run", }) - if err == nil { - t.Fatalf("expected error containing %q; got stdout=%s stderr=%s", tc.wantMatch, stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), tc.wantMatch) { - t.Errorf("expected error containing %q; got: %s | %s | %v", tc.wantMatch, stdout, stderr, err) - } + requireValidation(t, err, tc.wantMatch) }) } } diff --git a/shortcuts/sheets/lark_sheet_dataframe_test.go b/shortcuts/sheets/lark_sheet_dataframe_test.go index a119187cc..155f2bcfd 100644 --- a/shortcuts/sheets/lark_sheet_dataframe_test.go +++ b/shortcuts/sheets/lark_sheet_dataframe_test.go @@ -239,12 +239,7 @@ func TestReadDataframeBytes_RejectsSecondStdinConsumer(t *testing.T) { rctx.MarkStdinConsumed() _, err := readDataframeBytes(rctx, "-") - if err == nil { - t.Fatal("err is nil; want stdin-already-consumed validation error") - } - if !strings.Contains(err.Error(), "stdin (-) can only be used by one flag") { - t.Fatalf("err = %q, want it to flag the stdin-already-consumed conflict", err.Error()) - } + requireValidation(t, err, "stdin (-) can only be used by one flag") } // TestDataframe_EncodeRoundTrip checks --dataframe-out's encoder against its diff --git a/shortcuts/sheets/lark_sheet_object_crud_test.go b/shortcuts/sheets/lark_sheet_object_crud_test.go index 222c11699..b6306e6f0 100644 --- a/shortcuts/sheets/lark_sheet_object_crud_test.go +++ b/shortcuts/sheets/lark_sheet_object_crud_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -472,24 +473,18 @@ func TestPivotCreate_SheetSelectorSemantics(t *testing.T) { t.Run("both set is rejected", func(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, PivotCreate, []string{ + _, _, err := runShortcutCapturingErr(t, PivotCreate, []string{ "--url", testURL, "--target-sheet-id", testSheetID, "--target-sheet-name", "Sheet1", "--properties", `{"rows":[{"field":"A"}]}`, "--source", "Sheet1!A1:F1000", }) - if err == nil { - t.Fatalf("expected CLI to reject both --target-sheet-id and --target-sheet-name set; stderr=%s", stderr) - } - combined := stderr + err.Error() - if !strings.Contains(combined, "mutually exclusive") { - t.Errorf("expected error to say 'mutually exclusive'; got=%s|%v", stderr, err) - } + ve := requireValidation(t, err, "mutually exclusive") // 错误信息必须用真实的 flag 名(target-*),否则模型按消息提示去 // 改 --sheet-id 还是错的。 - if !strings.Contains(combined, "--target-sheet-id") { - t.Errorf("expected error to quote --target-sheet-id flag name; got=%s|%v", stderr, err) + if !strings.Contains(ve.Message, "--target-sheet-id") { + t.Errorf("expected error to quote --target-sheet-id flag name; got message=%q", ve.Message) } }) @@ -519,7 +514,7 @@ func TestPivotCreate_TargetPositionRangeMutex(t *testing.T) { t.Run("both non-default values rejected", func(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, PivotCreate, []string{ + _, _, err := runShortcutCapturingErr(t, PivotCreate, []string{ "--url", testURL, "--target-sheet-id", testSheetID, "--properties", `{"rows":[{"field":"A"}]}`, @@ -527,15 +522,9 @@ func TestPivotCreate_TargetPositionRangeMutex(t *testing.T) { "--target-position", "B5", "--range", "F1", }) - if err == nil { - t.Fatalf("expected CLI to reject --target-position with --range; stderr=%s", stderr) - } - combined := stderr + err.Error() - if !strings.Contains(combined, "mutually exclusive") { - t.Errorf("expected error to say 'mutually exclusive'; got=%s|%v", stderr, err) - } - if !strings.Contains(combined, "--target-position") || !strings.Contains(combined, "--range") { - t.Errorf("expected error to quote both --target-position and --range; got=%s|%v", stderr, err) + ve := requireValidation(t, err, "mutually exclusive") + if !strings.Contains(ve.Message, "--target-position") || !strings.Contains(ve.Message, "--range") { + t.Errorf("expected error to quote both --target-position and --range; got message=%q", ve.Message) } }) @@ -568,35 +557,27 @@ func TestPivotCreate_SchemaValidates(t *testing.T) { t.Run("rejects wrong type for rows", func(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, PivotCreate, []string{ + _, _, err := runShortcutCapturingErr(t, PivotCreate, []string{ "--url", testURL, "--properties", `{"rows":"not-an-array"}`, "--source", "Sheet1!A1:F1000", "--dry-run", }) - if err == nil { - t.Fatalf("expected schema validator to reject rows=string; stderr=%s", stderr) - } - combined := stderr + err.Error() - if !strings.Contains(combined, "rows") || !strings.Contains(combined, "array") { - t.Errorf("expected error to mention rows/array; got=%s|%v", stderr, err) + ve := requireValidation(t, err, "rows") + if !strings.Contains(ve.Message, "array") { + t.Errorf("expected error to mention array; got message=%q", ve.Message) } }) t.Run("rejects out-of-enum summarize_by", func(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, PivotCreate, []string{ + _, _, err := runShortcutCapturingErr(t, PivotCreate, []string{ "--url", testURL, "--properties", `{"values":[{"field":"A","summarize_by":"BOGUS"}]}`, "--source", "Sheet1!A1:F1000", "--dry-run", }) - if err == nil { - t.Fatalf("expected schema validator to reject summarize_by=BOGUS; stderr=%s", stderr) - } - if !strings.Contains(stderr+err.Error(), "summarize_by") { - t.Errorf("expected error to mention summarize_by; got=%s|%v", stderr, err) - } + requireValidation(t, err, "summarize_by") }) t.Run("schema-conformant input is accepted", func(t *testing.T) { @@ -630,14 +611,8 @@ func TestObjectCreate_RequiresSheetSelector(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, tt.sc, tt.args) - if err == nil { - t.Fatalf("expected CLI to reject empty sheet selector for +%s-create; stderr=%s", tt.name, stderr) - } - combined := stderr + err.Error() - if !strings.Contains(combined, "specify at least one of --sheet-id or --sheet-name") { - t.Errorf("expected 'specify at least one of --sheet-id or --sheet-name'; got=%s|%v", stderr, err) - } + _, _, err := runShortcutCapturingErr(t, tt.sc, tt.args) + requireValidation(t, err, "specify at least one of --sheet-id or --sheet-name") }) } } @@ -648,19 +623,13 @@ func TestObjectCreate_RequiresSheetSelector(t *testing.T) { // +sparkline-list, before any server call goes out. func TestSparklineUpdate_MissingSparklineID(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, SparklineUpdate, []string{ + _, _, err := runShortcutCapturingErr(t, SparklineUpdate, []string{ "--url", testURL, "--sheet-id", testSheetID, "--group-id", "grpA", "--properties", `{"sparklines":[{"source":"Sheet1!A1:A10"}]}`, }) - if err == nil { - t.Fatalf("expected CLI to reject missing sparkline_id; stderr=%s", stderr) - } - combined := stderr + err.Error() - if !strings.Contains(combined, "missing sparkline_id") { - t.Errorf("expected error to mention missing sparkline_id; got=%s|%v", stderr, err) - } - if !strings.Contains(combined, "+sparkline-list") { - t.Errorf("expected error to point at +sparkline-list; got=%s|%v", stderr, err) + ve := requireValidation(t, err, "missing sparkline_id") + if !strings.Contains(ve.Message, "+sparkline-list") { + t.Errorf("expected error to point at +sparkline-list; got message=%q", ve.Message) } } @@ -729,12 +698,7 @@ func TestCondFormatAttrs_ShapeMatchesRuleType(t *testing.T) { t.Parallel() _, stderr, err := runShortcutCapturingErr(t, tt.sc, tt.args) if tt.wantErr { - if err == nil { - t.Fatalf("expected rejection; stderr=%s", stderr) - } - if combined := stderr + err.Error(); tt.wantMsg != "" && !strings.Contains(combined, tt.wantMsg) { - t.Errorf("expected error to mention %q; got=%s|%v", tt.wantMsg, stderr, err) - } + requireValidation(t, err, tt.wantMsg) return } if err != nil { @@ -851,18 +815,13 @@ func TestCondFormatAttrsRequired_MatchesSchemaOneOf(t *testing.T) { // create still mandates one of --image / --image-token / --image-uri. func TestFloatImageCreate_RequiresImageSource(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, FloatImageCreate, []string{ + _, _, err := runShortcutCapturingErr(t, FloatImageCreate, []string{ "--url", testURL, "--sheet-id", testSheetID, "--image-name", "x.png", "--position-row", "0", "--position-col", "A", "--size-width", "10", "--size-height", "10", }) - if err == nil { - t.Fatalf("expected CLI to require an image source on create; stderr=%s", stderr) - } - if combined := stderr + err.Error(); !strings.Contains(combined, "one of --image, --image-token, or --image-uri is required") { - t.Errorf("expected error to require an image source; got=%s|%v", stderr, err) - } + requireValidation(t, err, "one of --image, --image-token, or --image-uri is required") } // TestObjectDelete_AllHighRisk asserts every delete shortcut blocks @@ -885,14 +844,8 @@ func TestObjectDelete_AllHighRisk(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, tt.sc, tt.args) - if err == nil { - t.Fatalf("expected confirmation_required; stdout=%s stderr=%s", stdout, stderr) - } - combined := stdout + stderr + err.Error() - if !strings.Contains(combined, "confirmation_required") && !strings.Contains(combined, "requires confirmation") { - t.Errorf("expected confirmation gate; got=%s|%s|%v", stdout, stderr, err) - } + _, _, err := runShortcutCapturingErr(t, tt.sc, tt.args) + requireProblem(t, err, errs.CategoryConfirmation, errs.SubtypeConfirmationRequired, "") }) } } diff --git a/shortcuts/sheets/lark_sheet_range_operations_test.go b/shortcuts/sheets/lark_sheet_range_operations_test.go index 0c85bbb66..c2d573fb6 100644 --- a/shortcuts/sheets/lark_sheet_range_operations_test.go +++ b/shortcuts/sheets/lark_sheet_range_operations_test.go @@ -287,16 +287,11 @@ func TestRangeSort_RejectsMalformedKeys(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, RangeSort, []string{ + _, _, err := runShortcutCapturingErr(t, RangeSort, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "A1:E10", "--sort-keys", c.keys, "--dry-run", }) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), c.want) { - t.Errorf("want substring %q in error; got stdout=%s stderr=%s err=%v", c.want, stdout, stderr, err) - } + requireValidation(t, err, c.want) }) } } @@ -349,13 +344,8 @@ func TestResize_TypeAndSizeGuards(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, tt.sc, append(tt.args, "--dry-run")) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), tt.want) { - t.Errorf("expected %q; got=%s|%s|%v", tt.want, stdout, stderr, err) - } + _, _, err := runShortcutCapturingErr(t, tt.sc, append(tt.args, "--dry-run")) + requireValidation(t, err, tt.want) }) } } diff --git a/shortcuts/sheets/lark_sheet_read_data_test.go b/shortcuts/sheets/lark_sheet_read_data_test.go index 6408526cc..ab2a35230 100644 --- a/shortcuts/sheets/lark_sheet_read_data_test.go +++ b/shortcuts/sheets/lark_sheet_read_data_test.go @@ -81,15 +81,12 @@ func TestReadDataShortcuts_DryRun(t *testing.T) { // every other get_cell_ranges wrapper uses. func TestDropdownGet_RequiresSheetSelector(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, DropdownGet, []string{ + _, _, err := runShortcutCapturingErr(t, DropdownGet, []string{ "--url", testURL, "--range", "A2:A100", "--dry-run", }) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } - combined := stdout + stderr + err.Error() - if !strings.Contains(combined, "sheet-id") && !strings.Contains(combined, "sheet-name") { - t.Errorf("expected --sheet-id/--sheet-name guard; got=%s|%s|%v", stdout, stderr, err) + ve := requireValidation(t, err, "") + if !strings.Contains(ve.Message, "sheet-id") && !strings.Contains(ve.Message, "sheet-name") { + t.Errorf("expected --sheet-id/--sheet-name guard; got message=%q", ve.Message) } } @@ -109,15 +106,10 @@ func TestReadData_RequiresRange(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, c.sc, []string{ + _, _, err := runShortcutCapturingErr(t, c.sc, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", " ", "--dry-run", }) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), "--range is required") { - t.Errorf("expected --range guard; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "--range is required") }) } } diff --git a/shortcuts/sheets/lark_sheet_search_replace_test.go b/shortcuts/sheets/lark_sheet_search_replace_test.go index 7e58abee3..56bf81803 100644 --- a/shortcuts/sheets/lark_sheet_search_replace_test.go +++ b/shortcuts/sheets/lark_sheet_search_replace_test.go @@ -89,14 +89,17 @@ func TestSearchReplaceShortcuts_DryRun(t *testing.T) { func TestCellsReplace_RequireFlag(t *testing.T) { t.Parallel() - // --replace not passed at all (vs empty string) should error. + // --replace not passed at all (vs empty string) should error. This trips + // cobra's required-flag gate before our Validate hook runs, so the error + // is cobra's plain `required flag(s) "replacement" not set` rather than a + // typed *errs.ValidationError — keep this assertion as a substring check. stdout, stderr, err := runShortcutCapturingErr(t, CellsReplace, []string{ "--url", testURL, "--sheet-id", testSheetID, "--find", "foo", "--dry-run", }) if err == nil { - t.Fatalf("expected error when --replace omitted; stdout=%s stderr=%s", stdout, stderr) + t.Fatalf("expected error when --replacement omitted; stdout=%s stderr=%s", stdout, stderr) } - if !strings.Contains(stdout+stderr+err.Error(), "replace") { - t.Errorf("expected message about --replace; got=%s|%s|%v", stdout, stderr, err) + if !strings.Contains(err.Error(), "replacement") { + t.Errorf("expected message about --replacement; got=%s|%s|%v", stdout, stderr, err) } } diff --git a/shortcuts/sheets/lark_sheet_sheet_structure_test.go b/shortcuts/sheets/lark_sheet_sheet_structure_test.go index 827f33e75..18787926f 100644 --- a/shortcuts/sheets/lark_sheet_sheet_structure_test.go +++ b/shortcuts/sheets/lark_sheet_sheet_structure_test.go @@ -198,13 +198,8 @@ func TestDimRange_Validation(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, DimHide, tt.args) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), tt.want) { - t.Errorf("expected %q substring; got=%s|%s|%v", tt.want, stdout, stderr, err) - } + _, _, err := runShortcutCapturingErr(t, DimHide, tt.args) + requireValidation(t, err, tt.want) }) } } @@ -269,16 +264,11 @@ func TestDimMove_Column(t *testing.T) { // column (or vice versa) is rejected at Validate. func TestDimMove_MismatchedDimension(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, DimMove, []string{ + _, _, err := runShortcutCapturingErr(t, DimMove, []string{ "--url", testURL, "--sheet-id", testSheetID, "--source-range", "1:3", "--target", "H", "--dry-run", }) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), "must match --source-range") { - t.Errorf("expected dimension-mismatch guard; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "must match --source-range") } // TestParseA1Range covers parser edge cases directly. diff --git a/shortcuts/sheets/lark_sheet_style_alias_test.go b/shortcuts/sheets/lark_sheet_style_alias_test.go index a9f473ee0..d0db6c327 100644 --- a/shortcuts/sheets/lark_sheet_style_alias_test.go +++ b/shortcuts/sheets/lark_sheet_style_alias_test.go @@ -58,12 +58,7 @@ func TestNormalizeCellStyleAliases(t *testing.T) { "horizontal_alignment": "left", } err := normalizeCellStyleAliases(style, "cell_styles[0]") - if err == nil { - t.Fatalf("expected conflict error, got nil") - } - if !strings.Contains(err.Error(), "conflicts with horizontal_alignment") { - t.Errorf("error should name the conflict: %v", err) - } + requireValidation(t, err, "conflicts with horizontal_alignment") }) t.Run("no shorthand leaves the map untouched", func(t *testing.T) { @@ -142,12 +137,7 @@ func TestNormalizeTypedCellsStyleAliases(t *testing.T) { }, } err := normalizeTypedCellsStyleAliases(cells, "--cells") - if err == nil { - t.Fatalf("expected conflict error, got nil") - } - if !strings.Contains(err.Error(), "--cells[0][0].cell_styles") { - t.Errorf("error should carry the cell path: %v", err) - } + requireValidation(t, err, "--cells[0][0].cell_styles") }) } diff --git a/shortcuts/sheets/lark_sheet_table_io_test.go b/shortcuts/sheets/lark_sheet_table_io_test.go index ab0fdafee..d387fc4e4 100644 --- a/shortcuts/sheets/lark_sheet_table_io_test.go +++ b/shortcuts/sheets/lark_sheet_table_io_test.go @@ -285,9 +285,7 @@ func TestTablePut_PayloadValidation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() _, err := parseTablePutPayload(stubFlagView{"sheets": tt.json}) - if err == nil || !strings.Contains(err.Error(), tt.want) { - t.Errorf("want error containing %q, got %v", tt.want, err) - } + requireValidation(t, err, tt.want) }) } } @@ -392,18 +390,13 @@ func TestTablePut_DryRunWithStyles(t *testing.T) { // doesn't match the payload sheet is rejected at Validate, before any write. func TestTablePut_StylesNameMismatchRejected(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, TablePut, []string{ + _, _, err := runShortcutCapturingErr(t, TablePut, []string{ "--url", testURL, "--sheets", `{"sheets":[{"name":"数据","columns":["a"],"data":[["x"]]}]}`, "--styles", `{"styles":[{"name":"其他","cell_styles":[{"range":"A1","font_weight":"bold"}]}]}`, "--dry-run", }) - if err == nil { - t.Fatalf("expected validation error; got nil. stdout=%s stderr=%s", stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), "must match") { - t.Errorf("error should flag the name mismatch; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "must match") } // TestTablePut_ExecuteWithStyles drives the full write + visual-ops path: the @@ -469,13 +462,8 @@ func TestTablePut_Validation(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, TablePut, append(tt.args, "--dry-run")) - if err == nil { - t.Fatalf("expected validation error; got nil. stdout=%s stderr=%s", stdout, stderr) - } - if !strings.Contains(stdout+stderr+err.Error(), tt.want) { - t.Errorf("error missing %q; got=%s|%s|%v", tt.want, stdout, stderr, err) - } + _, _, err := runShortcutCapturingErr(t, TablePut, append(tt.args, "--dry-run")) + requireValidation(t, err, tt.want) }) } } @@ -692,13 +680,8 @@ func TestWorkbookCreate_TypedMutualExclusion(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, WorkbookCreate, tc.args) - if err == nil { - t.Fatalf("expected mutual-exclusion error; got nil (stderr=%s)", stderr) - } - if !strings.Contains(err.Error(), "mutually exclusive") { - t.Errorf("want 'mutually exclusive' error; got %v", err) - } + _, _, err := runShortcutCapturingErr(t, WorkbookCreate, tc.args) + requireValidation(t, err, "mutually exclusive") }) } } @@ -708,13 +691,8 @@ func TestWorkbookCreate_TypedMutualExclusion(t *testing.T) { // through to creating an empty workbook. func TestWorkbookCreate_EmptySheetsErrors(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, WorkbookCreate, []string{"--title", "X", "--sheets", ""}) - if err == nil { - t.Fatalf("expected error for empty --sheets; got nil (stderr=%s)", stderr) - } - if !strings.Contains(err.Error(), "empty") { - t.Errorf("want 'empty' error; got %v", err) - } + _, _, err := runShortcutCapturingErr(t, WorkbookCreate, []string{"--title", "X", "--sheets", ""}) + requireValidation(t, err, "empty") } // TestWorkbookCreate_TypedAdoptsDefaultSheet covers the one-step typed create: @@ -897,9 +875,7 @@ func TestTablePut_HeaderAndMode(t *testing.T) { func TestTablePut_BadModeRejected(t *testing.T) { t.Parallel() _, err := parseTablePutPayload(stubFlagView{"sheets": `{"sheets":[{"name":"S","mode":"upsert","columns":["a"],"data":[]}]}`}) - if err == nil || !strings.Contains(err.Error(), "invalid") { - t.Errorf("mode \"upsert\" should be rejected, got %v", err) - } + requireValidation(t, err, "invalid") } // TestTablePut_AppendEmptySheetWritesHeader: appending to an EMPTY sheet still @@ -1210,17 +1186,11 @@ func TestTableGet_DuplicateHeaderRejected(t *testing.T) { `[{"value":"amount"},{"value":"amount"}],`+ `[{"value":1},{"value":2}]`+ `]}]}`) - out, err := runShortcutWithStubs(t, TableGet, + _, err := runShortcutWithStubs(t, TableGet, []string{"--url", testURL, "--sheet-name", "S"}, structure, region, cells) - if err == nil { - t.Fatalf("expected validation error for duplicate header; got nil. out=%s", out) - } - combined := out + err.Error() - if !strings.Contains(combined, "duplicate header column name") { - t.Errorf("error should flag duplicate header; got=%s", combined) - } - if !strings.Contains(combined, "--no-header") { - t.Errorf("error should hint about --no-header; got=%s", combined) + ve := requireValidation(t, err, "duplicate header column name") + if !strings.Contains(ve.Message, "--no-header") { + t.Errorf("error should hint about --no-header; got message=%q", ve.Message) } } diff --git a/shortcuts/sheets/lark_sheet_workbook_import_test.go b/shortcuts/sheets/lark_sheet_workbook_import_test.go index 84b1e8850..bdfebada7 100644 --- a/shortcuts/sheets/lark_sheet_workbook_import_test.go +++ b/shortcuts/sheets/lark_sheet_workbook_import_test.go @@ -66,10 +66,8 @@ func TestWorkbookImport_RejectsNonSheetFile(t *testing.T) { // Validate runs before DryRun, so the pinned-sheet check rejects .docx up // front and the error surfaces through the normal envelope/err path. - stdout, stderr, err := runShortcutCapturingErr(t, WorkbookImport, []string{"--file", "./notes.docx", "--dry-run"}) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "can only be imported") { - t.Errorf("expected .docx → sheet type-mismatch rejection; got stdout=%s stderr=%s err=%v", stdout, stderr, err) - } + _, _, err := runShortcutCapturingErr(t, WorkbookImport, []string{"--file", "./notes.docx", "--dry-run"}) + requireValidation(t, err, "can only be imported") } // TestWorkbookImport_ExecuteCreatesSheet runs the full upload → create → poll diff --git a/shortcuts/sheets/lark_sheet_workbook_test.go b/shortcuts/sheets/lark_sheet_workbook_test.go index 9d3807514..9445b7d1f 100644 --- a/shortcuts/sheets/lark_sheet_workbook_test.go +++ b/shortcuts/sheets/lark_sheet_workbook_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -227,14 +228,8 @@ func TestSheetMove_DryRunResolvePlaceholders(t *testing.T) { // high-risk-write — exit code 10 (confirmation_required) without --yes. func TestSheetDelete_HighRiskWriteRequiresYes(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, SheetDelete, []string{"--url", testURL, "--sheet-id", testSheetID}) - if err == nil { - t.Fatalf("expected confirmation_required error; got nil. stdout=%s stderr=%s", stdout, stderr) - } - combined := stdout + stderr + err.Error() - if !strings.Contains(combined, "confirmation_required") && !strings.Contains(combined, "requires confirmation") { - t.Errorf("expected confirmation envelope; got=%s|%s|%v", stdout, stderr, err) - } + _, _, err := runShortcutCapturingErr(t, SheetDelete, []string{"--url", testURL, "--sheet-id", testSheetID}) + requireProblem(t, err, errs.CategoryConfirmation, errs.SubtypeConfirmationRequired, "") } // TestWorkbook_Validation covers a few critical validation paths shared @@ -248,6 +243,11 @@ func TestWorkbook_Validation(t *testing.T) { sc common.Shortcut args []string wantMsg string + // cobraNative=true means the error originates from cobra's native + // flag parsing (e.g. required-flag enforcement) which is not wrapped + // into a typed errs.ValidationError, so the test falls back to a + // substring match on err.Error(). + cobraNative bool }{ { name: "+workbook-info needs --url or --spreadsheet-token", @@ -256,10 +256,11 @@ func TestWorkbook_Validation(t *testing.T) { wantMsg: "at least one of --url or --spreadsheet-token", }, { - name: "+workbook-info rejects both url and token", - sc: WorkbookInfo, - args: []string{"--url", testURL, "--spreadsheet-token", testToken}, - wantMsg: "mutually exclusive", + name: "+workbook-info rejects both url and token", + sc: WorkbookInfo, + args: []string{"--url", testURL, "--spreadsheet-token", testToken}, + wantMsg: "mutually exclusive", + cobraNative: true, }, { name: "+sheet-delete needs sheet selector", @@ -268,10 +269,11 @@ func TestWorkbook_Validation(t *testing.T) { wantMsg: "at least one of --sheet-id or --sheet-name", }, { - name: "+sheet-create requires --title", - sc: SheetCreate, - args: []string{"--url", testURL}, - wantMsg: "required flag(s) \"title\" not set", + name: "+sheet-create requires --title", + sc: SheetCreate, + args: []string{"--url", testURL}, + wantMsg: "required flag(s) \"title\" not set", + cobraNative: true, }, { name: "+sheet-create row-count over cap", @@ -283,14 +285,14 @@ func TestWorkbook_Validation(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, tt.sc, append(tt.args, "--dry-run")) - if err == nil { - t.Fatalf("expected validation error; got nil. stdout=%s stderr=%s", stdout, stderr) - } - combined := stdout + stderr + err.Error() - if !strings.Contains(combined, tt.wantMsg) { - t.Errorf("error message missing %q; got=%s", tt.wantMsg, combined) + _, _, err := runShortcutCapturingErr(t, tt.sc, append(tt.args, "--dry-run")) + if tt.cobraNative { + if err == nil || !strings.Contains(err.Error(), tt.wantMsg) { + t.Errorf("error message missing %q; got=%v", tt.wantMsg, err) + } + return } + requireValidation(t, err, tt.wantMsg) }) } } @@ -495,10 +497,8 @@ func TestWorkbookCreate_DataValidation(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, WorkbookCreate, append(tt.args, "--dry-run")) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), tt.want) { - t.Errorf("expected %q; got=%s|%s|%v", tt.want, stdout, stderr, err) - } + _, _, err := runShortcutCapturingErr(t, WorkbookCreate, append(tt.args, "--dry-run")) + requireValidation(t, err, tt.want) }) } } @@ -542,12 +542,10 @@ func TestWorkbookExport_DryRun(t *testing.T) { t.Run("csv requires --sheet-id", func(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, WorkbookExport, []string{ + _, _, err := runShortcutCapturingErr(t, WorkbookExport, []string{ "--url", testURL, "--file-extension", "csv", "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "--sheet-id is required") { - t.Errorf("expected sheet-id guard; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "--sheet-id is required") }) } diff --git a/shortcuts/sheets/lark_sheet_write_cells_test.go b/shortcuts/sheets/lark_sheet_write_cells_test.go index 3f5ca70f2..b450bb621 100644 --- a/shortcuts/sheets/lark_sheet_write_cells_test.go +++ b/shortcuts/sheets/lark_sheet_write_cells_test.go @@ -242,18 +242,16 @@ func TestDropdownSet_HighlightTriState(t *testing.T) { // cycles the rest through a built-in palette). func TestDropdownSet_ColorsLongerThanOptions(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, DropdownSet, []string{ + _, _, err := runShortcutCapturingErr(t, DropdownSet, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "A2:A4", "--options", `["a","b"]`, "--colors", `["#FFE699","#bff7d9","#ffb3b3"]`, "--dry-run", }) - if err == nil { - t.Fatal("expected --colors length error, got nil") - } - if !strings.Contains(stderr, "must not exceed dropdown source size") && !strings.Contains(err.Error(), "must not exceed dropdown source size") { - t.Errorf("error message missing length-overflow hint:\nerr=%v\nstderr=%s", err, stderr) + ve := requireValidation(t, err, "must not exceed dropdown source size") + if ve.Param != "--colors" { + t.Errorf("param = %q, want --colors", ve.Param) } } @@ -319,7 +317,7 @@ func TestDropdownSet_ListFromRange(t *testing.T) { // must be refused). func TestDropdownSet_ListFromRange_ColorsLongerThanCells(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, DropdownSet, []string{ + _, _, err := runShortcutCapturingErr(t, DropdownSet, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "B2:B21", "--source-range", "Sheet1!T1:T3", @@ -327,11 +325,9 @@ func TestDropdownSet_ListFromRange_ColorsLongerThanCells(t *testing.T) { "--highlight", "--dry-run", }) - if err == nil { - t.Fatal("expected --colors length error, got nil") - } - if !strings.Contains(stderr, "must not exceed dropdown source size") && !strings.Contains(err.Error(), "must not exceed dropdown source size") { - t.Errorf("error message missing source-size hint:\nerr=%v\nstderr=%s", err, stderr) + ve := requireValidation(t, err, "must not exceed dropdown source size") + if ve.Param != "--colors" { + t.Errorf("param = %q, want --colors", ve.Param) } } @@ -339,36 +335,26 @@ func TestDropdownSet_ListFromRange_ColorsLongerThanCells(t *testing.T) { // --source-range. func TestDropdownSet_XorBothSet(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, DropdownSet, []string{ + _, _, err := runShortcutCapturingErr(t, DropdownSet, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "B2:B21", "--options", `["a","b"]`, "--source-range", "Sheet1!T1:T3", "--dry-run", }) - if err == nil { - t.Fatal("expected XOR error, got nil") - } - if !strings.Contains(stderr, "mutually exclusive") && !strings.Contains(err.Error(), "mutually exclusive") { - t.Errorf("error message missing XOR hint:\nerr=%v\nstderr=%s", err, stderr) - } + requireValidation(t, err, "mutually exclusive") } // TestDropdownSet_XorNeitherSet rejects passing neither --options nor // --source-range. func TestDropdownSet_XorNeitherSet(t *testing.T) { t.Parallel() - _, stderr, err := runShortcutCapturingErr(t, DropdownSet, []string{ + _, _, err := runShortcutCapturingErr(t, DropdownSet, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "B2:B21", "--dry-run", }) - if err == nil { - t.Fatal("expected required-one error, got nil") - } - if !strings.Contains(stderr, "one of --options") && !strings.Contains(err.Error(), "one of --options") { - t.Errorf("error message missing required-one hint:\nerr=%v\nstderr=%s", err, stderr) - } + requireValidation(t, err, "one of --options") } // TestCellsSetStyle_FlatFlags verifies that the 11 flat style flags + @@ -401,30 +387,25 @@ func TestCellsSetStyle_FlatFlags(t *testing.T) { func TestCellsSetStyle_RequiresAtLeastOneFlag(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, CellsSetStyle, []string{ + _, _, err := runShortcutCapturingErr(t, CellsSetStyle, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "A1:B2", "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "at least one style flag") { - t.Errorf("expected style-flag guard; got=%s|%s|%v", stdout, stderr, err) - } + requireValidation(t, err, "at least one style flag") } func TestCellsSet_RequiresJSONArray(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, CellsSet, []string{ + _, _, err := runShortcutCapturingErr(t, CellsSet, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "A1", "--cells", `{"foo":"bar"}`, "--dry-run", }) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } // Schema validator fires first now: "--cells: expected type \"array\", got \"object\"". // Either the schema phrasing or the legacy requireJSONArray phrasing is // acceptable — both pin the same contract. - combined := stdout + stderr + err.Error() - if !strings.Contains(combined, `expected type "array"`) && !strings.Contains(combined, "must be a JSON array") { - t.Errorf("expected array-type guard; got=%s|%s|%v", stdout, stderr, err) + ve := requireValidation(t, err, "") + if !strings.Contains(ve.Message, `expected type "array"`) && !strings.Contains(ve.Message, "must be a JSON array") { + t.Errorf("expected array-type guard; got message=%q", ve.Message) } } @@ -437,16 +418,13 @@ func TestCellsSet_RequiresJSONArray(t *testing.T) { func TestCellsSet_RejectsUnsupportedMentionType(t *testing.T) { t.Parallel() cells := `[[{"rich_text":[{"type":"mention","text":"x","mention_type":6,"mention_token":"t"}]}]]` - stdout, stderr, err := runShortcutCapturingErr(t, CellsSet, []string{ + _, _, err := runShortcutCapturingErr(t, CellsSet, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "A1", "--cells", cells, "--dry-run", }) - if err == nil { - t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr) - } - combined := stdout + stderr + err.Error() - if !strings.Contains(combined, "mention_type") || !strings.Contains(combined, "not in enum") { - t.Errorf("expected mention_type enum guard; got=%s|%s|%v", stdout, stderr, err) + ve := requireValidation(t, err, "mention_type") + if !strings.Contains(ve.Message, "not in enum") { + t.Errorf("expected enum guard; got message=%q", ve.Message) } } @@ -520,12 +498,13 @@ func TestCellsSetImage_DryRun(t *testing.T) { func TestCellsSetImage_RangeMustBeSingleCell(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, CellsSetImage, []string{ + _, _, err := runShortcutCapturingErr(t, CellsSetImage, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "A1:B2", "--image", "./foo.png", "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "must be exactly one cell") { - t.Errorf("expected single-cell guard; got=%s|%s|%v", stdout, stderr, err) + ve := requireValidation(t, err, "must be exactly one cell") + if ve.Param != "--range" { + t.Errorf("param = %q, want --range", ve.Param) } } @@ -534,12 +513,13 @@ func TestCellsSetImage_RangeMustBeSingleCell(t *testing.T) { // same way as a real run instead of printing a misleading success preview. func TestCellsSetImage_DryRunRejectsUnsafePath(t *testing.T) { t.Parallel() - stdout, stderr, err := runShortcutCapturingErr(t, CellsSetImage, []string{ + _, _, err := runShortcutCapturingErr(t, CellsSetImage, []string{ "--url", testURL, "--sheet-id", testSheetID, "--range", "A1", "--image", "/etc/hosts", "--dry-run", }) - if err == nil || !strings.Contains(stdout+stderr+err.Error(), "must be a relative path") { - t.Errorf("expected unsafe-path guard during dry-run; got=%s|%s|%v", stdout, stderr, err) + ve := requireValidation(t, err, "must be a relative path") + if ve.Param != "--image" { + t.Errorf("param = %q, want --image", ve.Param) } }