mirror of
https://github.com/larksuite/cli.git
synced 2026-09-14 18:42:53 +08:00
R0bynZhu
46e2186adf
Assembling the --slides JSON array by hand is what callers keep getting
wrong. A page of SML is multi-line and quote-heavy, and shell has no
built-in way to JSON-escape it, so callers reached for `jq -n --rawfile`
to build the array. In environments without jq the substitution silently
became an empty string and the command ran on to create an empty deck,
or the half-escaped XML reached the backend and came back as an opaque
3350001 after the presentation already existed.
Two input forms remove the escaping step:
--slides now declares Input{file, stdin}, so a finished array can be
read with `--slides @deck.json` or piped in with `--slides -`.
--slide is repeatable, takes one complete <slide> document (or @path),
and the CLI assembles the array. Repetition order is page order.
The forms are mutually exclusive: merging them would make page order
depend on flag-parsing rules nobody wants to reason about.
Notes on the repeatable flag: the framework only resolves Flag.Input for
single-valued string flags, so --slide resolves @path itself, through
the same cmdutil.ReadInputFile the framework uses, keeping the
"relative path under the current directory" rule identical. It rejects
"-" outright, because a process has one stdin and that cannot mean "this
occurrence" on a repeatable flag; the error names both forms that work.
Structural validation now runs on the assembled array, so both forms
fail the same way, and it runs before the create call so a malformed
page can no longer leave an orphaned empty presentation behind.
Three inputs the first round of review found still slipping through are
now rejected or normalized before the create call. `--slides null` is
valid JSON for a slice, so it parsed without error and left the array
nil, which read as "no pages given" and produced the blank deck
reported as success that the empty-value check exists to prevent. An
`<?xml ...?>` prolog is well-formed XML, so the parser accepted it and
only the backend rejected it, with 4001000 buildSnNode, after the
presentation already existed; it is now caught in the shared slide
validator, which covers +add-slide and +replace-pages too. And a
leading UTF-8 BOM made `--slide @page.xml` reject a file that
`--slides @deck.json` accepted, because the framework strips it for
Input flags and the repeatable flag resolved @path itself;
StripUTF8BOM is exported so both paths normalize the same way.
Also threads the source flag name through uploadSlidesPlaceholders,
which previously reported +add-slide upload failures as --slides.
Docs: the create/troubleshooting references now teach the file inputs
instead of the jq array-building template, and the follow-up snippet
uses the CLI's own --jq instead of piping to an external jq.
The lint gate in SKILL.md now names `slides +create` as a whole rather
than only its --slide form.
29 lines
6.5 KiB
Markdown
29 lines
6.5 KiB
Markdown
# Slides CLI E2E Coverage
|
|
|
|
## Metrics
|
|
- Denominator: 6 leaf commands
|
|
- Covered: 5
|
|
- Coverage: 83.3%
|
|
|
|
## Summary
|
|
- TestSlides_CreateWorkflowAsUser: proves the user slides workflow through `create presentation with slide as user` and `get created presentation xml as user`; creates a fresh presentation, asserts returned IDs, then reads back the XML content to prove the title and slide body persisted.
|
|
- TestSlidesCreateRepeatedSlideFileDryRunE2E / TestSlidesCreateSlidesFileAndStdinDryRunE2E / TestSlidesCreateRejectsBothSlideFormsDryRunE2E: cover the `+create` file inputs through the built binary, which is the only layer that can prove the point of the feature — a multi-line, quote-heavy page reaches `slide.content` byte-for-byte with no JSON encoder in the caller's hands, and repeated `--slide` order is page order. The refusal case pins the exit-2 envelope (`param: "--slide"`) that an agent parses to repair its own command; the package test never runs the dispatcher and so cannot see it.
|
|
- TestSlidesAddSlideDryRunE2E / TestSlidesDeleteSlideDryRunE2E: pin the request shapes the unit tests cover, but through the real binary, which is the only layer that proves a full `<slide>` XML document survives flag parsing with its quotes and angle brackets intact. Delete additionally proves the shortcut runs without `--yes`, unlike the high-risk-write raw command.
|
|
- TestSlides_SlideAddDeleteWorkflowAsUser: live add/delete round trip on a throwaway presentation created and torn down per run. Asserts against a readback rather than the write's own response, so it proves what the request shape cannot: the returned `slide_id` addresses a real page, `--before-slide-id` positions the page between its neighbours instead of merely being forwarded, and the deleted page is gone while both neighbours survive.
|
|
- TestSlidesUpdateSlideDryRunE2E / TestSlidesUpdateAliasDryRunE2E / TestSlidesUpdateSlideRefuses*DryRunE2E: dry-run coverage for `+update-slide` through the built binary — one request carrying one `block_replace` part whose `block_id` is the PAGE id (the whole design; an element id there would replace one element and leave the rest of the page), the `slide` service alias / `+update` command alias / `--xml` spelling, and the two refusals that must not produce a request (a bare element root, and a root id naming a different page).
|
|
- TestSlidesUpdateSlideLiveE2E: required live-backend assertion for `+update-slide`; creates a throwaway presentation with the lane's bot credential, replaces its page using the returned page id as `block_id`, reads that page back through `+xml-get`, verifies the new marker replaced the old marker without changing `slide_id`, and self-cleans.
|
|
- TestSlides_HistoryWorkflow: opt-in live round-trip coverage for `+update-slide`; creates a presentation, updates its page in place, asserts the returned `slide_id`, the persisted marker, and that an element written back with its original id keeps that id, then reverts through slide history and self-cleans. It runs only when `LARK_SLIDES_HISTORY_E2E=1` and therefore does not yet exercise the default live lane.
|
|
- TestSlidesReplaceSlideUnknownFieldDryRunE2E / TestSlidesReplaceSlideEmptyReplacementDryRunE2E / TestSlidesReplaceSlideDryRunE2E: dry-run coverage for `+replace-slide` through the built binary. The package tests receive the Go error directly and never run the dispatcher, so only these pin the user-visible contract: exit code 2, an empty stdout, and a stderr envelope carrying `validation` / `invalid_argument`, `param: "--parts"` and the actionable `hint` — which is what an agent parses to repair its own command. The three cases split the two failures that must stay distinguishable (a wrong field name names the field and suggests the right one; an actually-empty payload keeps the `requires non-empty` wording) and prove the field whitelist still admits a legitimate mixed `block_replace` + `block_insert` batch with the block id injected.
|
|
- Blocked area: `slides +media-upload` is still uncovered because it needs a deterministic local image fixture plus XML follow-up proof that is separate from the base create/read workflow.
|
|
|
|
## Command Table
|
|
|
|
| Status | Cmd | Type | Testcase | Key parameter shapes | Notes / uncovered reason |
|
|
| --- | --- | --- | --- | --- | --- |
|
|
| ✓ | slides +create | shortcut | slides_create_workflow_test.go::TestSlides_CreateWorkflowAsUser/create presentation with slide as user; slides_create_slide_inputs_dryrun_test.go::TestSlidesCreateRepeatedSlideFileDryRunE2E, TestSlidesCreateSlidesFileAndStdinDryRunE2E, TestSlidesCreateRejectsBothSlideFormsDryRunE2E | `--title`; `--slides ["<slide ...>"]` / `--slides @deck.json` / `--slides -`; repeated `--slide @page.xml` | live lane reads back through raw slides API to prove persisted XML; dry-run lane pins the file inputs and the mutual-exclusion refusal |
|
|
| ✓ | slides +add-slide | shortcut | slides_slide_add_delete_dryrun_test.go::TestSlidesAddSlideDryRunE2E; slides_slide_add_delete_workflow_test.go::TestSlides_SlideAddDeleteWorkflowAsUser | `--slide "<slide ...>"`; `--before-slide-id`; `--revision-id` | live append and insert both verified by reading the deck back |
|
|
| ✓ | slides +delete-slide | shortcut | slides_slide_add_delete_dryrun_test.go::TestSlidesDeleteSlideDryRunE2E, TestSlidesDeleteSlideWikiDryRunE2E; slides_slide_add_delete_workflow_test.go::TestSlides_SlideAddDeleteWorkflowAsUser | `--slide-id`; `--revision-id`; wiki URL | live delete runs on a throwaway deck; readback proves the neighbours survive |
|
|
| ✓ | slides +update-slide | shortcut | slides_update_slide_dryrun_test.go::TestSlidesUpdateSlideDryRunE2E (+ alias / refusal cases); slides_update_slide_workflow_test.go::TestSlidesUpdateSlideLiveE2E; slides_history_workflow_test.go::TestSlides_HistoryWorkflow (opt-in history/revert) | `--presentation`; `--slide-id`; `--content "<slide ...>"`; `--revision-id` | default live CI proves a real in-place update and readback; opt-in workflow adds history/revert and element-id preservation |
|
|
| ◐ | slides +replace-slide | shortcut | slides_replace_slide_dryrun_test.go::TestSlidesReplaceSlideUnknownFieldDryRunE2E, TestSlidesReplaceSlideEmptyReplacementDryRunE2E, TestSlidesReplaceSlideDryRunE2E | `--presentation`; `--slide-id`; `--parts '[{action,block_id,replacement}]'` / `'[{action,insertion,insert_before_block_id}]'` | dry-run only; covers the rejected-field envelope, the empty-payload split and an accepted mixed batch. No live lane yet: a real round trip needs a throwaway deck plus a readback proving only the named block moved |
|
|
| ✕ | slides +media-upload | shortcut | | none | needs a stable local image fixture plus follow-up slide XML proof |
|