mirror of
https://github.com/larksuite/cli.git
synced 2026-09-14 18:42:53 +08:00
xiongyuanwen-byted
835b52cc88
* feat(sheets): cut the top command-error clusters from the 08-18..24 eval batch
A trace analysis over 14,818 `lark-cli sheets` calls attributed 2,036 command
errors to 57 (subcommand, flag) groups, with the top 6 covering 76%. Four of
them are ours to fix; each is addressed at the layer that produced it.
--styles vocabulary (291 cases, the only group whose retry also failed):
- Prescribe the border family as a whole. foldBorderFamilyAliases already
absorbs border / borders / border_<side> / border_<attr>; what still
reached the error path was the Lark OpenAPI's border_type (FULL_BORDER,
OUTER_BORDER) and CSS's border_width — real vocabularies with no
equivalent here, and border_type was the single top field in the group.
Neither maps unambiguously onto a per-side style/weight/color triple, so
they get one shared answer, not a silent alias.
- Prescribe the OpenAPI's nested {range, style:{...}} envelope, plus
bg_color / fill_color / text_color.
- Match prescriptions on the key's letters alone, so border_type,
borderType and border-type are one mistake, not three.
- Collapse repeated issues in the --styles / --writes folds. One wrong
field name in a payload styling N cells produced N identical issues, each
re-listing the full supported vocabulary: the fold meant to save round
trips was burying its own answer. The defect is now stated once and the
other locations are named.
--sheets payload (419 cases):
- Accept dtypes / formats as a positional array. The same pandas habit that
produces `columns` and `data` produces df.dtypes.tolist(), and the
payloads were otherwise correct. Only a 1:1 match with `columns` is
accepted; a length mismatch is rejected rather than guessed.
+csv-put --file (35 cases):
- Read the value as a path. --file is aliased onto --csv because agents
reach for it, but the names promise different things — rewriting only the
name left the path to be written into the sheet as literal text, which the
file-path guard then rejected, with an error naming a flag the caller
never typed. The read goes through the same cmdutil.ReadInputFile as
@file, so the relative-path policy is unchanged and stdin stays the
out-of-tree route. A value naming nothing readable still falls through to
the guard, so --file holding literal CSV keeps working.
--help (88 cases of "required flag(s) ... not set"):
- Mark required flags in the sheets help. MarkFlagRequired only sets a
completion annotation cobra never renders, so a required flag read exactly
like an optional one. Sourced from flag-defs, since +chart-create and
+csv-put deliberately clear that annotation after mounting; a flag cobra
has put in a one-required group is left unmarked, because neither member
of such a pair is individually required.
The remaining big group (absolute paths passed to --file / @file, 636 cases)
is deliberately untouched: the cwd-relative policy is a protocol decision, and
that thread is being followed up separately.
* fix(sheets): correct --file alias provenance and mark its value resolved
Two defects in the +csv-put --file rule from the previous commit, both found
in review:
- The alias record was written from the flag-name normalizer, which pflag also
runs for Lookup and Set — including once with the canonical name right after
the rewrite, and again on every later lookup. `--file a.csv --csv ./b.csv`
therefore still counted as "supplied by --file", and an explicit --csv path
was silently read as a file instead of meeting its guard. The spelling is now
staged by the normalizer and committed by the flag's Value, which runs once
per real occurrence, so the last occurrence wins in either order.
- The rewritten value was not marked as read from a source, so a file whose
contents are themselves path-shaped ("report.csv") failed csvPutInput's shape
check — a valid CSV rejected as a caller who forgot the @. Marking it makes
--file behave exactly like --csv @<path> all the way down, which also lets
the guard skip it on its own rather than through a special case in Validate.
RuntimeContext gains an exported MarkInputResolved for the second half: the bit
already existed for @file / stdin, and a domain that resolves a source itself
needs to set it. No other domain calls it, so nothing else changes.
Also assert the typed contract (Param, Cause) rather than message text alone in
the style-prescription corpus and the collapsed-issue fold, per the repo's
error-test guideline.
* fix(sheets): answer every unreadable --file path under --file, and stage alias provenance only while parsing
Both from self-review of the branch.
An unreadable path passed as --file fell through to the --csv guard, which
answered naming a flag the caller never typed — and for a file that exists but
cannot be opened, prescribed "pass the same path with an @ prefix", which routes
through this very reader and fails identically. Only one case may fall through
now: a value that names nothing AND is not path-shaped, i.e. literal CSV text,
which --file accepted before this rule existed. A path-shaped value naming
nothing, an unreadable file, and a directory each answer under --file.
The alias spelling was staged with no Parsed() guard (the first commit had one;
flagalias.Bind still does). chainFlagAliases looks its aliases up while
installing and pflag normalizes on Lookup, so composing PostMount twice — which
installAliasProvenance explicitly anticipates — replayed "file" through the
already-installed normalizer at mount time, and the next real --csv occurrence
committed it: an explicit --csv path would then be read from disk instead of
meeting its guard. Verified the new regression test fails without the guard.
* fix(sheets): reset alias staging on remount, and assert cause on every --file read error
Second review pass, both valid.
FlagSet.Parsed() stays true once parsing has started, so the guard added in the
last commit only covers a remount that happens BEFORE the first parse. A remount
afterwards — its own alias lookups running through the normalizer the first pass
installed — could still leave a spelling staged for the next occurrence to
commit, which would read an explicit --csv path from disk. Re-running the
install now resets staging, closing the window from the other side. Verified the
new parse/remount/reparse test fails without the reset.
The unreadable-file and directory branches preserve the read error as Cause;
their tests now assert it, matching the sibling that already did.
67 lines
2.5 KiB
Go
67 lines
2.5 KiB
Go
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package common
|
|
|
|
import (
|
|
"context"
|
|
"sync"
|
|
|
|
"github.com/spf13/cobra"
|
|
|
|
"github.com/larksuite/cli/internal/cmdutil"
|
|
"github.com/larksuite/cli/internal/core"
|
|
)
|
|
|
|
// TestNewRuntimeContext creates a RuntimeContext for testing purposes.
|
|
// Only Cmd and Config are set; other fields (Factory, larkSDK, etc.) are nil.
|
|
func TestNewRuntimeContext(cmd *cobra.Command, cfg *core.CliConfig) *RuntimeContext {
|
|
return &RuntimeContext{Cmd: cmd, Config: cfg}
|
|
}
|
|
|
|
// TestNewRuntimeContextWithCtx creates a RuntimeContext with an explicit context
|
|
// for tests that invoke functions which call Ctx() (e.g. HTTP request helpers).
|
|
func TestNewRuntimeContextWithCtx(ctx context.Context, cmd *cobra.Command, cfg *core.CliConfig) *RuntimeContext {
|
|
return &RuntimeContext{ctx: ctx, Cmd: cmd, Config: cfg}
|
|
}
|
|
|
|
// TestNewRuntimeContextWithIdentity creates a RuntimeContext with a specific identity for testing.
|
|
func TestNewRuntimeContextWithIdentity(cmd *cobra.Command, cfg *core.CliConfig, as core.Identity) *RuntimeContext {
|
|
return &RuntimeContext{Cmd: cmd, Config: cfg, resolvedAs: as}
|
|
}
|
|
|
|
// TestNewRuntimeContextWithBotInfo creates a RuntimeContext with a pre-set BotInfo for testing.
|
|
func TestNewRuntimeContextWithBotInfo(cmd *cobra.Command, cfg *core.CliConfig, info *BotInfo) *RuntimeContext {
|
|
rctx := &RuntimeContext{Cmd: cmd, Config: cfg}
|
|
rctx.botInfoFunc = sync.OnceValues(func() (*BotInfo, error) {
|
|
return info, nil
|
|
})
|
|
return rctx
|
|
}
|
|
|
|
// TestMarkInputResolved marks a flag as resolved from @file / stdin, so
|
|
// domain tests can exercise guards that branch on InputResolvedFromSource
|
|
// without wiring the full resolveInputFlags path.
|
|
func TestMarkInputResolved(rctx *RuntimeContext, name string) {
|
|
rctx.MarkInputResolved(name)
|
|
}
|
|
|
|
// TestNewRuntimeContextForAPI creates a RuntimeContext ready for HTTP tests:
|
|
// sets Cmd, Config, Factory, context, and the requested identity so callers
|
|
// can invoke DoAPI / CallAPI directly without wiring through a cobra parent
|
|
// command.
|
|
//
|
|
// Pass core.AsBot or core.AsUser explicitly — exposing the identity as a
|
|
// parameter keeps the helper reusable for tests that need to exercise the
|
|
// user-identity code path (token store, auth login, etc.) without forking
|
|
// into a second near-identical helper.
|
|
func TestNewRuntimeContextForAPI(ctx context.Context, cmd *cobra.Command, cfg *core.CliConfig, f *cmdutil.Factory, as core.Identity) *RuntimeContext {
|
|
return &RuntimeContext{
|
|
ctx: ctx,
|
|
Cmd: cmd,
|
|
Config: cfg,
|
|
Factory: f,
|
|
resolvedAs: as,
|
|
}
|
|
}
|