Skip to content

test: use tri-state wiki node identity in delete verification#1931

Merged
luozhixiong01 merged 1 commit into
mainfrom
fix/wiki-delete-verify-tri-state
Jul 17, 2026
Merged

test: use tri-state wiki node identity in delete verification#1931
luozhixiong01 merged 1 commit into
mainfrom
fix/wiki-delete-verify-tri-state

Conversation

@luozhixiong01

@luozhixiong01 luozhixiong01 commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

The wiki get_node success response may omit data.node/node_token (the field is optional). The previous verification logic treated a missing token as proof that the original node was deleted, so isWikiNodeDeleted could report a false deletion and deleteWikiNodeAndVerify could silently skip cleanup, leaking real wiki nodes. This PR replaces the two-state check with a tri-state identity classification so only positive evidence counts.

Changes

  • Replace boolean wikiGetNodeMatchesToken with tri-state wikiGetNodeIdentity in tests/cli_e2e/wiki/helpers_test.go: same token = node exists; different non-empty token = original node gone (move-to-drive); missing/empty token = indeterminate
  • isWikiNodeDeleted treats only a different non-empty node_token as deletion proof; an indeterminate response keeps polling instead of reporting deleted
  • deleteWikiNodeAndVerify returns early only on a confirmed different identity; an indeterminate response still attempts the cleanup delete
  • Add TestWikiGetNodeIdentity regression test with 5 subcases covering the tri-state semantics

Test Plan

  • go test ./tests/cli_e2e/wiki/ -run 'TestWikiGetNodeIdentity|TestWikiVerifyTransientResult' passed (unit-style tests, no live credentials required)
  • go vet ./tests/cli_e2e/wiki/ and gofmt -l clean
  • skipped: additional local live E2E — E2E-test-helper-only change with no product code surface; the live path is exercised by TestWiki_MoveToDriveWorkflow in this PR's e2e-live CI job
  • manual verification: with this change applied, e2e-live run 29561268329 no longer reproduces the deletion-verification misjudgment (remaining failures there were unrelated tenant-token HTTP 429 rate limiting)

Related Issues

N/A

Summary by CodeRabbit

  • Bug Fixes
    • Improved wiki node deletion verification when a node has been moved or replaced.
    • Prevented successful API responses from being incorrectly interpreted as proof that the original node was deleted.
    • Enhanced handling of missing, empty, or mismatched node identifiers during cleanup and verification.

A get_node success response may omit data.node/node_token (the field is
optional), so a missing token must not be read as proof of deletion.
Classify the response as same / different / unknown: only a different
non-empty node_token proves the original node is gone (move-to-drive),
while an unknown identity keeps polling in isWikiNodeDeleted and still
attempts deletion in deleteWikiNodeAndVerify instead of leaking nodes.
@luozhixiong01 luozhixiong01 added the bugfix Bug fixes label Jul 17, 2026
@github-actions github-actions Bot added the size/S Low-risk docs, CI, test, or chore only changes label Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 188bbe82-f395-45ae-bee1-77e6041b9a0f

📥 Commits

Reviewing files that changed from the base of the PR and between 47dc003 and 57f165d.

📒 Files selected for processing (1)
  • tests/cli_e2e/wiki/helpers_test.go

📝 Walkthrough

Walkthrough

Wiki E2E helpers now classify get_node responses by resolved node token. Cleanup treats a different resolved token as proof that the original node is gone, while deletion checks distinguish same-token, different-token, and indeterminate responses.

Changes

Wiki node cleanup verification

Layer / File(s) Summary
Node identity classification and coverage
tests/cli_e2e/wiki/helpers_test.go
Adds wikiNodeIdentity and wikiGetNodeIdentity to classify same, different, and unknown node tokens, with tests for payload and token edge cases.
Cleanup and deletion decisions
tests/cli_e2e/wiki/helpers_test.go
Updates cleanup and deletion checks to treat a different resolved token as evidence that the queried node is already gone.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • larksuite/cli#401: Both changes modify the Wiki CLI E2E test helpers used for node verification and deletion.

Suggested reviewers: yxzhaao

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main change: tri-state wiki node identity in delete verification.
Description check ✅ Passed The description matches the template and includes summary, changes, test plan, and related issues.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wiki-delete-verify-tri-state

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.

@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@57f165d09dbd099ac8df009949dcfdf9813a9b45

🧩 Skill update

npx skills add larksuite/cli#fix/wiki-delete-verify-tri-state -y -g

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.96%. Comparing base (47dc003) to head (57f165d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1931   +/-   ##
=======================================
  Coverage   74.96%   74.96%           
=======================================
  Files         892      892           
  Lines       94058    94058           
=======================================
  Hits        70506    70506           
  Misses      18139    18139           
  Partials     5413     5413           

☔ View full report in Codecov by Harness.
📢 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.

@luozhixiong01
luozhixiong01 merged commit c363acf into main Jul 17, 2026
53 of 55 checks passed
@luozhixiong01
luozhixiong01 deleted the fix/wiki-delete-verify-tri-state branch July 17, 2026 09:52
@liangshuo-1 liangshuo-1 mentioned this pull request Jul 17, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fixes 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