Skip to content

test: drop stale yes flags from e2e#815

Merged
liangshuo-1 merged 1 commit intolarksuite:mainfrom
yxzhaao:fix/remove-stale-e2e-yes-flags
May 11, 2026
Merged

test: drop stale yes flags from e2e#815
liangshuo-1 merged 1 commit intolarksuite:mainfrom
yxzhaao:fix/remove-stale-e2e-yes-flags

Conversation

@yxzhaao
Copy link
Copy Markdown
Contributor

@yxzhaao yxzhaao commented May 11, 2026

Summary

  • remove stale Yes: true from calendar events delete e2e after the command stopped registering --yes
  • remove stale Yes: true from sheets spreadsheet.sheet.filters create e2e after the command stopped registering --yes

Verification

  • go test ./tests/cli_e2e/calendar ./tests/cli_e2e/sheets -run '^$'

Note: local full pre-commit Go test is blocked by unrelated local-only failures in internal/auth/token_store_test.go and an internal/binding exec-provider timeout.

Summary by CodeRabbit

  • Tests
    • Updated calendar and sheets E2E test workflows to adjust confirmation handling for event deletion and filter creation operations.

Review Change Stack

@github-actions github-actions Bot added the size/S Low-risk docs, CI, test, or chore only changes label May 11, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e97e4c38-8b23-48db-9efe-6cc0649315d8

📥 Commits

Reviewing files that changed from the base of the PR and between 16f1a0f and 491a8a1.

📒 Files selected for processing (2)
  • tests/cli_e2e/calendar/calendar_create_event_test.go
  • tests/cli_e2e/sheets/sheets_filter_workflow_test.go
💤 Files with no reviewable changes (2)
  • tests/cli_e2e/sheets/sheets_filter_workflow_test.go
  • tests/cli_e2e/calendar/calendar_create_event_test.go

📝 Walkthrough

Walkthrough

This PR removes the Yes: true automatic confirmation flag from two E2E test cases in the calendar and sheets workflows. The calendar event deletion and sheets filter creation test steps no longer bypass confirmation prompts, aligning test behavior with updated CLI confirmation handling patterns.

Changes

Confirmation Flag Removal from E2E Tests

Layer / File(s) Summary
E2E Test Confirmation Flag Removal
tests/cli_e2e/calendar/calendar_create_event_test.go, tests/cli_e2e/sheets/sheets_filter_workflow_test.go
The Yes: true field is removed from clie2e.Request in the "delete event as bot" subtest (calendar) and the filter creation step (sheets), allowing these operations to handle confirmation differently.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • larksuite/cli#633: Both PRs modify E2E test invocations related to the --yes confirmation flag (the retrieved PR adds/uses Request.Yes for high-risk confirmation; the main PR removes Yes: true in specific tests).
  • larksuite/cli#794: Both PRs remove stale Yes: true confirmation flags from CLI E2E tests (affecting calendar and sheets test requests).
  • larksuite/cli#701: Both PRs modify the E2E tests' confirmation flag handling for delete operations—specifically they touch the same calendar delete test, but in opposite directions.

Suggested reviewers

  • liangshuo-1
  • liuxinyanglxy

Poem

🐰 A flag removed, a prompt set free,
No more auto-yes, let users see,
Confirmation calls shall have their day,
The tests now ask in the proper way! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is incomplete. It deviates from the template by omitting the Changes section with bullet points and the Test Plan checklist with verification steps. Reorganize the description to match the template: add a Changes section listing the two removed Yes flags, and add a Test Plan section with checkboxes for test verification.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: removing stale yes flags from end-to-end tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yxzhaao yxzhaao force-pushed the fix/remove-stale-e2e-yes-flags branch from b899293 to 491a8a1 Compare May 11, 2026 05:41
@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.60%. Comparing base (16f1a0f) to head (491a8a1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #815   +/-   ##
=======================================
  Coverage   65.60%   65.60%           
=======================================
  Files         513      513           
  Lines       47644    47644           
=======================================
  Hits        31259    31259           
  Misses      13685    13685           
  Partials     2700     2700           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 11, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@491a8a1ae637c5d615b6dbfd6a372627a1d6664f

🧩 Skill update

npx skills add yxzhaao/cli#fix/remove-stale-e2e-yes-flags -y -g

@liangshuo-1 liangshuo-1 merged commit 5352e6a into larksuite:main May 11, 2026
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Low-risk docs, CI, test, or chore only changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants