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
liuxinyanglxy pushed a commit that referenced this pull request Aug 5, 2026
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.
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