skills/larksuite__cli
1
0
Fork 0
mirror of https://github.com/larksuite/cli.git synced 2026-09-14 18:42:53 +08:00
Code Issues Packages Projects Releases Wiki Activity
Files
main
larksuite__cli/shortcuts/sheets/json_flag_normalize_test.go
T
xiongyuanwen-byted c2afcce1b5 feat(sheets): accept the payload spellings the 08-29..31 reflow rejected (#2611) 
* feat(sheets): accept the payload spellings the 08-29..31 reflow rejected

The 08-29..31 reflow report attributes 2839 failures across 31388 sheets
calls, and a large share of them carry well-formed intent in a spelling the
flag does not document: a numeric column whose cells arrived as JSON strings
(155), a row cut short at its last non-empty cell (25), `columns` written as
one object per column (35), the sub-sheet list without its {"sheets":...}
envelope (10), a quoted font_size (30). Each rejection cost a retry that
rewrote the same data differently, and each shape has exactly one reading.

Accept them, on the acceptance-layer contract already stated in
style_vocab.go: one canonical form documented, a wide layer undocumented, and
no rewrite whose meaning is in doubt. A non-numeric string in a numeric
column, a row wider than `columns`, and a column-less sheet that does carry
rows all still fail. `dtypes` additionally reads the writer's own type
vocabulary (number, date), which used to fall through to the string default
and silently write figures as text -- a wrong result rather than a rejection,
so it never surfaced as an error at all.

Three more commands get the same treatment. --properties on
+cond-format-create renames `operator` to compare_type under the {value} /
{text} shapes, canonicalizes symbol and abbreviation comparisons, stringifies
a numeric threshold to match the schema, joins a two-element between list,
and lifts a bare attrs object into the one-entry list; rules whose own
contract spells `operator` keep it. +csv-put answers to --data / --content /
--csv-file, and a path-shaped --csv value naming a real file is read as
@<path> would, reported in the result's warnings rather than silently --
requiring the prefix there was an inconsistency of that flag's own making,
since every sibling path-valued flag takes the same value bare. --font-line
takes the CSS text-decoration words.

Where a fix would have to guess, the error carries the fix instead:
+cells-get rejects an Excel multi-area range client-side with the enclosing
rectangle spelled out (54 rejections the backend answered with a bare
"invalid range"), and row_height / col_width / wrap / unmerge_cells inside a
cell_styles item name where they really live.

Everything stays inside shortcuts/sheets. Deliberately unchanged: shrinking
--range to fit --cells (the dimension check documents why), chunking a
--styles spec past the 100-operation cap (it would drop the atomicity of one
batch_update), and the server-side merged-region conflicts.

* feat(sheets): answer the reflow report's full error breakdown

The 08-29..31 reflow report gained two sections: a 58-row table covering
every long-tail command, and a per-command breakdown of the "other" buckets
its first pass had summarized as a count. Together they name ~1000 failures
the earlier commit could not see. This works through them on the same rule:
accept what has one reading, prescribe what does not, and leave alone what
needs a guess.

The one defect among them is on --cells. A cell carrying style fields at its
top level ({"value":"x","font_weight":"bold","border":{…}}) passed every
client check and reached the backend verbatim, which answered
`[cells[0][0].border] unexpected property "border" is not defined` -- 33
rejections on payloads the --styles path accepts, which is exactly the
vocabulary-parity break style_vocab.go's contract forbids. Style fields now
fold into the carrier that holds them (border into border_styles, scalars
into cell_styles) and their values canonicalize on the same pass, so a
boolean word_wrap no longer dies on the schema's "expected type string" one
step later. A key this domain does not know is still passed through: the
tool contract may gain fields between builds.

Accepted, each with the rejection count it removes: the two object spellings
of --writes, the {"writes":[…]} envelope and a lone write object (54);
`type` as the line-kind slot inside a border spec, which is the Lark OpenAPI
and openpyxl spelling (29); a blank column heading, which is a spacer column
or the cells under a merged title (13); blank text in a date column, mirroring
the numeric rule (7); `custom` as a row/col sizing type when the op carries a
size (Excel's and Lark's own UI word); and cell-style vocabulary inside a
conditional-format rule's style block, where background_color / font_color /
font_weight / font_line fold onto back_color / fore_color / font /
text_decoration (9).

Prescribed, where the fix changes the shape: --position on +dim-delete, whose
sibling +dim-insert does take it (18); --ranges on +cells-unmerge (7);
--include on +csv-get (3); the sheet selectors on +styles-put; --payload on
+table-put; a whole-column or whole-row range where a cell style needs a
rectangle, naming row_sizes / col_sizes as the carrier that does take that
form (28); and border keys written on a styles item instead of a cell_styles
entry. Four invented subcommands (+cells-get-style, +dropdown-list, +meta,
+sheets-list) name the command they meant.

Renamed silently, where the command already has the input under another
spelling: --title on +workbook-import (16), --file / --outdir on
+workbook-export (15), --replace on +cells-replace (7), --output on +csv-get,
plus --sheet-name on +sheet-create and --new-title on +sheet-rename.

Finally, the Excel multi-area --range moves from +cells-get's own Validate to
the shared --range chain: 5 more of those landed on +csv-get and 4 on
+cells-set-style, so the habit is not specific to reads. It is chained after
the sheet-prefix rewrite, so a sheet name containing a comma cannot be read
as several areas.

* feat(sheets): retry transient reads, and answer three stale-state failures

Four fixes from the reflow report's long-tail table that need no protocol
decision, in descending order of the rejections they remove.

A read tool call is now reissued up to twice on a transient failure. The
report's single largest cause on +csv-get was "API call failed: server time
out error" -- 25 of its 71 rejections -- with more on +cells-get and
+workbook-info, each on a command that was written correctly and succeeded
when the agent reissued it by hand. Reads only: this API has no idempotency
key, so a create that timed out after the backend committed it would be
committed twice, which is why the shared RetryTransport is installed with
MaxRetries at 0 and why this sits where the read/write classification is
already known. A rate limit is excluded even though the classifier marks it
retryable -- the server is asking for less traffic, and a fixed sub-second
backoff answers that by sending more.

+filter-create accepted its own documented contract only in the flag
description. "The flag is optional overall -- if omitted, an empty filter is
created on --range" was false: `rules` is required at the properties root, so
omitting --properties failed with `required property "rules" is missing`, an
error about a flag the caller deliberately did not pass. The empty rule set
is now the default, which is what a filter with no column conditions is.

A "not found" on an id-addressed update or delete now points at the list
command that reports the live ids. The backend answers with the id alone
("conditional format iXGbyDwC not found"), which reads like a transport
problem rather than a stale reference; +cond-format-update and
+cond-format-delete contributed 14 rejections between them, and the same
prescription covers every id-addressed object.

Not done here, and not because they are hard: the ~480 rejections behind the
four protocol decisions (a --title default, shrinking --range to fit --cells,
merged-region conflicts, chunking past the operation cap) are the caller's
call, not mine; the ~185 PowerShell argv failures are destroyed before the
process starts and can only be answered by the Windows reference; and write
retries need an idempotency key from the backend.

* feat(sheets): narrow an oversized --range, prescribe merge conflicts, split large style specs

Three of the four decisions the reflow report left open, as decided by the
caller. The fourth, defaulting --title from the first sub-sheet name, is
declined: the title is what the workbook is called in Drive, and deriving it
would produce a shelf of spreadsheets named after their first tab. That one is
answered in the skill reference instead.

--cells that FITS inside --range now narrows the write to the payload instead
of failing. The anchor is unchanged, so every cell lands where the caller put
it, and no cell outside the payload is touched -- the stated range only ever
said how far the caller thought the payload reached. The dominant shape is a
one-cell title against the range it will occupy once merged, where writing the
top-left is what a merged region needs anyway. A payload that OVERFLOWS its
range is still rejected: growing the range would write over cells nobody
named. The narrowing rides back in the success envelope's warnings, because
the caller did state an extent. 122 rejections.

A merged-cell rejection now carries the commands that resolve it. The backend
names the obstacle -- the top-left of the region a write landed inside, or the
0-based bounds of the region a merge would overlap -- but never in A1
notation, and never with the command that clears it; "[0,0-0,6]" becomes
`+cells-unmerge --range "A1:G1"`. The message is read, never used to rewrite
the request: auto-redirecting a write would put data where the caller did not
ask for it, and auto-unmerging would discard a merge nobody agreed to lose. A
parse that finds nothing adds no hint. 193 rejections, which stay failures but
become one-retry failures.

+styles-put splits a spec past the per-request cap instead of refusing it. The
100 was never a server contract -- it is our own materialization guard -- so
the fix is to keep each request at that known-good size and send several,
which is the same license coalesceStyleStamps already takes when it fuses
adjacent stamps and the same thing +table-put does when it slices a large
write. The whole-spec ceiling moves to 1000, the request count rides in the
result, and a mid-way failure names the chunks that already applied and says
re-running the whole spec is safe. --writes keeps its cap: it is one atomic
batch_update by contract. 48 rejections.

* fix(sheets): make the payload prescriptions correct on PowerShell

Every composite flag in this domain takes its payload three ways -- inline, as
a relative @file, or on stdin -- and the prescriptions named the third one in
a spelling PowerShell does not have. `--cells - < cells.json` fails there with
"The '<' operator is reserved for future use": an error about the shell, on a
line this CLI told the caller to run. The pipe is no better, since PowerShell
5 re-encodes non-ASCII on the way through and turns a CJK payload into the
same invalid JSON by another route; the windows-compat skill reference rules
both out, and this code was contradicting it.

On windows every prescription now lands on @file, quoted -- a bare @ opens a
splatting expression -- and says why inlining failed in the first place: the
shell splits a JSON argument on the quotes and commas inside it, and single
quotes do not prevent that. The caller quoted the argument and has no reason
to suspect the quoting, which is what made these 89 rejections cost more than
one retry apiece. POSIX shells keep the redirection form they do have.

The other half of that class arrived as "positional arguments are not
supported": once the quote stack is split, the tail of the JSON is a
positional argument. The framework's message ("pass values via flags") is
correct and useless there -- the caller DID pass a flag -- but it is one
message for every command on every platform, so the cause is named here
instead, where the domain knows both the shell and which of its flags carry a
payload big enough to split. Chained onto cobra's Args the same way the
unknown-flag and enum rewrites chain onto their hooks; non-windows keeps the
framework wording, where a positional argument usually is one. 35 rejections.

* fix(sheets): answer the review of the reflow leniency PR

Nine defects the review found in the acceptance layer, plus the lint /
deadcode failure that blocked CI.

Silent wrong values. A `between` threshold list of any length was joined
into a comma string, so one or three thresholds passed the local shape
check and failed at the backend. The literal "null" decodes into any
destination without an error and leaves it at the zero value: in a
numeric column it marshalled back out as 0, and under `font_size` it
became a zero-point font. Both now reject it. `outer` and `all` in one
border spec dropped `outer` even when the two named different boxes,
applying half the caller's intent; a duplicate is still folded away, a
conflict now reaches the invalid-side check.

Rejections the acceptance layer meant to absorb. The multi-area `--range`
prescription took its rectangle from the first and last area, so
"A3,J3,G3" prescribed "A3:G3" and dropped J3; it now spans every area on
both axes. `type:"custom"` was rewritten to pixel only when the op
carried `size`, so the `height` / `width` alias the same parser accepts
everywhere else failed the enum check first.

False reports. `+cells-set` reconstructed the stated range without the
payload, so every bare `--range A1` with a multi-cell payload reported
itself as narrowed. `+table-put --dry-run` skipped the style expansion in
append mode, hiding a style-only write the execute path performs.

Contract. `+styles-put` still promised one fail-fast batch request after
gaining chunking, and its partial-failure advice called a whole-spec
retry safe — replaying an applied `merge_cells` is rejected as an
overlap. Both now state that a large spec can partially apply, and the
merge route is to read back and resend only what did not land.

The windows positional-argument annotation flattened the framework's
error to text; it keeps it as the cause, and the annotation is split out
so its typed shape is asserted on any host rather than only on windows.

CI: `payloadStdinForm` was unreachable — only its `goos` variant is used.

* fix(sheets): answer the second review round

Six findings, one of them a revert.

P1, --csv. A path-shaped value naming a real file was read as @<path>
would. That makes the flag's meaning depend on the working directory: a
caller writing the literal "./data.csv" into a cell uploaded a same-named
local file instead, and learned of the substitution only from a warning
that arrived after the remote write. File intent has to be explicit, so
the read is gone and the existing prescription -- @<path> or stdin --
stands on its own again. The substitution machinery goes with it.

--outdir and --output-dir were aliased onto --output-path, which reads a
value as a directory only when one already exists there. "--outdir
./exports" with no ./exports therefore wrote a file named "exports".
Both spellings say directory in their own name, so they now carry that
intent through the alias-provenance channel already built for +csv-put's
--file, and applyWorkbookOutputPath honors it without probing disk.

Range narrowing reached only the standalone +cells-set. The narrowing
happens in the shared input builder, so a --writes item or a
+batch-update sub-op shipped the shrunken range in silence. The builder
now returns the note fitCellsRange already produced, and all three paths
report it; the standalone path stops reconstructing it from its flags.

A conditional-format style folded any existing `font` value that was not
the incoming word into "bold italic", so {"font":"normal",
"font_weight":"bold"} became valid input with a slant nobody wrote. Only
bold and italic combine now; anything else keeps its value for the schema
to reject.

The stale-object-ID hint matched any "not found", including one about the
sheet selector, and told the caller to refresh rule ids when the ids were
the one input that was right. It now requires the message to name the id
the command addressed, and skips sheet/workbook-scoped misses.

P3: a first-chunk failure in +styles-put reported "requests 1-0 already
applied". That case says nothing landed instead, without the merge
caveat, which does not apply when the sheet is untouched.

Co-authored-by: TRAE CLI <traecli@bytedance.com>

* fix(sheets): answer the third review round

Three follow-ups on the previous round, one of them a correction to it.

The narrowing note reached only the success envelope. A batch is
fail-fast but not transactional, and a transport failure leaves a single
write's outcome unknown, so a narrowed write can be on the sheet after an
error. Both branches now attach the note to the failure as well; a caller
reconciling against the range they stated rather than the one written is
exactly the confusion the note exists to prevent.

A `font` already reading "bold italic" sent the redundant flat spelling
along with it: the previous round stopped combining unrecognized values
but reached the same branch for this one, so the alias was left in the
outgoing style and the schema had no field for it. It is dropped now,
while a genuinely unknown `font` value still keeps its flat sibling
visible for the schema to reject.

The first-chunk case overcorrected. A failed FIRST request does not mean
an untouched sheet: operations before the failing one inside that request
stay applied, and a timeout settles nothing at all. Claiming otherwise
sent the caller into a whole-spec retry that can replay an applied
merge_cells. Only the backend's own "0 succeeded" now earns that message
-- the same test flattenToolErrorMsg uses, so the two cannot disagree
about what a failed batch left behind. Every other first-request failure
keeps the read-back route and the merge caveat.

Co-authored-by: TRAE CLI <traecli@bytedance.com>

---------

Co-authored-by: TRAE CLI <traecli@bytedance.com>
2026-09-07 21:29:15 +08:00

520 lines
18 KiB
Go
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT
package sheets
import (
"encoding/json"
"fmt"
"strings"
"testing"
)
// TestWrapLoneCellObject pins the auto-wrap contract: a bare cell object —
// the classic missing-[[…]] shape agents produce for a 1×1 write — is
// rewritten to [[cell]]; anything whose meaning is not beyond doubt stays
// untouched for the schema validator to prescribe.
func TestWrapLoneCellObject(t *testing.T) {
t.Parallel()
cases := []struct {
name string
in string
wrapped bool
}{
{"lone value cell", `{"value":"hi"}`, true},
{"lone formula cell with styles", `{"formula":"=SUM(A1:A3)","cell_styles":{"font_weight":"bold"}}`, true},
{"unknown key stays", `{"value":"hi","range":"A1"}`, false},
{"array of cells stays (row vs column ambiguous)", `[{"value":"a"},{"value":"b"}]`, false},
{"proper 2D array stays", `[[{"value":"a"}]]`, false},
{"empty object stays", `{}`, false},
{"scalar stays", `"hi"`, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
var v interface{}
if err := json.Unmarshal([]byte(tc.in), &v); err != nil {
t.Fatalf("bad fixture: %v", err)
}
out := wrapLoneCellObject(v)
_, isWrapped := out.([]interface{})
_, wasArray := v.([]interface{})
if tc.wrapped && (!isWrapped || wasArray) {
t.Errorf("expected wrap to [[cell]], got %#v", out)
}
if !tc.wrapped && !wasArray && isWrapped {
t.Errorf("expected no wrap, got %#v", out)
}
if tc.wrapped {
rows, _ := out.([]interface{})
if len(rows) != 1 {
t.Fatalf("want 1 row, got %d", len(rows))
}
cells, _ := rows[0].([]interface{})
if len(cells) != 1 {
t.Fatalf("want 1 cell, got %d", len(cells))
}
}
})
}
}
// TestCellObjectKeys_MatchEmbeddedSchema drift-guards the hardcoded cell
// vocabulary against the embedded +cells-set --cells schema: if the spec
// repo adds or removes a cell property, this fails and cellObjectKeys must
// be updated (an outdated set only narrows the auto-wrap, but silently
// narrowing is still drift).
func TestCellObjectKeys_MatchEmbeddedSchema(t *testing.T) {
t.Parallel()
idx, err := loadFlagSchemas()
if err != nil {
t.Fatalf("loadFlagSchemas: %v", err)
}
raw, ok := idx.Flags["+cells-set"]["cells"]
if !ok {
t.Fatal("embedded schema for +cells-set --cells missing")
}
var schema schemaProperty
if err := json.Unmarshal(raw, &schema); err != nil {
t.Fatalf("unmarshal schema: %v", err)
}
cell := schema.Items
if cell != nil && cell.Items != nil {
cell = cell.Items
}
if cell == nil || len(cell.Properties) == 0 {
t.Fatal("schema shape changed: expected array→array→object with properties")
}
for k := range cell.Properties {
if _, ok := cellObjectKeys[k]; !ok {
t.Errorf("schema property %q missing from cellObjectKeys", k)
}
}
for k := range cellObjectKeys {
if _, ok := cell.Properties[k]; !ok {
t.Errorf("cellObjectKeys has %q which the schema no longer declares", k)
}
}
}
// TestCellsSet_LoneCellObjectAutoWraps runs the mounted path end-to-end: the
// eval-trace failure shape (--cells with a bare object) now dry-runs clean
// instead of failing "expected type array, got object".
func TestCellsSet_LoneCellObjectAutoWraps(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1",
"--cells", `{"value":"hello"}`,
"--dry-run",
})
if err != nil {
t.Fatalf("lone cell object should auto-wrap to [[cell]], got: %v", err)
}
if !strings.Contains(stdout, "hello") {
t.Errorf("dry-run body should carry the cell value, got %q", stdout)
}
}
// TestUnwrapCellsEnvelope pins the {"cells": …} envelope contract: a lone
// "cells" key is the flag name mistaken for a JSON key and unwraps; an
// object carrying siblings is the whole tool input and must not silently
// lose them.
func TestUnwrapCellsEnvelope(t *testing.T) {
t.Parallel()
cases := []struct {
name string
in string
unwrapped bool
}{
{"envelope around a 2D array", `{"cells":[[{"value":"a"}]]}`, true},
{"envelope around a lone cell object", `{"cells":{"value":"a"}}`, true},
{"envelope around an empty array", `{"cells":[]}`, true},
{"sibling keys stay (dropping them would write elsewhere)", `{"cells":[[{"value":"a"}]],"range":"A1"}`, false},
{"scalar under the key stays", `{"cells":"A1"}`, false},
{"unrelated object stays", `{"value":"a"}`, false},
{"proper 2D array stays", `[[{"value":"a"}]]`, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
var v interface{}
if err := json.Unmarshal([]byte(tc.in), &v); err != nil {
t.Fatalf("bad fixture: %v", err)
}
out := unwrapCellsEnvelope(v)
changed := fmt.Sprintf("%#v", out) != fmt.Sprintf("%#v", v)
if changed != tc.unwrapped {
t.Errorf("unwrapped=%v, want %v (got %#v)", changed, tc.unwrapped, out)
}
})
}
}
// TestScalarCellValue pins which bare cell-slot values lift into
// {"value": …}. null stays nil on purpose: {} and {"value":""} are both
// plausible readings, so it belongs to the validator, not the normalizer.
func TestScalarCellValue(t *testing.T) {
t.Parallel()
cases := []struct {
name string
in interface{}
lifted bool
}{
{"string", "hello", true},
{"number", float64(42), true},
{"bool", true, true},
{"json.Number", json.Number("42"), true},
{"null stays for the validator", nil, false},
{"cell object stays", map[string]interface{}{"value": "a"}, false},
{"array stays", []interface{}{"a"}, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
got := scalarCellValue(tc.in)
if (got != nil) != tc.lifted {
t.Fatalf("lifted=%v, want %v", got != nil, tc.lifted)
}
if tc.lifted && got["value"] != tc.in {
t.Errorf("want value %#v, got %#v", tc.in, got["value"])
}
})
}
}
// TestCellsSet_EnvelopeAndScalarCellsAccepted runs both new rewrites through
// the mounted path with the exact eval-trace shapes: a payload script's
// json.dump({"cells": cells}) envelope, and the openpyxl / gspread habit of
// a plain values matrix — which real rows MIX with cell objects as soon as a
// formula appears.
func TestCellsSet_EnvelopeAndScalarCellsAccepted(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set")
t.Run("envelope with a mixed scalar / object row", func(t *testing.T) {
t.Parallel()
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1:C1",
"--cells", `{"cells":[["电动大门",10331.00,{"formula":"=A1*2"}]]}`,
"--dry-run",
})
if err != nil {
t.Fatalf("envelope + scalar cells should normalize, got: %v", err)
}
for _, want := range []string{"电动大门", "10331", "=A1*2"} {
if !strings.Contains(stdout, want) {
t.Errorf("dry-run body should carry %s, got %q", want, stdout)
}
}
})
t.Run("null cell keeps the validation error", func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1",
"--cells", `[[null]]`,
"--dry-run",
})
requireValidation(t, err, `got "null"`)
})
t.Run("envelope with sibling keys keeps the validation error", func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1",
"--cells", `{"cells":[[{"value":"a"}]],"range":"A1"}`,
"--dry-run",
})
requireValidation(t, err, `expected type "array"`)
})
}
// TestCellsSetStyle_BorderWeightWordInStyleNormalizes pins the reachability
// fix for the border acceptance layer on the --border-styles flag path: the
// eval-trace failure shape ({"style":"thin"} — 07-28 root-cause report #2,
// 173 occurrences) must normalize to style:solid + weight:thin BEFORE the
// schema enum check, instead of dying on `value "thin" is not in enum`.
func TestCellsSetStyle_BorderWeightWordInStyleNormalizes(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set-style")
t.Run("full nested form with weight word in style", func(t *testing.T) {
t.Parallel()
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1:B2",
"--border-styles", `{"top":{"style":"thin","color":"#B4B4B4"},"bottom":{"style":"thin","color":"#B4B4B4"}}`,
"--dry-run",
})
if err != nil {
t.Fatalf("weight word in style slot should normalize, got: %v", err)
}
for _, want := range []string{`"style": "solid"`, `"weight": "thin"`} {
if !strings.Contains(stdout, want) {
t.Errorf("dry-run body should carry %s, got %q", want, stdout)
}
}
})
t.Run("all shorthand with weight word in style", func(t *testing.T) {
t.Parallel()
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1",
"--border-styles", `{"all":{"style":"medium","color":"#000000"}}`,
"--dry-run",
})
if err != nil {
t.Fatalf("all shorthand + weight word should normalize, got: %v", err)
}
for _, want := range []string{`"top"`, `"bottom"`, `"weight": "medium"`, `"style": "solid"`} {
if !strings.Contains(stdout, want) {
t.Errorf("dry-run body should carry %s, got %q", want, stdout)
}
}
})
t.Run("explicit conflicting weight keeps the enum error", func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1",
"--border-styles", `{"top":{"style":"thin","weight":"thick"}}`,
"--dry-run",
})
requireValidation(t, err, "not in enum")
})
}
// TestCellsSetStyle_BorderValueVocabularyNormalizes pins the two border
// value habits the 08-11 trace tally (596 traces) actually shows, beyond
// the weight-word-in-style half fixed on 07-28: openpyxl's "hair" (476
// hits / 19 tasks in the weight slot) and a thickness that arrives as a
// number (the 07-28 report's second failing shape). Everything else in
// openpyxl's line-style list scored zero and must STAY rejected — the
// last row pins that, so the table cannot quietly grow past its evidence.
func TestCellsSetStyle_BorderValueVocabularyNormalizes(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set-style")
cases := []struct {
name string
border string
want []string
wantErr string
}{
{name: "numeric width as string", border: `{"top":{"style":"solid","weight":"1"}}`,
want: []string{`"style": "solid"`, `"weight": "thin"`}},
{name: "numeric width as number", border: `{"top":{"style":"solid","weight":2}}`,
want: []string{`"style": "solid"`, `"weight": "medium"`}},
{name: "hair in the weight slot", border: `{"top":{"style":"solid","weight":"hair"}}`,
want: []string{`"weight": "thin"`}},
{name: "hair in the style slot", border: `{"all":{"style":"hair"}}`,
want: []string{`"style": "solid"`, `"weight": "thin"`}},
{name: "Google Sheets width key", border: `{"top":{"style":"solid","width":1}}`,
want: []string{`"weight": "thin"`}},
// The two ends of the width mapping: 3 is where thick starts, and a
// width the mapping deliberately does not guess at ("no width" is a
// border the caller should spell style:"none") keeps its own error.
{name: "numeric width at the thick boundary", border: `{"top":{"style":"solid","weight":3}}`,
want: []string{`"weight": "thick"`}},
{name: "zero width is not guessed at", border: `{"top":{"style":"solid","weight":0}}`,
wantErr: `expected type "string", got "number"`},
// strconv.ParseFloat answers yes to these; an infinite line width is
// not a width the caller meant, so it must not fold onto thick.
{name: "infinite width stays rejected", border: `{"top":{"style":"solid","weight":"Inf"}}`,
wantErr: "not in enum"},
{name: "unobserved line style stays rejected", border: `{"top":{"style":"dashDot"}}`,
wantErr: "not in enum"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1",
"--border-styles", tc.border,
"--dry-run",
})
if tc.wantErr != "" {
requireValidation(t, err, tc.wantErr)
return
}
if err != nil {
t.Fatalf("%s should normalize, got: %v", tc.border, err)
}
for _, want := range tc.want {
if !strings.Contains(stdout, want) {
t.Errorf("dry-run body should carry %s, got %q", want, stdout)
}
}
})
}
}
// TestCellsSet_BorderWeightWordInStyleNormalizes pins the same reachability
// fix on the typed --cells carrier (07-28 root-cause report #10, 58
// occurrences): border_styles inside a cell object normalizes before the
// enum check.
func TestCellsSet_BorderWeightWordInStyleNormalizes(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1",
"--cells", `[[{"value":"x","border_styles":{"top":{"style":"thin","color":"#000000"}}}]]`,
"--dry-run",
})
if err != nil {
t.Fatalf("weight word in style slot should normalize on --cells, got: %v", err)
}
for _, want := range []string{`"style": "solid"`, `"weight": "thin"`} {
if !strings.Contains(stdout, want) {
t.Errorf("dry-run body should carry %s, got %q", want, stdout)
}
}
}
// TestTablePut_SheetsDecodeHints pins the two decode-failure prescriptions:
// wrong JSON kind inlines the expected shape; mangled JSON steers to
// stdin/@file.
func TestTablePut_SheetsDecodeHints(t *testing.T) {
t.Parallel()
t.Run("type mismatch inlines skeleton", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+table-put")
// `data` as an object is a kind mismatch with no accepted reading —
// unlike object-form `columns`, which columnHeadings now decodes.
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheets", `{"sheets":[{"name":"s","columns":["a"],"data":{"0":["x"]}}]}`,
"--dry-run",
})
ve := requireValidation(t, err, "--sheets: invalid JSON")
for _, want := range []string{"expected shape:", `"columns":["City","Revenue"]`, `"dtypes":{"Revenue":"float64"}`} {
if !strings.Contains(ve.Hint, want) {
t.Errorf("hint should contain %q, got %q", want, ve.Hint)
}
}
})
t.Run("bare array is accepted as the sub-sheet list", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+table-put")
// The envelope is the only thing such a payload is missing, and an
// array at the top level can only be the list it would have held
// (07-28 root-cause report #4, 84 occurrences; still 10 in the
// 08-29..31 reflow after the error message was made explicit).
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheets", `[{"name":"s","columns":["a"],"data":[["x"]]}]`,
"--dry-run",
})
if err != nil {
t.Fatalf("bare sub-sheet list should be accepted, got %v", err)
}
if !strings.Contains(stdout, "set_cell_range") {
t.Errorf("dry-run should plan the write, got %q", stdout)
}
})
t.Run("bare scalar still names the expected shapes", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+table-put")
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheets", `"just a string"`,
"--dry-run",
})
ve := requireValidation(t, err, `top level must be the object {"sheets":[…]}`)
if strings.Contains(ve.Message, "cannot unmarshal") {
t.Errorf("message should not leak the Go unmarshal wording, got %q", ve.Message)
}
if !strings.Contains(ve.Hint, "expected shape:") {
t.Errorf("hint should still inline the skeleton, got %q", ve.Hint)
}
})
t.Run("syntax error steers to stdin or @file", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+table-put")
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheets", `{"sheets":[)`,
"--dry-run",
})
ve := requireValidation(t, err, "--sheets: invalid JSON")
for _, want := range []string{"stdin", "@./payload.json"} {
if !strings.Contains(ve.Hint, want) {
t.Errorf("hint should contain %q, got %q", want, ve.Hint)
}
}
})
}
// TestNormalizeChartHexColors pins the '#' prefixing on bare hex color
// values (eval V2U024: bars.color "4472C4" rejected server-side) and the
// pass-through of everything else, including the parseJSONFlag wiring for
// the batch sub-op path.
func TestNormalizeChartHexColors(t *testing.T) {
t.Parallel()
props := map[string]interface{}{
"plotArea": map[string]interface{}{
"plot": map[string]interface{}{
"series": []interface{}{
map[string]interface{}{"bars": map[string]interface{}{"color": "4472C4"}},
map[string]interface{}{"line": map[string]interface{}{"color": "#ED7D31"}},
map[string]interface{}{"area": map[string]interface{}{"color": "rgba(1,2,3,0.5)"}},
map[string]interface{}{"font_color": "ED7D31AA", "label": "not a color 4472C4"},
},
},
},
}
normalizeChartHexColors(props)
series := props["plotArea"].(map[string]interface{})["plot"].(map[string]interface{})["series"].([]interface{})
if got := series[0].(map[string]interface{})["bars"].(map[string]interface{})["color"]; got != "#4472C4" {
t.Errorf("bare hex should gain #, got %v", got)
}
if got := series[1].(map[string]interface{})["line"].(map[string]interface{})["color"]; got != "#ED7D31" {
t.Errorf("already-prefixed color must not change, got %v", got)
}
if got := series[2].(map[string]interface{})["area"].(map[string]interface{})["color"]; got != "rgba(1,2,3,0.5)" {
t.Errorf("rgba color must not change, got %v", got)
}
last := series[3].(map[string]interface{})
if got := last["font_color"]; got != "#ED7D31AA" {
t.Errorf("8-digit hex on a *_color key should gain #, got %v", got)
}
if got := last["label"]; got != "not a color 4472C4" {
t.Errorf("non-color key must not change, got %v", got)
}
// Wiring: a +chart-create sub-op style view routes through parseJSONFlag
// and picks up the normalizer.
fv := newMapFlagViewForCommand("+chart-create", map[string]interface{}{
"properties": map[string]interface{}{"title": map[string]interface{}{"font_color": "112233"}},
})
out, err := parseJSONFlag(fv, "properties")
if err != nil {
t.Fatalf("parseJSONFlag: %v", err)
}
title := out.(map[string]interface{})["title"].(map[string]interface{})
if title["font_color"] != "#112233" {
t.Errorf("parseJSONFlag should apply the chart color normalizer, got %v", title["font_color"])
}
}
Reference in New Issue View Git Blame Copy Permalink