Feat/cli e2e tests with UAT#528
Merged
liangshuo-1 merged 2 commits intolarksuite:mainfrom Apr 17, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request expands CLI E2E test coverage across multiple domains (calendar, contact, docs, drive, IM, mail, sheets, slides, task, wiki) by introducing user-focused workflows, updating existing bot tests with explicit identity markers, refactoring helpers to support dual-context operations, enhancing core test utilities with cleanup handlers and user-token validation, and adding coverage documentation for each domain. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
the
size/S
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/cli_e2e/task/tasklist_workflow_test.go (1)
50-78:⚠️ Potential issue | 🟠 MajorUse a bounded context for E2E cleanup.
Both cleanup paths call the CLI with
context.Background(). If a delete request hangs, the whole suite waits forever instead of failing fast.Suggested pattern
- deleteResult, deleteErr := clie2e.RunCmd(context.Background(), clie2e.Request{ + cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + deleteResult, deleteErr := clie2e.RunCmd(cleanupCtx, clie2e.Request{Based on learnings, live E2E tests must be self-contained with create/use/cleanup flows and placed in
tests/cli_e2e/<domain>/.Also applies to: 148-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/task/tasklist_workflow_test.go` around lines 50 - 78, The cleanup goroutines call clie2e.RunCmd using context.Background(), which can hang indefinitely; update both parentT.Cleanup closures (the ones that call clie2e.RunCmd for "task tasks delete" and "task tasklists delete" referencing taskGUID and tasklistGUID) to create a bounded context (e.g., via context.WithTimeout or context.WithDeadline) and pass that context into clie2e.RunCmd, cancel the context when done, and handle the timeout error in the existing deleteErr check; apply the same change to the other cleanup block mentioned around lines 148-165 that also invokes clie2e.RunCmd so all E2E cleanup calls use a timeout-bound context.tests/cli_e2e/docs/docs_create_fetch_test.go (1)
35-47:⚠️ Potential issue | 🟠 MajorKeep the bot fetch step on bot identity.
This subtest dropped
DefaultAs: "bot", so the renamed bot workflow now depends on whatever auth state happens to be active on the runner. That makes the create step bot-scoped but the fetch step ambient-scoped.🤖 Suggested fix
result, err := clie2e.RunCmd(ctx, clie2e.Request{ Args: []string{ "docs", "+fetch", "--doc", docToken, }, + DefaultAs: "bot", })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/docs/docs_create_fetch_test.go` around lines 35 - 47, The fetch subtest dropped the bot identity so it runs with ambient auth; update the clie2e.Request passed to clie2e.RunCmd in the "fetch" t.Run to include DefaultAs: "bot" (i.e., set clie2e.Request{Args: ..., DefaultAs: "bot"}) so the fetch is executed under the same bot scope as the create step; this change should be made in the t.Run("fetch", func(t *testing.T) { ... }) block that uses docToken and clie2e.RunCmd.
🧹 Nitpick comments (4)
tests/cli_e2e/docs/coverage.md (1)
14-16: Add a blank line before the table to fix markdown lint warning.The markdownlint tool flags MD058 because the table should be surrounded by blank lines. Add a blank line after line 14 (before the "## Command Table" heading or before the table itself).
📝 Proposed fix
- Blocked area: media and search shortcuts still need deterministic fixtures and local file orchestration. + ## Command Table🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/docs/coverage.md` around lines 14 - 16, Add a blank line before the table to satisfy markdownlint MD058: insert an empty line between the "## Command Table" heading and the start of the pipe table (the line beginning with "| Status | Cmd | Type | Testcase | Key parameter shapes | Notes / uncovered reason |") so the table is surrounded by blank lines.tests/cli_e2e/im/coverage.md (1)
16-18: Add a blank line before the table to fix markdown lint warning.Same MD058 issue - add a blank line before the Command Table section.
📝 Proposed fix
- Blocked area: `im +chat-search` did not reliably return freshly created private chats in UAT, and `im +messages-search` did not reliably index freshly sent messages in time for a deterministic read-after-write assertion, so both remain uncovered. + ## Command Table🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/im/coverage.md` around lines 16 - 18, The markdown linter flags MD058 because the table immediately follows the "## Command Table" heading; add a single blank line between the "## Command Table" heading and the start of the table (the line beginning with "| Status | Cmd | Type | Testcase | ...") so the file tests/cli_e2e/im/coverage.md has an empty line after the "Command Table" header.tests/cli_e2e/contact/coverage.md (1)
12-14: Add a blank line before the table to fix markdown lint warning.Same MD058 issue as in
docs/coverage.md- the table needs a blank line before it.📝 Proposed fix
- Blocked area: `contact +search-user` did not reliably return the current user in UAT even when queried with self-derived identifiers, so it remains uncovered rather than being counted from a flaky tenant-dependent assertion. + ## Command Table🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/contact/coverage.md` around lines 12 - 14, Add a blank line immediately before the markdown table that follows the "## Command Table" heading to satisfy MD058; locate the "## Command Table" heading (or the table starting with "| Status | Cmd | Type | Testcase | Key parameter shapes | Notes / uncovered reason |") and insert one empty line between the heading and the table so the table is separated from the heading.tests/cli_e2e/core.go (1)
225-226: Usevfs.*for artifact directory creation.
ArtifactDirintroduces new filesystem writes viaos.MkdirAll, which bypasses the repo’s mockable filesystem layer.As per coding guidelines, "Use
vfs.*functions instead ofos.*for all filesystem access to enable test mocking".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/core.go` around lines 225 - 226, In ArtifactDir replace the direct filesystem call os.MkdirAll(dir, 0o755) with the mockable vfs equivalent (vfs.MkdirAll or the project's vfs helper) and keep the same error handling (t.Fatalf message) so tests use the repo's virtual filesystem; also add or adjust the import to include the vfs package used by the test helper if not already present and remove the os usage for directory creation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/cli_e2e/calendar/calendar_user_workflow_test.go`:
- Around line 31-44: The freebusy subtest only checks that "data" exists and
doesn't verify RSVP changes; update the "query freebusy as user" t.Run (and the
sibling +freebusy subtests in the same file) to parse the returned freebusy
blocks from clie2e.RunCmd's result (use gjson.Get(result.Stdout, "data") or
similar) and locate the specific window/block created earlier, then assert the
block's status transitions (e.g., from busy -> tentative -> accepted) after each
RSVP action (use the same identifiers used when creating the window/event) so
the test fails if RSVP does not change availability as expected.
In `@tests/cli_e2e/calendar/calendar_view_agenda_test.go`:
- Around line 23-45: The tests calling clie2e.RunCmd with
clie2e.Request{DefaultAs: "user"} only check success but not that the request
was executed as that identity; update both subtests to parse result.Stdout (via
gjson.Get(result.Stdout, "...")) to assert the returned identity/actor equals
"user" (or the expected user id/name) in the response JSON in addition to
existing assertions so that DefaultAs is actually validated; reference
clie2e.RunCmd, clie2e.Request, DefaultAs, result.Stdout and gjson.Get to locate
and add the identity assertion.
In `@tests/cli_e2e/calendar/coverage.md`:
- Around line 17-42: The Command Table in coverage.md is not surrounded by blank
lines (causing MD058); edit tests/cli_e2e/calendar/coverage.md and add a blank
line immediately before the "## Command Table" table start and another blank
line immediately after the table end (i.e., surround the entire pipe-table block
that follows the "## Command Table" heading with one blank line above and below)
so the Markdown linter no longer flags MD058.
In `@tests/cli_e2e/core.go`:
- Around line 38-40: The auth probe is running with the local default identity
because RunCmd no longer mutates identity; update the Request passed to RunCmd
(the Request struct used at the call site where Args: []string{"auth", "status",
"--verify"}) to include DefaultAs: "user" so the probe is forced to run as user;
apply the same change to the other call sites that match the pattern (the
similar Request at the other location referenced, previously lines 56-58).
- Around line 261-265: The cleanup currently calls RunCmd inside parentT.Cleanup
and ignores its returned error/exit status, so update the parentT.Cleanup block
to capture the result from RunCmd(Request{Args: []string{"drive", "+delete",
"--file-token", folderToken, "--type", "folder"}, DefaultAs: defaultAs}) and
check for errors or non-zero exit; if RunCmd returns an error or indicates
failure, call parentT.Fatalf or parentT.Errorf (or fail the test) reporting the
command, args and the error/output so folder-cleanup failures are surfaced
instead of swallowed. Ensure you reference the same Request, Args, folderToken
and defaultAs symbols when adding the check.
In `@tests/cli_e2e/docs/helpers_test.go`:
- Around line 20-35: The helper currently creates a doc with clie2e.RunCmd using
clie2e.Request.DefaultAs but never asserts the returned identity; update the
test to extract the created identity from result.Stdout (e.g. with
gjson.Get(result.Stdout, "data.identity").String()) after obtaining docToken and
add a require.Equal(t, defaultAs, identity) (or require.NotEmpty +
require.EqualValues) to verify the create ran as the requested DefaultAs; keep
existing docToken extraction and other assertions intact and fail the test if
the identity differs.
- Around line 36-46: The cleanup registered with parentT.Cleanup calls
clie2e.RunCmd using context.Background() and ignores the returned error/exit
code, which can hang or silently fail; change the cleanup to use a
cancellable/timeout context (e.g., context.WithTimeout) and capture the result
of clie2e.RunCmd (the deleteErr / exit code) for assertion or logging, and
handle failures (fail the test or log and attempt best-effort retry) using the
same docToken/defaultAs parameters so cleanup failures are visible and bounded
rather than swallowed.
In `@tests/cli_e2e/drive/coverage.md`:
- Around line 13-16: The markdown lint error (MD058) is due to the table being
immediately adjacent to the heading; add a single blank line between the "##
Command Table" heading and the table start so the heading and the table are
separated. Locate the "## Command Table" heading and insert one empty line
before the table's first row (the "| Status | Cmd | Type | Testcase | Key
parameter shapes | Notes / uncovered reason |" line) to satisfy the linter.
In `@tests/cli_e2e/im/chat_message_workflow_test.go`:
- Around line 38-64: The test does an immediate call to clie2e.RunCmd to list
messages which can flake; change this to poll until the sent message appears by
using clie2e.RunCmdWithRetry (or implement a small retry loop) in the "list chat
messages as user" subtest: repeatedly call clie2e.RunCmdWithRetry/RunCmd inside
a bounded timeout (e.g. few seconds) and break when gjson.Get(result.Stdout,
"data.messages") contains the expected message_id (messageID) whose content
contains messageText; keep the same assertions (result.AssertExitCode,
result.AssertStdoutStatus) on success and fail the test if the timeout is
exceeded.
In `@tests/cli_e2e/im/message_reply_workflow_test.go`:
- Around line 47-85: The test currently does single-run calls via clie2e.RunCmd
to fetch the original message (listResult) and thread messages (threadResult),
causing intermittent failures due to eventual consistency; replace those single
calls with clie2e.RunCmdWithRetry (or wrap the list/threads logic in a retry
loop) so you repeatedly call the listing logic until threadID is non-empty and
the replyText is found in the threads response. Specifically, retry the block
that obtains threadID from gjson.Get(listResult.Stdout, "data.messages") and the
subsequent threads lookup (Args: {"im", "+threads-messages-list", "--thread",
threadID}) until both require.NotEmpty(threadID, ...) and the check that
strings.Contains(item.Get("content").String(), replyText) succeed, then proceed
with the existing assertions.
In `@tests/cli_e2e/mail/coverage.md`:
- Around line 13-78: The "## Command Table" section in coverage.md is not
separated by blank lines which triggers MD058; add a single blank line
immediately before the "## Command Table" heading and another blank line
immediately after the end of the table (after the final row) so the table is
isolated from surrounding text; locate the table by the "## Command Table"
heading and the pipe-delimited rows and insert the blank lines around that
block.
In `@tests/cli_e2e/mail/mail_self_send_workflow_test.go`:
- Around line 39-41: The cleanup closure is using the test's main ctx which may
be expired at teardown; change the parentT.Cleanup closures (where replyDraftID
and other resources are cleaned) to create and use a fresh context (e.g.,
ctxCleanup := context.Background() or ctxCleanup, cancel :=
context.WithTimeout(context.Background(), time.Second*30); defer cancel()) and
pass that ctxCleanup into clie2e.RunCmd and any other cleanup calls instead of
the original ctx; update every cleanup block (including the other occurrences
around the replyDraftID and the blocks at the commented lines 57-58 and 83-84)
to use this new context so cleanup runs reliably even if the main ctx has timed
out.
In `@tests/cli_e2e/sheets/coverage.md`:
- Around line 15-43: The Markdown table under the heading "## Command Table" is
not separated by blank lines and triggers markdownlint-cli2; fix by adding a
blank line above and below the table so the table is isolated from the
heading/body — update the area that contains the table rows (the block starting
with "| Status | Cmd | Type | Testcase | Key parameter shapes | Notes /
uncovered reason |") to include one empty line before the first table line and
one empty line after the last table line.
In `@tests/cli_e2e/sheets/sheets_user_workflow_test.go`:
- Around line 28-45: After calling clie2e.RunCmd in the "+info as user" test,
add an assertion that the CLI actually ran as the "user" identity by extracting
the actor/identity from the command response (use result and gjson.Get on
result.Stdout) and asserting it equals "user" (e.g. assert.Equal(t, "user",
gjson.Get(result.Stdout, "data.request.auth.actor").String())). This ensures
clie2e.RunCmd (and the DefaultAs: "user" setting) is honored for the sheets+info
invocation that uses spreadsheetToken.
In `@tests/cli_e2e/slides/coverage.md`:
- Around line 12-16: Insert a blank line immediately before the table that
begins at the "## Command Table" header and another blank line immediately after
the table block so there is an empty line separating the header and table and an
empty line following the table; this will satisfy markdownlint MD058 by ensuring
there is an empty line before and after the table block.
In `@tests/cli_e2e/task/coverage.md`:
- Around line 17-18: Update the "Blocked area" note in
tests/cli_e2e/task/coverage.md to reflect that the suite is not exclusively
bot-safe anymore: change the wording that currently says "the current suite is
bot-safe only" to acknowledge that TestTask_UpdateWorkflow and
TestTask_TasklistWorkflowAsUser are user-scoped; keep the rest of the note about
remaining user-oriented shortcuts and the need for deterministic user fixtures
for assignee/follower/tasklist member mutations intact, but remove or replace
the incorrect claim about being bot-safe only and mention user-scoped coverage
by referencing the two tests by name.
In `@tests/cli_e2e/task/task_update_workflow_test.go`:
- Around line 164-165: After patching the task summary, update all subsequent
+get-my-tasks invocations to use the patched summary variable instead of the
original taskSummary; replace occurrences of "--query", taskSummary with
"--query", patchedTaskSummary in the test (look for variables named taskSummary
and patchedTaskSummary and the CLI calls that assemble "--query" arguments) so
the assertions target the updated task text. This change should be applied to
the later calls that currently still use taskSummary (also at the other
indicated occurrences).
In `@tests/cli_e2e/wiki/coverage.md`:
- Around line 12-20: The Markdown table under the "## Command Table" header
violates MD058 because it is not separated by blank lines; fix it by inserting a
blank line immediately before the table (between the header and the first | row)
and another blank line immediately after the final table row so the table block
is isolated; update the contents in tests/cli_e2e/wiki/coverage.md around the
"## Command Table" section to add those blank lines.
In `@tests/cli_e2e/wiki/helpers_test.go`:
- Around line 103-133: The pagination loop in findWikiNodeByToken trusts the API
to advance page_token and can spin if the same token repeats; restore cycle
protection by tracking seen page tokens (e.g., a map or set) and aborting with a
clear error if the next page_token was already observed; modify the loop to
record the initial pageToken value before each request, check
parsed.Get("data.page_token").String() against the seen set, and t.Fatalf if
it's a repeat (or when you detect a loop) to prevent infinite looping while
searching for nodeToken.
In `@tests/cli_e2e/wiki/wiki_workflow_test.go`:
- Around line 119-120: The test currently does unsafe panicking type assertions
when extracting copiedNodeToken and copiedSpaceID; instead parse the response
once and safely extract fields the same way other tests do (gjson-style), verify
that "data" and "data.node" exist before reading, and only assign
copiedNodeToken and copiedSpaceID after confirming the values are strings (use
ok checks or type assertions with existence checks rather than direct .(string)
casts on result.StdoutJSON(t)). Locate the extraction around the variables
copiedNodeToken / copiedSpaceID and replace the duplicate direct casts with a
single safe extraction and presence/type checks for "data.node.node_token" and
"data.node.space_id".
---
Outside diff comments:
In `@tests/cli_e2e/docs/docs_create_fetch_test.go`:
- Around line 35-47: The fetch subtest dropped the bot identity so it runs with
ambient auth; update the clie2e.Request passed to clie2e.RunCmd in the "fetch"
t.Run to include DefaultAs: "bot" (i.e., set clie2e.Request{Args: ...,
DefaultAs: "bot"}) so the fetch is executed under the same bot scope as the
create step; this change should be made in the t.Run("fetch", func(t *testing.T)
{ ... }) block that uses docToken and clie2e.RunCmd.
In `@tests/cli_e2e/task/tasklist_workflow_test.go`:
- Around line 50-78: The cleanup goroutines call clie2e.RunCmd using
context.Background(), which can hang indefinitely; update both parentT.Cleanup
closures (the ones that call clie2e.RunCmd for "task tasks delete" and "task
tasklists delete" referencing taskGUID and tasklistGUID) to create a bounded
context (e.g., via context.WithTimeout or context.WithDeadline) and pass that
context into clie2e.RunCmd, cancel the context when done, and handle the timeout
error in the existing deleteErr check; apply the same change to the other
cleanup block mentioned around lines 148-165 that also invokes clie2e.RunCmd so
all E2E cleanup calls use a timeout-bound context.
---
Nitpick comments:
In `@tests/cli_e2e/contact/coverage.md`:
- Around line 12-14: Add a blank line immediately before the markdown table that
follows the "## Command Table" heading to satisfy MD058; locate the "## Command
Table" heading (or the table starting with "| Status | Cmd | Type | Testcase |
Key parameter shapes | Notes / uncovered reason |") and insert one empty line
between the heading and the table so the table is separated from the heading.
In `@tests/cli_e2e/core.go`:
- Around line 225-226: In ArtifactDir replace the direct filesystem call
os.MkdirAll(dir, 0o755) with the mockable vfs equivalent (vfs.MkdirAll or the
project's vfs helper) and keep the same error handling (t.Fatalf message) so
tests use the repo's virtual filesystem; also add or adjust the import to
include the vfs package used by the test helper if not already present and
remove the os usage for directory creation.
In `@tests/cli_e2e/docs/coverage.md`:
- Around line 14-16: Add a blank line before the table to satisfy markdownlint
MD058: insert an empty line between the "## Command Table" heading and the start
of the pipe table (the line beginning with "| Status | Cmd | Type | Testcase |
Key parameter shapes | Notes / uncovered reason |") so the table is surrounded
by blank lines.
In `@tests/cli_e2e/im/coverage.md`:
- Around line 16-18: The markdown linter flags MD058 because the table
immediately follows the "## Command Table" heading; add a single blank line
between the "## Command Table" heading and the start of the table (the line
beginning with "| Status | Cmd | Type | Testcase | ...") so the file
tests/cli_e2e/im/coverage.md has an empty line after the "Command Table" header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2a1eb49-7764-4054-8c36-d5ab9ad123f5
📒 Files selected for processing (52)
.github/workflows/cli-e2e.ymltests/cli_e2e/base/base_basic_workflow_test.gotests/cli_e2e/base/base_role_workflow_test.gotests/cli_e2e/base/coverage.mdtests/cli_e2e/calendar/calendar_create_event_test.gotests/cli_e2e/calendar/calendar_manage_calendar_test.gotests/cli_e2e/calendar/calendar_personal_event_workflow_test.gotests/cli_e2e/calendar/calendar_user_workflow_test.gotests/cli_e2e/calendar/calendar_view_agenda_test.gotests/cli_e2e/calendar/coverage.mdtests/cli_e2e/calendar/helpers_test.gotests/cli_e2e/contact/contact_lookup_workflow_test.gotests/cli_e2e/contact/contact_shortcut_test.gotests/cli_e2e/contact/coverage.mdtests/cli_e2e/core.gotests/cli_e2e/core_test.gotests/cli_e2e/demo/task_lifecycle_test.gotests/cli_e2e/docs/coverage.mdtests/cli_e2e/docs/docs_create_fetch_test.gotests/cli_e2e/docs/docs_update_test.gotests/cli_e2e/docs/helpers_test.gotests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_files_workflow_test.gotests/cli_e2e/drive/helpers_test.gotests/cli_e2e/im/chat_message_workflow_test.gotests/cli_e2e/im/chat_workflow_test.gotests/cli_e2e/im/coverage.mdtests/cli_e2e/im/helpers_test.gotests/cli_e2e/im/message_get_workflow_test.gotests/cli_e2e/im/message_reply_workflow_test.gotests/cli_e2e/im/message_workflow_test.gotests/cli_e2e/mail/coverage.mdtests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.gotests/cli_e2e/mail/mail_self_send_workflow_test.gotests/cli_e2e/sheets/coverage.mdtests/cli_e2e/sheets/helpers_test.gotests/cli_e2e/sheets/sheets_crud_workflow_test.gotests/cli_e2e/sheets/sheets_filter_workflow_test.gotests/cli_e2e/sheets/sheets_user_workflow_test.gotests/cli_e2e/slides/coverage.mdtests/cli_e2e/slides/slides_create_workflow_test.gotests/cli_e2e/task/coverage.mdtests/cli_e2e/task/helpers_test.gotests/cli_e2e/task/task_comment_workflow_test.gotests/cli_e2e/task/task_reminder_workflow_test.gotests/cli_e2e/task/task_status_workflow_test.gotests/cli_e2e/task/task_update_workflow_test.gotests/cli_e2e/task/tasklist_add_task_workflow_test.gotests/cli_e2e/task/tasklist_workflow_test.gotests/cli_e2e/wiki/coverage.mdtests/cli_e2e/wiki/helpers_test.gotests/cli_e2e/wiki/wiki_workflow_test.go
💤 Files with no reviewable changes (2)
- tests/cli_e2e/im/message_workflow_test.go
- tests/cli_e2e/contact/contact_shortcut_test.go
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
tests/cli_e2e/sheets/sheets_create_workflow_test.go (1)
35-44:⚠️ Potential issue | 🟠 MajorAssert the command ran as
user, not just that it succeeded.This test proves success, but not identity routing. If
DefaultAs: "user"is ignored, the test can still pass under another identity.Suggested assertion
require.NoError(t, err) result.AssertExitCode(t, 0) result.AssertStdoutStatus(t, true) + assert.Equal(t, "user", gjson.Get(result.Stdout, "data.request.auth.actor").String(), "stdout:\n%s", result.Stdout) assert.Equal(t, spreadsheetToken, gjson.Get(result.Stdout, "data.spreadsheet.spreadsheet.token").String())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/sheets/sheets_create_workflow_test.go` around lines 35 - 44, The test currently verifies success but not that the command ran with the requested identity; update the test around clie2e.RunCmd and the clie2e.Request{DefaultAs: "user"} call to assert the executed identity by inspecting result.Stdout (use gjson.Get on the response JSON) and comparing the identity field to "user" (e.g., check the appropriate metadata/requester field that echoes the caller identity), so the test fails if DefaultAs is ignored.tests/cli_e2e/core.go (1)
38-40:⚠️ Potential issue | 🟠 MajorForce user identity in the auth verification probe.
Line 38 currently runs
auth status --verifywith whichever identity is locally default, which can incorrectly skip user-scoped E2E tests.Minimal fix
result, err := RunCmd(ctx, Request{ - Args: []string{"auth", "status", "--verify"}, + Args: []string{"auth", "status", "--verify"}, + DefaultAs: "user", })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/core.go` around lines 38 - 40, The auth verification call using RunCmd with Request.Args currently relies on the local default identity; update the Args passed to RunCmd in tests/cli_e2e/core.go (the Request struct used in the RunCmd(...) call) to explicitly force a user identity by adding the identity flag and value (e.g., include "--identity" and the user identity token or name) so the probe always runs under a user-scoped identity rather than the local default.tests/cli_e2e/task/coverage.md (1)
17-17:⚠️ Potential issue | 🟡 MinorUpdate the blocked-area wording to reflect current user coverage.
Line 17 says the suite is “bot-safe only,” but this file now documents user-scoped coverage (e.g.,
TestTask_UpdateWorkflow,TestTask_TasklistWorkflowAsUser).Suggested doc fix
-- Blocked area: assignee, follower, and tasklist member mutations still require stable real-user `open_id` fixtures; the current suite is bot-safe only. +- Blocked area: assignee, follower, and tasklist member mutations still require stable real-user `open_id` fixtures; the suite now includes both bot- and user-scoped coverage, but these collaborator mutations still need deterministic fixtures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/task/coverage.md` at line 17, Update the blocked-area sentence in tests/cli_e2e/task/coverage.md to reflect that the suite now includes user-scoped coverage (e.g., TestTask_UpdateWorkflow and TestTask_TasklistWorkflowAsUser) rather than being “bot-safe only”; keep the remaining caveat that assignee, follower, and tasklist member mutations still require stable real-user open_id fixtures. Mention the two test names (TestTask_UpdateWorkflow, TestTask_TasklistWorkflowAsUser) to make the new coverage explicit and retain the warning about needing real-user fixtures for those specific mutation types.
🧹 Nitpick comments (1)
tests/cli_e2e/mail/mail_send_workflow_test.go (1)
262-339: Add local precondition guards for dependent subtests.Several late steps (reply/forward/inspect) assume IDs from prior subtests; adding
require.NotEmpty(...)at each step will prevent cascade noise and make failures easier to triage.Example guard additions
t.Run("reply to received message with shortcut as user", func(t *testing.T) { + require.NotEmpty(t, inboxMessageID, "inbox message id should be available before reply") result, err := clie2e.RunCmd(ctx, clie2e.Request{ ... }) ... }) t.Run("inspect reply draft as user", func(t *testing.T) { + require.NotEmpty(t, replyDraftID, "reply draft id should be available before inspect") result, err := clie2e.RunCmd(ctx, clie2e.Request{ ... }) ... }) t.Run("forward received message with shortcut as user", func(t *testing.T) { + require.NotEmpty(t, inboxMessageID, "inbox message id should be available before forward") result, err := clie2e.RunCmd(ctx, clie2e.Request{ ... }) ... }) t.Run("inspect forward draft as user", func(t *testing.T) { + require.NotEmpty(t, forwardDraftID, "forward draft id should be available before inspect") result, err := clie2e.RunCmd(ctx, clie2e.Request{ ... }) ... })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/mail/mail_send_workflow_test.go` around lines 262 - 339, Each dependent subtest should guard required IDs before use: add require.NotEmpty checks for inboxMessageID (before the "+reply" and "+forward" runs), for replyDraftID (before the "inspect reply draft" run), for forwardDraftID (before the "inspect forward draft" run), and for mailboxID where "+draft-edit" is invoked; place the checks at the start of the corresponding t.Run blocks that reference those symbols so failures stop early and give clear diagnostics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/cli_e2e/im/chat_message_workflow_test.go`:
- Around line 63-66: Add an explicit assertion that the command ran as the
"user" actor to catch regressions in DefaultAs; after the existing
require.NoError(t, err) and result.AssertExitCode/AssertStdoutStatus calls,
decode the command output or payload from the test's result (use the same
variable named result) and assert the actor identity equals "user" (e.g., check
payload.Actor or the JSON field that records actor). Use the test's
require/Assert helpers to fail the test if the actor is not "user".
In `@tests/cli_e2e/task/helpers_test.go`:
- Around line 34-37: The cleanup commands call clie2e.RunCmd with
Request.DefaultAs = "bot" which mismatches resources created with req.DefaultAs
= "user", causing auth failures; update the delete calls that build the Request
(used for task deletion at the clie2e.RunCmd invocation around taskGUID and the
similar one at lines ~64-67) to reuse the creator identity instead of hardcoding
"bot" — e.g., read the original request's DefaultAs (or store the creator
identity in a variable when creating the resource) and set Request.DefaultAs to
that value so cleanup runs as the same identity that created the resource.
---
Duplicate comments:
In `@tests/cli_e2e/core.go`:
- Around line 38-40: The auth verification call using RunCmd with Request.Args
currently relies on the local default identity; update the Args passed to RunCmd
in tests/cli_e2e/core.go (the Request struct used in the RunCmd(...) call) to
explicitly force a user identity by adding the identity flag and value (e.g.,
include "--identity" and the user identity token or name) so the probe always
runs under a user-scoped identity rather than the local default.
In `@tests/cli_e2e/sheets/sheets_create_workflow_test.go`:
- Around line 35-44: The test currently verifies success but not that the
command ran with the requested identity; update the test around clie2e.RunCmd
and the clie2e.Request{DefaultAs: "user"} call to assert the executed identity
by inspecting result.Stdout (use gjson.Get on the response JSON) and comparing
the identity field to "user" (e.g., check the appropriate metadata/requester
field that echoes the caller identity), so the test fails if DefaultAs is
ignored.
In `@tests/cli_e2e/task/coverage.md`:
- Line 17: Update the blocked-area sentence in tests/cli_e2e/task/coverage.md to
reflect that the suite now includes user-scoped coverage (e.g.,
TestTask_UpdateWorkflow and TestTask_TasklistWorkflowAsUser) rather than being
“bot-safe only”; keep the remaining caveat that assignee, follower, and tasklist
member mutations still require stable real-user open_id fixtures. Mention the
two test names (TestTask_UpdateWorkflow, TestTask_TasklistWorkflowAsUser) to
make the new coverage explicit and retain the warning about needing real-user
fixtures for those specific mutation types.
---
Nitpick comments:
In `@tests/cli_e2e/mail/mail_send_workflow_test.go`:
- Around line 262-339: Each dependent subtest should guard required IDs before
use: add require.NotEmpty checks for inboxMessageID (before the "+reply" and
"+forward" runs), for replyDraftID (before the "inspect reply draft" run), for
forwardDraftID (before the "inspect forward draft" run), and for mailboxID where
"+draft-edit" is invoked; place the checks at the start of the corresponding
t.Run blocks that reference those symbols so failures stop early and give clear
diagnostics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9006df0d-f080-4f4d-8309-008579f48975
📒 Files selected for processing (32)
tests/cli_e2e/calendar/calendar_personal_event_workflow_test.gotests/cli_e2e/calendar/calendar_rsvp_workflow_test.gotests/cli_e2e/calendar/coverage.mdtests/cli_e2e/contact/coverage.mdtests/cli_e2e/core.gotests/cli_e2e/demo/task_lifecycle_test.gotests/cli_e2e/docs/coverage.mdtests/cli_e2e/docs/docs_create_fetch_test.gotests/cli_e2e/docs/docs_update_test.gotests/cli_e2e/docs/helpers_test.gotests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/helpers.gotests/cli_e2e/drive/helpers_test.gotests/cli_e2e/im/chat_message_workflow_test.gotests/cli_e2e/im/coverage.mdtests/cli_e2e/im/message_reply_workflow_test.gotests/cli_e2e/mail/coverage.mdtests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.gotests/cli_e2e/mail/mail_send_workflow_test.gotests/cli_e2e/sheets/coverage.mdtests/cli_e2e/sheets/helpers_test.gotests/cli_e2e/sheets/sheets_create_workflow_test.gotests/cli_e2e/sheets/sheets_crud_workflow_test.gotests/cli_e2e/slides/coverage.mdtests/cli_e2e/slides/slides_create_workflow_test.gotests/cli_e2e/task/coverage.mdtests/cli_e2e/task/helpers_test.gotests/cli_e2e/task/task_update_workflow_test.gotests/cli_e2e/task/tasklist_workflow_test.gotests/cli_e2e/wiki/coverage.mdtests/cli_e2e/wiki/helpers_test.gotests/cli_e2e/wiki/wiki_workflow_test.go
✅ Files skipped from review due to trivial changes (7)
- tests/cli_e2e/slides/coverage.md
- tests/cli_e2e/wiki/coverage.md
- tests/cli_e2e/docs/coverage.md
- tests/cli_e2e/contact/coverage.md
- tests/cli_e2e/calendar/coverage.md
- tests/cli_e2e/drive/coverage.md
- tests/cli_e2e/drive/helpers_test.go
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/cli_e2e/sheets/helpers_test.go
- tests/cli_e2e/docs/docs_update_test.go
- tests/cli_e2e/calendar/calendar_personal_event_workflow_test.go
- tests/cli_e2e/docs/docs_create_fetch_test.go
- tests/cli_e2e/task/tasklist_workflow_test.go
- tests/cli_e2e/slides/slides_create_workflow_test.go
- tests/cli_e2e/wiki/wiki_workflow_test.go
- tests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.go
- tests/cli_e2e/sheets/sheets_crud_workflow_test.go
- tests/cli_e2e/docs/helpers_test.go
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/cli_e2e/calendar/calendar_rsvp_workflow_test.go (2)
127-140: Harden freebusy verification against eventual consistency.The two “verify ... freebusy” subtests do immediate read-after-write checks; in live calendar systems this can be briefly stale and cause flakes. Consider polling with a short timeout/interval before failing.
Suggested direction
- requireFreebusyEntry(t, result.Stdout, startAt, endAt, "tentative") + require.Eventually(t, func() bool { + verifyResult, verifyErr := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{"calendar", "+freebusy", "--start", startTime, "--end", endTime}, + DefaultAs: "user", + }) + if verifyErr != nil || verifyResult.ExitCode != 0 || !gjson.Get(verifyResult.Stdout, "status").Bool() { + return false + } + return freebusyEntryExists(verifyResult.Stdout, startAt, endAt, "tentative") + }, 20*time.Second, 2*time.Second, "freebusy did not reflect tentative RSVP in time")Also applies to: 160-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/calendar/calendar_rsvp_workflow_test.go` around lines 127 - 140, The immediate read-after-write checks in the "verify tentative freebusy as user" (and the other similar subtest) can flake due to eventual consistency; change the test to poll: wrap the clie2e.RunCmd + requireFreebusyEntry call in a retry loop that runs clie2e.RunCmd repeatedly (e.g., every 200–500ms) until requireFreebusyEntry succeeds or a short timeout elapses (e.g., 5–10s), and fail only after the timeout; apply the same polling pattern to the other verify freebusy subtest and reference the clie2e.RunCmd invocation and requireFreebusyEntry helper to perform the check inside the loop.
142-149: Add the same precondition guard used in the tentative step.
reply accept as usershould also asserteventIDis present before issuing RSVP, so failures stay localized and clearer.Small consistency patch
t.Run("reply accept as user", func(t *testing.T) { + require.NotEmpty(t, eventID, "event should be created before RSVP") + result, err := clie2e.RunCmd(ctx, clie2e.Request{ Args: []string{ "calendar", "+rsvp",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/calendar/calendar_rsvp_workflow_test.go` around lines 142 - 149, Add the same precondition guard used in the tentative step to the "reply accept as user" test: before calling clie2e.RunCmd, assert that the eventID variable is present (non-empty) and fail or skip the test if it's missing so failures remain localized; update the test block for "reply accept as user" (which calls clie2e.RunCmd) to check eventID and call t.Fatalf or t.Skip with a clear message when eventID == "".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cli-e2e.yml:
- Around line 39-40: The TEST_USER_ACCESS_TOKEN is currently set at job scope
and should be moved to the specific test step to minimize exposure; remove
TEST_USER_ACCESS_TOKEN from the job-level env block and add it under the env
section of the test step (e.g., the step that runs the CLI E2E tests) as
TEST_USER_ACCESS_TOKEN: ${{ secrets.TEST_USER_ACCESS_TOKEN }} so only that step
receives the secret.
- Around line 67-73: Fail-fast: add an explicit guard that checks the
TEST_USER_ACCESS_TOKEN before running the test commands so missing user token
causes the workflow to exit non-zero instead of silently skipping tests. Locate
the CI run block that exports LARKSUITE_CLI_USER_ACCESS_TOKEN from
env.TEST_USER_ACCESS_TOKEN and add a check for an empty TEST_USER_ACCESS_TOKEN
(or LARKSUITE_CLI_USER_ACCESS_TOKEN) immediately before creating artifact dirs /
computing packages; if empty, print a clear error message and exit 1 to stop the
job and surface the problem.
---
Nitpick comments:
In `@tests/cli_e2e/calendar/calendar_rsvp_workflow_test.go`:
- Around line 127-140: The immediate read-after-write checks in the "verify
tentative freebusy as user" (and the other similar subtest) can flake due to
eventual consistency; change the test to poll: wrap the clie2e.RunCmd +
requireFreebusyEntry call in a retry loop that runs clie2e.RunCmd repeatedly
(e.g., every 200–500ms) until requireFreebusyEntry succeeds or a short timeout
elapses (e.g., 5–10s), and fail only after the timeout; apply the same polling
pattern to the other verify freebusy subtest and reference the clie2e.RunCmd
invocation and requireFreebusyEntry helper to perform the check inside the loop.
- Around line 142-149: Add the same precondition guard used in the tentative
step to the "reply accept as user" test: before calling clie2e.RunCmd, assert
that the eventID variable is present (non-empty) and fail or skip the test if
it's missing so failures remain localized; update the test block for "reply
accept as user" (which calls clie2e.RunCmd) to check eventID and call t.Fatalf
or t.Skip with a clear message when eventID == "".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2dac8401-1259-495f-9097-3655a3ea96d0
📒 Files selected for processing (3)
.github/workflows/cli-e2e.ymltests/cli_e2e/calendar/calendar_rsvp_workflow_test.gotests/cli_e2e/drive/helpers.go
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/cli_e2e/drive/helpers.go
yxzhaao
force-pushed
the
feat/cli-e2e-tests-for-user
branch
from
819bf21 to
d755500
Compare
github-actions
bot
added
size/M
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
tests/cli_e2e/docs/docs_create_fetch_test.go (1)
18-19: Stale doc comment after rename.The comment still references
TestDocs_CreateAndFetchWorkflowand doesn't reflect the new bot/user split. Consider updating it (and adding one for the newAsUservariant) to describe the identity context being exercised.📝 Suggested tweak
-// TestDocs_CreateAndFetchWorkflow tests the create and fetch lifecycle. +// TestDocs_CreateAndFetchWorkflowAsBot tests the docs create/fetch lifecycle using bot access. func TestDocs_CreateAndFetchWorkflowAsBot(t *testing.T) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/docs/docs_create_fetch_test.go` around lines 18 - 19, The test comment above TestDocs_CreateAndFetchWorkflowAsBot is stale (still mentions TestDocs_CreateAndFetchWorkflow) — update the doc comment to explicitly state this test exercises the create-and-fetch workflow as a bot (i.e., bot identity/context), and add or update the corresponding comment for TestDocs_CreateAndFetchWorkflowAsUser to state it exercises the same workflow as a user; locate the comments near the test function declarations (TestDocs_CreateAndFetchWorkflowAsBot and TestDocs_CreateAndFetchWorkflowAsUser) and make them accurate and descriptive of the identity context being tested.tests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.go (1)
119-119: Nit: document themessage_state == 3magic number.Consider adding a brief comment describing what state
3represents (e.g., "draft"), so the assertion intent is obvious to future readers and survives any enum renumbering audit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.go` at line 119, The assertion uses the magic number 3 for message_state (assert.Equal(t, int64(3), gjson.Get(result.Stdout, "data.draft.message.message_state").Int()))—replace the bare literal with either a named constant (e.g., MessageStateDraft or whatever enum exists) or, if no constant is available, add a one-line comment after the assertion explaining that 3 == "draft" so future readers understand the intent and to help detect enum renumbering; reference the JSON field "data.draft.message.message_state" when making the change.tests/cli_e2e/sheets/helpers_test.go (1)
40-49: UseRunCmdWithRetryfor cleanup consistency.The sibling
drivee2e.CreateDriveFolder(seetests/cli_e2e/drive/helpers.goL47-L51) usesclie2e.RunCmdWithRetryfor its cleanupdrive +deletecall, which is more robust against transient failures during teardown. This helper uses plainRunCmd, so a single network blip will leak the spreadsheet resource. Consider aligning to match the retry pattern used across the other suites.♻️ Proposed fix
- deleteResult, deleteErr := clie2e.RunCmd(cleanupCtx, clie2e.Request{ - Args: []string{ - "drive", "+delete", - "--file-token", spreadsheetToken, - "--type", "sheet", - "--yes", - }, - DefaultAs: defaultAs, - }) + deleteResult, deleteErr := clie2e.RunCmdWithRetry(cleanupCtx, clie2e.Request{ + Args: []string{ + "drive", "+delete", + "--file-token", spreadsheetToken, + "--type", "sheet", + "--yes", + }, + DefaultAs: defaultAs, + }, clie2e.RetryOptions{})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/sheets/helpers_test.go` around lines 40 - 49, The cleanup call currently uses clie2e.RunCmd which can leak resources on transient failures; change the call in the cleanup block to use clie2e.RunCmdWithRetry with the same cleanupCtx and clie2e.Request (preserving Args, DefaultAs and spreadsheetToken) and keep passing the returned result and error into clie2e.ReportCleanupFailure (leave cleanupCtx, cleanup description, and defaultAs unchanged). Ensure the signature matches RunCmdWithRetry’s return values and retry behavior consistent with drivee2e.CreateDriveFolder.tests/cli_e2e/drive/coverage.md (1)
1-44: Coverage documentation is clear and well-structured.The metrics are accurate (1/28 ≈ 3.6%), the summary provides helpful context about cleanup exclusions, and the command table clearly maps test coverage. The blank line between the heading and table (line 14) properly addresses MD058.
Optional: The markdown linter flags line 23 because the table lacks a trailing blank line at EOF. Adding a blank line after line 44 would satisfy MD058 completely, though many projects don't enforce trailing newlines in markdown.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/drive/coverage.md` around lines 1 - 44, The markdown file "Drive CLI E2E Coverage" is missing a trailing blank line which triggers MD058; open the coverage.md document (the "Drive CLI E2E Coverage" header and the Command Table block) and add a single blank newline at the end of the file (after the table/last line) so the file ends with an empty line to satisfy the markdown linter.tests/cli_e2e/docs/coverage.md (1)
1-26: Comprehensive coverage documentation with clear bot/user workflow distinctions.The metrics are accurate (3/8 = 37.5%), and the summary helpfully documents both bot and user test variants. The blank line at line 16 correctly separates the heading from the table, and the command table clearly maps shortcuts to specific test subtests with parameter shapes.
Optional: Similar to the drive file, the markdown linter flags line 23 due to a missing trailing blank line at EOF (after line 26). Adding one would fully satisfy MD058, though it's stylistic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/docs/coverage.md` around lines 1 - 26, The markdown is missing a trailing blank line at EOF which trips MD058; open the docs coverage.md and add a single newline at the end of the file (after the final table/row such as the "docs +whiteboard-update" entry) so the file ends with a blank line, then re-run the linter to confirm MD058 is resolved.tests/cli_e2e/task/task_update_workflow_test.go (1)
17-47: MoveSkipWithoutUserTokenabove local setup to avoid unnecessary work on skip.
SkipWithoutUserToken(t)on Line 31 runs after suffix/variable setup and aftert.Cleanup(cancel)registration. Functionally correct (cancel is idempotent, no GUID so cleanup no-ops), but moving the skip check to immediately afterparentT := tavoids theauth status --verifysubprocess + suffix allocation on skip and better matches the pattern inTestTask_TasklistWorkflowAsUser.Suggested reorder
func TestTask_UpdateWorkflow(t *testing.T) { parentT := t + clie2e.SkipWithoutUserToken(t) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) t.Cleanup(cancel) @@ taskGUID := "" - - clie2e.SkipWithoutUserToken(t)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/task/task_update_workflow_test.go` around lines 17 - 47, Move the SkipWithoutUserToken check to the top of TestTask_UpdateWorkflow to avoid doing setup when the test will be skipped: call clie2e.SkipWithoutUserToken(t) immediately after parentT := t (before creating the context, registering t.Cleanup(cancel), and allocating suffix/summary/description variables) so no subprocess auth verification or suffix allocation runs on skip; keep the existing cleanup closure and variable names (taskGUID, suffix, taskSummary, etc.) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/cli_e2e/base/coverage.md`:
- Around line 14-16: The Markdown heading "## Command Table" is immediately
followed by a table which violates MD058; insert a single blank line between the
heading (the line containing "## Command Table") and the table header row ("|
Status | Cmd | Type | Testcase | Key parameter shapes | Notes / uncovered reason
|") so the file tests/cli_e2e/base/coverage.md has an empty line after the
heading to satisfy the linter.
In `@tests/cli_e2e/calendar/calendar_rsvp_workflow_test.go`:
- Around line 127-140: The freebusy assertion can be flaky because the backend
projection may lag; replace the direct single-call check in the "verify
tentative freebusy as user" subtest (which currently uses clie2e.RunCmd +
requireFreebusyEntry) with a short polling loop using require.Eventually (or
equivalent) that retries clie2e.RunCmd and calls requireFreebusyEntry until
success, with a total timeout of ~10–15s and ~500ms backoff; apply the same
change to the similar verification at the other block referenced (lines 160-173)
so both tests tolerate propagation delay.
- Around line 88-104: Register the cleanup immediately after extracting the
event identifier from the create response and before asserting with
require.NotEmpty to avoid leaking events when the response lacks data.event_id;
move or add the parentT.Cleanup call right after eventID :=
gjson.Get(result.Stdout, "data.event_id").String() and inside the cleanup
callback guard the delete by checking eventID != "" before calling clie2e.RunCmd
(the delete invocation that uses "calendar events delete" and params
calendar_id/event_id), then run the require.NotEmpty(t, eventID, ...) assertion
after the cleanup registration.
In `@tests/cli_e2e/im/coverage.md`:
- Line 28: The table row for the im +messages-search case uses the incorrect
phrase "write-after-read"; update that cell to "read-after-write" to match the
wording used earlier and in the +messages-reply row so the assertion pattern is
consistent (look for the row containing "+messages-search" and change
"write-after-read" → "read-after-write").
In `@tests/cli_e2e/task/tasklist_workflow_test.go`:
- Around line 205-220: The test "list tasklists and find patched tasklist as
user" is flaky because it only requests a single page with page_size:50 and may
miss the newly patched tasklist on busy accounts; update the test around
clie2e.RunCmd (Args: []string{"task","tasklists","list"}) to reliably find the
item by either requesting a deterministic sort/filter (e.g., add params for
sort_by=created_at_desc or filter=name==patchedTasklistName if API supports it)
or implement paging: call clie2e.RunCmd repeatedly using page_token from the
previous response until the item with guid == tasklistGUID (or name ==
patchedTasklistName) is found or no more pages; use tasklistGUID and
patchedTasklistName as the matching keys and assert after the loop, ensuring
require.NoError/AssertExitCode checks still run on each call.
In `@tests/cli_e2e/wiki/wiki_workflow_test.go`:
- Around line 90-94: The test "list spaces as bot" erroneously asserts
data.page_token which is only returned when has_more=true; update the test
around listWikiSpaces(t, ctx, 1) by removing the assert.True for
result.Get("data.page_token") and keep only the assert for
result.Get("data.items"), or alternatively seed multiple spaces beforehand (call
your space-creation helper, e.g., createWikiSpace or similar setup function) so
that listing with page_size=1 returns has_more=true and page_token is present;
update the test accordingly (modify the t.Run block containing listWikiSpaces).
---
Nitpick comments:
In `@tests/cli_e2e/docs/coverage.md`:
- Around line 1-26: The markdown is missing a trailing blank line at EOF which
trips MD058; open the docs coverage.md and add a single newline at the end of
the file (after the final table/row such as the "docs +whiteboard-update" entry)
so the file ends with a blank line, then re-run the linter to confirm MD058 is
resolved.
In `@tests/cli_e2e/docs/docs_create_fetch_test.go`:
- Around line 18-19: The test comment above TestDocs_CreateAndFetchWorkflowAsBot
is stale (still mentions TestDocs_CreateAndFetchWorkflow) — update the doc
comment to explicitly state this test exercises the create-and-fetch workflow as
a bot (i.e., bot identity/context), and add or update the corresponding comment
for TestDocs_CreateAndFetchWorkflowAsUser to state it exercises the same
workflow as a user; locate the comments near the test function declarations
(TestDocs_CreateAndFetchWorkflowAsBot and TestDocs_CreateAndFetchWorkflowAsUser)
and make them accurate and descriptive of the identity context being tested.
In `@tests/cli_e2e/drive/coverage.md`:
- Around line 1-44: The markdown file "Drive CLI E2E Coverage" is missing a
trailing blank line which triggers MD058; open the coverage.md document (the
"Drive CLI E2E Coverage" header and the Command Table block) and add a single
blank newline at the end of the file (after the table/last line) so the file
ends with an empty line to satisfy the markdown linter.
In `@tests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.go`:
- Line 119: The assertion uses the magic number 3 for message_state
(assert.Equal(t, int64(3), gjson.Get(result.Stdout,
"data.draft.message.message_state").Int()))—replace the bare literal with either
a named constant (e.g., MessageStateDraft or whatever enum exists) or, if no
constant is available, add a one-line comment after the assertion explaining
that 3 == "draft" so future readers understand the intent and to help detect
enum renumbering; reference the JSON field "data.draft.message.message_state"
when making the change.
In `@tests/cli_e2e/sheets/helpers_test.go`:
- Around line 40-49: The cleanup call currently uses clie2e.RunCmd which can
leak resources on transient failures; change the call in the cleanup block to
use clie2e.RunCmdWithRetry with the same cleanupCtx and clie2e.Request
(preserving Args, DefaultAs and spreadsheetToken) and keep passing the returned
result and error into clie2e.ReportCleanupFailure (leave cleanupCtx, cleanup
description, and defaultAs unchanged). Ensure the signature matches
RunCmdWithRetry’s return values and retry behavior consistent with
drivee2e.CreateDriveFolder.
In `@tests/cli_e2e/task/task_update_workflow_test.go`:
- Around line 17-47: Move the SkipWithoutUserToken check to the top of
TestTask_UpdateWorkflow to avoid doing setup when the test will be skipped: call
clie2e.SkipWithoutUserToken(t) immediately after parentT := t (before creating
the context, registering t.Cleanup(cancel), and allocating
suffix/summary/description variables) so no subprocess auth verification or
suffix allocation runs on skip; keep the existing cleanup closure and variable
names (taskGUID, suffix, taskSummary, etc.) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f813fb95-3fc1-4e9d-a1a4-ed49caee5a98
📒 Files selected for processing (53)
skills/lark-sheets/SKILL.mdtests/cli_e2e/base/base_basic_workflow_test.gotests/cli_e2e/base/base_role_workflow_test.gotests/cli_e2e/base/coverage.mdtests/cli_e2e/calendar/calendar_create_event_test.gotests/cli_e2e/calendar/calendar_manage_calendar_test.gotests/cli_e2e/calendar/calendar_personal_event_workflow_test.gotests/cli_e2e/calendar/calendar_rsvp_workflow_test.gotests/cli_e2e/calendar/calendar_view_agenda_test.gotests/cli_e2e/calendar/coverage.mdtests/cli_e2e/calendar/helpers_test.gotests/cli_e2e/contact/contact_lookup_workflow_test.gotests/cli_e2e/contact/contact_shortcut_test.gotests/cli_e2e/contact/coverage.mdtests/cli_e2e/core.gotests/cli_e2e/core_test.gotests/cli_e2e/demo/task_lifecycle_test.gotests/cli_e2e/docs/coverage.mdtests/cli_e2e/docs/docs_create_fetch_test.gotests/cli_e2e/docs/docs_update_test.gotests/cli_e2e/docs/helpers_test.gotests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_files_workflow_test.gotests/cli_e2e/drive/helpers.gotests/cli_e2e/drive/helpers_test.gotests/cli_e2e/im/chat_message_workflow_test.gotests/cli_e2e/im/chat_workflow_test.gotests/cli_e2e/im/coverage.mdtests/cli_e2e/im/helpers_test.gotests/cli_e2e/im/message_get_workflow_test.gotests/cli_e2e/im/message_reply_workflow_test.gotests/cli_e2e/im/message_workflow_test.gotests/cli_e2e/mail/coverage.mdtests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.gotests/cli_e2e/mail/mail_send_workflow_test.gotests/cli_e2e/sheets/coverage.mdtests/cli_e2e/sheets/helpers_test.gotests/cli_e2e/sheets/sheets_create_workflow_test.gotests/cli_e2e/sheets/sheets_crud_workflow_test.gotests/cli_e2e/sheets/sheets_filter_workflow_test.gotests/cli_e2e/slides/coverage.mdtests/cli_e2e/slides/slides_create_workflow_test.gotests/cli_e2e/task/coverage.mdtests/cli_e2e/task/helpers_test.gotests/cli_e2e/task/task_comment_workflow_test.gotests/cli_e2e/task/task_reminder_workflow_test.gotests/cli_e2e/task/task_status_workflow_test.gotests/cli_e2e/task/task_update_workflow_test.gotests/cli_e2e/task/tasklist_add_task_workflow_test.gotests/cli_e2e/task/tasklist_workflow_test.gotests/cli_e2e/wiki/coverage.mdtests/cli_e2e/wiki/helpers_test.gotests/cli_e2e/wiki/wiki_workflow_test.go
💤 Files with no reviewable changes (3)
- skills/lark-sheets/SKILL.md
- tests/cli_e2e/im/message_workflow_test.go
- tests/cli_e2e/contact/contact_shortcut_test.go
✅ Files skipped from review due to trivial changes (14)
- tests/cli_e2e/base/base_role_workflow_test.go
- tests/cli_e2e/calendar/calendar_create_event_test.go
- tests/cli_e2e/task/tasklist_add_task_workflow_test.go
- tests/cli_e2e/task/task_reminder_workflow_test.go
- tests/cli_e2e/calendar/calendar_view_agenda_test.go
- tests/cli_e2e/base/base_basic_workflow_test.go
- tests/cli_e2e/wiki/coverage.md
- tests/cli_e2e/demo/task_lifecycle_test.go
- tests/cli_e2e/contact/coverage.md
- tests/cli_e2e/slides/coverage.md
- tests/cli_e2e/im/message_get_workflow_test.go
- tests/cli_e2e/sheets/sheets_filter_workflow_test.go
- tests/cli_e2e/calendar/calendar_manage_calendar_test.go
- tests/cli_e2e/wiki/helpers_test.go
🚧 Files skipped from review as they are similar to previous changes (17)
- tests/cli_e2e/calendar/helpers_test.go
- tests/cli_e2e/drive/helpers_test.go
- tests/cli_e2e/drive/drive_files_workflow_test.go
- tests/cli_e2e/sheets/sheets_create_workflow_test.go
- tests/cli_e2e/docs/docs_update_test.go
- tests/cli_e2e/task/task_comment_workflow_test.go
- tests/cli_e2e/im/message_reply_workflow_test.go
- tests/cli_e2e/sheets/sheets_crud_workflow_test.go
- tests/cli_e2e/core_test.go
- tests/cli_e2e/im/chat_message_workflow_test.go
- tests/cli_e2e/calendar/calendar_personal_event_workflow_test.go
- tests/cli_e2e/core.go
- tests/cli_e2e/slides/slides_create_workflow_test.go
- tests/cli_e2e/drive/helpers.go
- tests/cli_e2e/mail/mail_send_workflow_test.go
- tests/cli_e2e/task/task_status_workflow_test.go
- tests/cli_e2e/docs/helpers_test.go
yxzhaao
force-pushed
the
feat/cli-e2e-tests-for-user
branch
from
d755500 to
f58dc34
Compare
github-actions
bot
added
size/S
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/cli_e2e/base/coverage.md (1)
14-15:⚠️ Potential issue | 🟡 MinorAdd the missing blank line before the command table.
Line 15 directly follows the heading on Line 14; this still violates MD058 (
blanks-around-tables).📝 Proposed fix
## Command Table + | Status | Cmd | Type | Testcase | Key parameter shapes | Notes / uncovered reason |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/base/coverage.md` around lines 14 - 15, Add a blank line between the heading "## Command Table" and the table that starts with "| Status | Cmd" to satisfy MD058 (blanks-around-tables); locate the heading "## Command Table" and insert one empty line before the table markdown so the table is separated from its heading.
🧹 Nitpick comments (6)
tests/cli_e2e/sheets/helpers_test.go (1)
40-49: ConsiderRunCmdWithRetryfor cleanup parity.
CreateDriveFoldercleans up its folder withclie2e.RunCmdWithRetry(seetests/cli_e2e/drive/helpers.go:48-50), but this spreadsheet cleanup uses plainRunCmd. Transient API blips during teardown will leak spreadsheets here while the folder delete would retry. Aligning them keeps cleanup resiliency consistent across the suite.♻️ Proposed diff
- deleteResult, deleteErr := clie2e.RunCmd(cleanupCtx, clie2e.Request{ - Args: []string{ - "drive", "+delete", - "--file-token", spreadsheetToken, - "--type", "sheet", - "--yes", - }, - DefaultAs: defaultAs, - }) + deleteResult, deleteErr := clie2e.RunCmdWithRetry(cleanupCtx, clie2e.Request{ + Args: []string{ + "drive", "+delete", + "--file-token", spreadsheetToken, + "--type", "sheet", + "--yes", + }, + DefaultAs: defaultAs, + }, clie2e.RetryOptions{})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/sheets/helpers_test.go` around lines 40 - 49, The cleanup call uses clie2e.RunCmd which doesn't retry on transient failures; replace this call with clie2e.RunCmdWithRetry using the same cleanupCtx and clie2e.Request (same Args and DefaultAs) so the delete gets retried on transient errors, then pass its result and error into clie2e.ReportCleanupFailure as before; locate the call to clie2e.RunCmd in tests/cli_e2e/sheets/helpers_test.go and swap it to clie2e.RunCmdWithRetry (keeping deleteResult/deleteErr and the ReportCleanupFailure invocation intact).tests/cli_e2e/slides/slides_create_workflow_test.go (1)
28-38: Preferjson.Marshalover manual escaping for--slides.
strings.ReplaceAll(slideXML,",")only escapes double quotes — it doesn't handle backslashes, newlines, or other JSON-reserved characters. The current payload happens to be safe, but if the slide template ever gains an attribute value or body containing\,\n, or a tab, the assembled--slideswill be invalid JSON and the test will fail opaquely. Usingencoding/jsonavoids that fragility.♻️ Proposed refactor
-import ( - "context" - "strings" - "testing" - "time" - - clie2e "github.com/larksuite/cli/tests/cli_e2e" - "github.com/stretchr/testify/require" - "github.com/tidwall/gjson" -) +import ( + "context" + "encoding/json" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) ... - "--slides", `["` + strings.ReplaceAll(slideXML, `"`, `\"`) + `"]`, + "--slides", mustJSON(t, []string{slideXML}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/slides/slides_create_workflow_test.go` around lines 28 - 38, The test builds the --slides arg by manually escaping quotes which misses other JSON-unsafe characters; replace the strings.ReplaceAll(...) construction that builds the slides JSON with a safe json.Marshal of a []string{slideXML} (import encoding/json) and use the resulting marshaled string as the "--slides" value passed into clie2e.RunCmd (the clie2e.Request.Args slice) so all characters (quotes, backslashes, newlines) are correctly escaped; locate the slideXML variable and the RunCmd/clie2e.Request.Args usage to make this change.tests/cli_e2e/mail/mail_send_workflow_test.go (2)
131-175: Break out of the inner loop once a distinct inbox message is found.In the attempt loop, the inner
for _, item := range messagesiterates every matching entry and unconditionally overwritesinboxMessageIDfor each non-sent match. If the triage response happens to include multiple non-sent matches (e.g., due to eventual-consistency duplicates), the last one wins nondeterministically. Safer to break as soon as a distinct id is captured.♻️ Proposed refinement
for _, item := range messages { if item.Get("subject").String() != subject { continue } messageID := item.Get("message_id").String() if messageID == "" { continue } if messageID != sentMessageID { inboxMessageID = messageID + break } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/mail/mail_send_workflow_test.go` around lines 131 - 175, The inner loop over messages currently may overwrite inboxMessageID multiple times; change the loop in the triage check so that as soon as you capture a distinct inbox message ID (when item.Get("message_id").String() != "" and != sentMessageID) you assign inboxMessageID and break out of the inner for loop (so the first distinct match is kept), leaving the outer attempt loop logic unchanged; refer to the variables/messages loop, inboxMessageID and sentMessageID when making this change.
17-22: Consider callingSkipWithoutUserTokenbefore registering the context cancel.Minor ordering nit:
clie2e.SkipWithoutUserToken(t)is invoked aftercontext.WithTimeout(...)andt.Cleanup(cancel). If the test is skipped (the common CI path when no user token is configured), we still pay for creating the context + registering the cleanup. Not a correctness issue, just consistent with the slides test below where skip is the second line. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/mail/mail_send_workflow_test.go` around lines 17 - 22, Move the test skip call earlier so the test exits before allocating resources: call clie2e.SkipWithoutUserToken(t) as the first line inside TestMail_SendWorkflowAsUser (before creating ctx via context.WithTimeout and before registering t.Cleanup(cancel)) so you avoid creating the context and cleanup when the test will be skipped; update the order around clie2e.SkipWithoutUserToken, context.WithTimeout, and t.Cleanup to reflect this.tests/cli_e2e/mail/coverage.md (1)
9-11: Minor grammar nit: hyphenate compound modifiers.Static analysis flags phrases like "self sent mail", "self send workflow", and "self send thread". When these are noun compound modifiers they should be hyphenated ("self-sent mail", "self-send workflow"). Since these strings double as
t.Runsubtest names echoed in the doc, fixing both here and inmail_send_workflow_test.gokeeps the two aligned. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/mail/coverage.md` around lines 9 - 11, Update the compound modifier wording to use hyphens where appropriate: change phrases like "self sent mail", "self send workflow", and "self send thread" to "self-sent mail", "self-send workflow", and "self-send thread" in the coverage.md entries for TestMail_SendWorkflowAsUser and TestMail_DraftLifecycleWorkflowAsUser and also update the matching t.Run subtest names in mail_send_workflow_test.go (and any t.Run strings that echo those phrases) so the documentation and tests stay aligned.tests/cli_e2e/demo/task_lifecycle_test.go (1)
22-22: Minor: considerclie2e.GenerateSuffix()for consistency.The new workflow tests in this PR (
mail_send_workflow_test.go,slides_create_workflow_test.go,calendar_personal_event_workflow_test.go, etc.) all useclie2e.GenerateSuffix(). Using the same helper here would keep suffix semantics (collision resistance, format) uniform across the e2e suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/demo/task_lifecycle_test.go` at line 22, Replace the ad-hoc suffix creation using time.Now().UTC().Format("20060102-150405") with the shared helper clie2e.GenerateSuffix() to ensure consistent suffix format and collision resistance; update the test to call clie2e.GenerateSuffix() where `suffix` is assigned (and add or adjust the clie2e import if missing) so all e2e tests use the same helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/cli_e2e/base/coverage.md`:
- Line 10: Update the documentation entry for TestBase_RoleWorkflow so the
subtest names exactly match the t.Run labels used in the test: replace
occurrences of "get as bot" and "update as bot" with "get role as bot" and
"update role as bot" (these correspond to the t.Run subtests inside
TestBase_RoleWorkflow in base_role_workflow_test.go); make the same replacements
at the other mentioned locations (lines referenced in the comment) so the doc
traceability matches the actual test labels.
In `@tests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.go`:
- Around line 198-212: Replace the fragile list-based existence check with a
direct get call for the known draft id: call clie2e.RunCmd with Args
["mail","user_mailbox.drafts","get"], DefaultAs "user", and Params including
"user_mailbox_id" and "draft_id" (use the existing mailboxID and draftID
variables), then assert the command returns the not-found error (non-zero exit
or specific error status) instead of searching data.items in the paginated list;
update the assertion logic near the t.Run("verify draft removed from list as
user", ...) block to expect a not-found response from the user_mailbox.drafts
get request.
---
Duplicate comments:
In `@tests/cli_e2e/base/coverage.md`:
- Around line 14-15: Add a blank line between the heading "## Command Table" and
the table that starts with "| Status | Cmd" to satisfy MD058
(blanks-around-tables); locate the heading "## Command Table" and insert one
empty line before the table markdown so the table is separated from its heading.
---
Nitpick comments:
In `@tests/cli_e2e/demo/task_lifecycle_test.go`:
- Line 22: Replace the ad-hoc suffix creation using
time.Now().UTC().Format("20060102-150405") with the shared helper
clie2e.GenerateSuffix() to ensure consistent suffix format and collision
resistance; update the test to call clie2e.GenerateSuffix() where `suffix` is
assigned (and add or adjust the clie2e import if missing) so all e2e tests use
the same helper.
In `@tests/cli_e2e/mail/coverage.md`:
- Around line 9-11: Update the compound modifier wording to use hyphens where
appropriate: change phrases like "self sent mail", "self send workflow", and
"self send thread" to "self-sent mail", "self-send workflow", and "self-send
thread" in the coverage.md entries for TestMail_SendWorkflowAsUser and
TestMail_DraftLifecycleWorkflowAsUser and also update the matching t.Run subtest
names in mail_send_workflow_test.go (and any t.Run strings that echo those
phrases) so the documentation and tests stay aligned.
In `@tests/cli_e2e/mail/mail_send_workflow_test.go`:
- Around line 131-175: The inner loop over messages currently may overwrite
inboxMessageID multiple times; change the loop in the triage check so that as
soon as you capture a distinct inbox message ID (when
item.Get("message_id").String() != "" and != sentMessageID) you assign
inboxMessageID and break out of the inner for loop (so the first distinct match
is kept), leaving the outer attempt loop logic unchanged; refer to the
variables/messages loop, inboxMessageID and sentMessageID when making this
change.
- Around line 17-22: Move the test skip call earlier so the test exits before
allocating resources: call clie2e.SkipWithoutUserToken(t) as the first line
inside TestMail_SendWorkflowAsUser (before creating ctx via context.WithTimeout
and before registering t.Cleanup(cancel)) so you avoid creating the context and
cleanup when the test will be skipped; update the order around
clie2e.SkipWithoutUserToken, context.WithTimeout, and t.Cleanup to reflect this.
In `@tests/cli_e2e/sheets/helpers_test.go`:
- Around line 40-49: The cleanup call uses clie2e.RunCmd which doesn't retry on
transient failures; replace this call with clie2e.RunCmdWithRetry using the same
cleanupCtx and clie2e.Request (same Args and DefaultAs) so the delete gets
retried on transient errors, then pass its result and error into
clie2e.ReportCleanupFailure as before; locate the call to clie2e.RunCmd in
tests/cli_e2e/sheets/helpers_test.go and swap it to clie2e.RunCmdWithRetry
(keeping deleteResult/deleteErr and the ReportCleanupFailure invocation intact).
In `@tests/cli_e2e/slides/slides_create_workflow_test.go`:
- Around line 28-38: The test builds the --slides arg by manually escaping
quotes which misses other JSON-unsafe characters; replace the
strings.ReplaceAll(...) construction that builds the slides JSON with a safe
json.Marshal of a []string{slideXML} (import encoding/json) and use the
resulting marshaled string as the "--slides" value passed into clie2e.RunCmd
(the clie2e.Request.Args slice) so all characters (quotes, backslashes,
newlines) are correctly escaped; locate the slideXML variable and the
RunCmd/clie2e.Request.Args usage to make this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 207f33d6-8ac2-4498-bac9-ddd2d0cd7037
📒 Files selected for processing (53)
.github/workflows/ci.ymltests/cli_e2e/base/base_basic_workflow_test.gotests/cli_e2e/base/base_role_workflow_test.gotests/cli_e2e/base/coverage.mdtests/cli_e2e/calendar/calendar_create_event_test.gotests/cli_e2e/calendar/calendar_manage_calendar_test.gotests/cli_e2e/calendar/calendar_personal_event_workflow_test.gotests/cli_e2e/calendar/calendar_rsvp_workflow_test.gotests/cli_e2e/calendar/calendar_view_agenda_test.gotests/cli_e2e/calendar/coverage.mdtests/cli_e2e/calendar/helpers_test.gotests/cli_e2e/contact/contact_lookup_workflow_test.gotests/cli_e2e/contact/contact_shortcut_test.gotests/cli_e2e/contact/coverage.mdtests/cli_e2e/core.gotests/cli_e2e/core_test.gotests/cli_e2e/demo/task_lifecycle_test.gotests/cli_e2e/docs/coverage.mdtests/cli_e2e/docs/docs_create_fetch_test.gotests/cli_e2e/docs/docs_update_test.gotests/cli_e2e/docs/helpers_test.gotests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_files_workflow_test.gotests/cli_e2e/drive/helpers.gotests/cli_e2e/drive/helpers_test.gotests/cli_e2e/im/chat_message_workflow_test.gotests/cli_e2e/im/chat_workflow_test.gotests/cli_e2e/im/coverage.mdtests/cli_e2e/im/helpers_test.gotests/cli_e2e/im/message_get_workflow_test.gotests/cli_e2e/im/message_reply_workflow_test.gotests/cli_e2e/im/message_workflow_test.gotests/cli_e2e/mail/coverage.mdtests/cli_e2e/mail/mail_draft_lifecycle_workflow_test.gotests/cli_e2e/mail/mail_send_workflow_test.gotests/cli_e2e/sheets/coverage.mdtests/cli_e2e/sheets/helpers_test.gotests/cli_e2e/sheets/sheets_create_workflow_test.gotests/cli_e2e/sheets/sheets_crud_workflow_test.gotests/cli_e2e/sheets/sheets_filter_workflow_test.gotests/cli_e2e/slides/coverage.mdtests/cli_e2e/slides/slides_create_workflow_test.gotests/cli_e2e/task/coverage.mdtests/cli_e2e/task/helpers_test.gotests/cli_e2e/task/task_comment_workflow_test.gotests/cli_e2e/task/task_reminder_workflow_test.gotests/cli_e2e/task/task_status_workflow_test.gotests/cli_e2e/task/task_update_workflow_test.gotests/cli_e2e/task/tasklist_add_task_workflow_test.gotests/cli_e2e/task/tasklist_workflow_test.gotests/cli_e2e/wiki/coverage.mdtests/cli_e2e/wiki/helpers_test.gotests/cli_e2e/wiki/wiki_workflow_test.go
💤 Files with no reviewable changes (2)
- tests/cli_e2e/contact/contact_shortcut_test.go
- tests/cli_e2e/im/message_workflow_test.go
✅ Files skipped from review due to trivial changes (16)
- tests/cli_e2e/calendar/calendar_create_event_test.go
- tests/cli_e2e/base/base_role_workflow_test.go
- tests/cli_e2e/base/base_basic_workflow_test.go
- tests/cli_e2e/docs/coverage.md
- tests/cli_e2e/drive/coverage.md
- tests/cli_e2e/wiki/coverage.md
- tests/cli_e2e/contact/coverage.md
- tests/cli_e2e/slides/coverage.md
- tests/cli_e2e/sheets/sheets_create_workflow_test.go
- tests/cli_e2e/calendar/coverage.md
- tests/cli_e2e/im/message_reply_workflow_test.go
- tests/cli_e2e/calendar/calendar_manage_calendar_test.go
- tests/cli_e2e/task/task_reminder_workflow_test.go
- tests/cli_e2e/task/task_update_workflow_test.go
- tests/cli_e2e/contact/contact_lookup_workflow_test.go
- tests/cli_e2e/calendar/calendar_rsvp_workflow_test.go
🚧 Files skipped from review as they are similar to previous changes (15)
- tests/cli_e2e/task/task_status_workflow_test.go
- tests/cli_e2e/drive/drive_files_workflow_test.go
- tests/cli_e2e/task/tasklist_add_task_workflow_test.go
- tests/cli_e2e/task/helpers_test.go
- tests/cli_e2e/docs/docs_update_test.go
- tests/cli_e2e/calendar/calendar_view_agenda_test.go
- tests/cli_e2e/im/message_get_workflow_test.go
- tests/cli_e2e/docs/docs_create_fetch_test.go
- tests/cli_e2e/task/task_comment_workflow_test.go
- tests/cli_e2e/im/chat_message_workflow_test.go
- tests/cli_e2e/sheets/sheets_crud_workflow_test.go
- tests/cli_e2e/core.go
- tests/cli_e2e/calendar/helpers_test.go
- tests/cli_e2e/docs/helpers_test.go
- tests/cli_e2e/drive/helpers.go
yxzhaao
force-pushed
the
feat/cli-e2e-tests-for-user
branch
from
f58dc34 to
cc67797
Compare
yxzhaao
force-pushed
the
feat/cli-e2e-tests-for-user
branch
from
cc67797 to
3c35f93
Compare
liangshuo-1
previously approved these changes
Apr 17, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #528 +/- ##
=======================================
Coverage 59.05% 59.06%
=======================================
Files 384 384
Lines 32636 32645 +9
=======================================
+ Hits 19274 19282 +8
- Misses 11553 11555 +2
+ Partials 1809 1808 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@dbb94b5c106c881c88f77e10e0da0d195cccf185🧩 Skill updatenpx skills add yxzhaao/cli#feat/cli-e2e-tests-for-user -y -g |
yxzhaao
force-pushed
the
feat/cli-e2e-tests-for-user
branch
from
30685c3 to
dbb94b5
Compare
liangshuo-1
approved these changes
Apr 17, 2026
liangshuo-1
approved these changes
Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Test Plan
lark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit
Release Notes
Tests
Chores