test: stabilize live e2e auth retries#1904
Open
yxzhaao wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
the
size/M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (39)
🚧 Files skipped from review as they are similar to previous changes (36)
📝 WalkthroughWalkthroughLive E2E CI now provisions a shared Feishu tenant access token and exports CLI identity variables. A shared test helper verifies tenant authentication, while tenant-dependent Base, Calendar, Contact, Docs, Drive, IM, Markdown, Sheets, Task, and Wiki workflows skip when credentials are unavailable. ChangesLive E2E tenant authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant FeishuOAuth
participant LarkCLI
participant LiveE2ETests
GitHubActions->>FeishuOAuth: Request tenant token
FeishuOAuth-->>GitHubActions: Return token
GitHubActions->>LarkCLI: Export token and CLI identity
LiveE2ETests->>LarkCLI: Verify bot authentication
LarkCLI-->>LiveE2ETests: Return verified status
LiveE2ETests->>LiveE2ETests: Run or skip workflow
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
yxzhaao
force-pushed
the
fix/e2e-invalid-response-retry
branch
from
477052f to
a9e21a3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci-workflow.test.sh`:
- Around line 312-324: Strengthen the workflow assertions in the test section
around the “Prepare shared live E2E tenant token” step: require the exact
LARKSUITE_CLI_BRAND: feishu environment entry and verify that
LARKSUITE_CLI_TENANT_ACCESS_TOKEN is written to GITHUB_ENV, not merely
referenced. Preserve the existing step-condition and job-scope checks.
🪄 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: bde452c3-4683-4681-a59f-0bed8ea40de3
📒 Files selected for processing (41)
.github/workflows/ci.ymlinternal/client/api_errors.gointernal/client/api_errors_test.goscripts/ci-workflow.test.shtests/cli_e2e/base/base_basic_workflow_test.gotests/cli_e2e/base/base_role_workflow_test.gotests/cli_e2e/calendar/calendar_create_event_test.gotests/cli_e2e/calendar/calendar_manage_calendar_test.gotests/cli_e2e/calendar/calendar_rsvp_workflow_test.gotests/cli_e2e/calendar/calendar_update_event_test.gotests/cli_e2e/contact/contact_lookup_workflow_test.gotests/cli_e2e/core.gotests/cli_e2e/core_test.gotests/cli_e2e/docs/docs_create_fetch_test.gotests/cli_e2e/docs/docs_update_test.gotests/cli_e2e/drive/drive_add_comment_workflow_test.gotests/cli_e2e/drive/drive_duplicate_sync_workflow_test.gotests/cli_e2e/drive/drive_files_workflow_test.gotests/cli_e2e/drive/drive_preview_workflow_test.gotests/cli_e2e/drive/drive_status_workflow_test.gotests/cli_e2e/drive/drive_sync_workflow_test.gotests/cli_e2e/drive/drive_upload_workflow_test.gotests/cli_e2e/drive/drive_version_workflow_test.gotests/cli_e2e/im/chat_workflow_test.gotests/cli_e2e/im/message_forward_workflow_test.gotests/cli_e2e/im/message_reply_workflow_test.gotests/cli_e2e/markdown/markdown_workflow_test.gotests/cli_e2e/sheets/sheets_crud_workflow_test.gotests/cli_e2e/sheets/sheets_filter_workflow_test.gotests/cli_e2e/sheets/sheets_gridline_workflow_test.gotests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.gotests/cli_e2e/sheets/sheets_table_get_workflow_test.gotests/cli_e2e/sheets/sheets_table_put_typed_workflow_test.gotests/cli_e2e/sheets/sheets_workbook_import_workflow_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/tasklist_add_task_workflow_test.gotests/cli_e2e/task/tasklist_workflow_test.gotests/cli_e2e/wiki/wiki_shortcut_workflow_test.gotests/cli_e2e/wiki/wiki_workflow_test.go
Comment on lines
+312
to
+324
| if ! grep -Fq "LARKSUITE_CLI_APP_ID: \${{ secrets.TEST_BOT1_APP_ID }}" <<<"$section"; then | ||
| echo "e2e-live should expose the bot app id through job env" | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! awk ' | ||
| /^ - name: Configure bot credentials/ { in_step = 1 } | ||
| in_step && /if: \$\{\{ steps\.e2e_domains\.outputs\.mode != '\''skip'\'' \}\}/ { found = 1 } | ||
| in_step && /^ - name:/ && !/Configure bot credentials/ { in_step = 0 } | ||
| END { exit found ? 0 : 1 } | ||
| /^ - name: Prepare shared live E2E tenant token/ { in_step = 1 } | ||
| in_step && /if: \$\{\{ steps\.e2e_domains\.outputs\.mode != '\''skip'\'' \}\}/ { has_if = 1 } | ||
| in_step && /LARKSUITE_CLI_TENANT_ACCESS_TOKEN/ { has_tat = 1 } | ||
| in_step && /^ - name:/ && !/Prepare shared live E2E tenant token/ { in_step = 0 } | ||
| END { exit has_if && has_tat ? 0 : 1 } | ||
| ' <<<"$section"; then | ||
| echo "e2e-live should only configure bot credentials when domain mode is not skip" | ||
| echo "e2e-live should prepare one shared tenant token for all live CLI child processes" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Assert the complete tenant-token environment contract.
This test can still pass if LARKSUITE_CLI_BRAND: feishu is removed or if the token variable is merely mentioned without being written to GITHUB_ENV. Check both the exact brand entry and the persistence statement directly.
As per coding guidelines, “contract tests must assert the new field or behavior directly.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ci-workflow.test.sh` around lines 312 - 324, Strengthen the workflow
assertions in the test section around the “Prepare shared live E2E tenant token”
step: require the exact LARKSUITE_CLI_BRAND: feishu environment entry and verify
that LARKSUITE_CLI_TENANT_ACCESS_TOKEN is written to GITHUB_ENV, not merely
referenced. Preserve the existing step-condition and job-scope checks.
Source: Coding guidelines
Change-Id: I330517f0cb9b1349962c086a71f2793db149d0fa
yxzhaao
force-pushed
the
fix/e2e-invalid-response-retry
branch
from
a9e21a3 to
df471b2
Compare
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
1 participant
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.
Change-Id: I330517f0cb9b1349962c086a71f2793db149d0fa
Summary
Changes
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit
Bug Fixes
Tests