diff --git a/tests/cli_e2e/drive/coverage.md b/tests/cli_e2e/drive/coverage.md index 0e5de38901..4ab4b773f1 100644 --- a/tests/cli_e2e/drive/coverage.md +++ b/tests/cli_e2e/drive/coverage.md @@ -16,7 +16,7 @@ - TestDriveAddCommentMarkdownFileWorkflow: opt-in live workflow skeleton for comment write/read, gated by `LARK_DRIVE_MD_COMMENT_E2E=1`; creates a Markdown file, adds a file comment, lists it back through `drive +list-comments`, and cleans up. - TestDrive_SecureLabelDryRun: dry-run coverage for `drive +secure-label-list` and `drive +secure-label-update`; asserts label-list query params and update URL→type inference, request method/URL/type query, and `label-id` body shape. Runs without hitting live APIs because update can trigger document-level security approval flows. - TestDriveExportDryRun_FileNameMetadata / TestDriveExportDryRun_WikiURLPlansResolveBeforeExportTask / TestDriveExportDryRun_WikiTokenTypePlansResolveBeforeExportTask / TestDriveExportDryRun_MarkdownFetchAPI / TestDriveExportDryRun_BitableBaseOnlySchema: dry-run coverage for `drive +export`; asserts export task request shape, Wiki URL and `--doc-type wiki` token `get_node -> export_tasks` planning, markdown fetch request shape without docs fetch `extra_param`, local `--file-name` / `--output-dir` metadata, and `bitable` `.base` `only_schema` request body without calling live APIs. -- TestDriveDeleteDryRunAsyncParams / TestDrive_DeleteAsyncWorkflow: dry-run coverage for `drive +delete` pins `DELETE /drive/v1/files/:file_token` params with `type` plus `async=true` and the follow-up `task_check` plan; live workflow creates and deletes a docx, an empty folder, and a non-empty folder, asserts each delete returns `task_id`, queries every returned task via `drive +task_result --scenario task_check`, and verifies the targets disappear. +- TestDriveDeleteDryRunAsyncParams / TestDrive_DeleteAsyncWorkflow: dry-run coverage for `drive +delete` pins `DELETE /drive/v1/files/:file_token` params with `type` plus `async=true` and the follow-up `task_check` plan; live workflow creates and deletes a docx, an empty folder, and a non-empty folder, converging every delete outcome to the resource-gone terminal state: async deletes (non-empty `task_id`) are verified via `drive +task_result --scenario task_check`, sync deletes (empty `task_id`) assert `deleted=true`, and the one verified backend transient (`server_error: "drive task failed"`) passes once the target is confirmed gone (retried up to 3 times otherwise); any other delete failure stays fatal. - TestDrive_PullDryRun / TestDrive_PullDryRunAcceptsDuplicateRemoteStrategies: dry-run coverage for `drive +pull`; asserts the list-files request shape, Validate-stage safety guards, and acceptance of `--on-duplicate-remote=rename|newest|oldest` by the real CLI binary. - TestDrive_PushDryRun / TestDrive_PushDryRunAcceptsDuplicateRemoteStrategies: dry-run coverage for `drive +push`; asserts the list-files request shape, Validate-stage safety guards, conditional delete preflight, and acceptance of `--on-duplicate-remote=newest|oldest` by the real CLI binary. - Cleanup note: `drive files delete` is only exercised in cleanup and is intentionally left uncovered. @@ -30,7 +30,7 @@ | ✓ | drive +add-comment | shortcut | drive_add_comment_dryrun_test.go::TestDriveAddCommentDryRun_File; drive_add_comment_dryrun_test.go::TestDriveAddCommentDryRun_Base | `--doc` file URL vs bare token + `--type file`; supported-extension metadata gate; placeholder `anchor.block_id`; Base URL with `--block-id !!` | dry-run coverage in place; opt-in live file workflow exists behind `LARK_DRIVE_MD_COMMENT_E2E=1` | | ✓ | drive +list-comments | shortcut | drive_list_comments_dryrun_test.go::TestDriveListCommentsDryRun_DocxDefaults; drive_list_comments_dryrun_test.go::TestDriveListCommentsDryRun_AppsPageURL; drive_list_comments_dryrun_test.go::TestDriveListCommentsDryRun_WikiToken; drive_add_comment_workflow_test.go::TestDriveAddCommentMarkdownFileWorkflow | `--url`; apps `/page/` URL; `--token + --type wiki`; `--solved-status=false\|all`; `--comment-scope=all\|partial`; `--need-relation`; `--page-size` | dry-run locks URL/token parsing, apps `file_type=apps`, default unresolved filter, omitted all-scope filter, omitted `user_id_type`, and Wiki unwrap request shape; opt-in live workflow verifies a created file comment can be listed back | | ✓ | drive +apply-permission | shortcut | drive_apply_permission_dryrun_test.go::TestDrive_ApplyPermissionDryRun | `--token` URL vs bare; `--type` (enum) with URL inference; `--perm view\|edit`; `--remark` optional | dry-run only; no live-apply E2E because a real request pushes a card to the owner | -| ✓ | drive +delete | shortcut | drive_delete_dryrun_test.go::TestDriveDeleteDryRunAsyncParams + drive_delete_workflow_test.go::TestDrive_DeleteAsyncWorkflow | `--file-token`; `--type`; fixed query `async=true`; `task_check` follow-up | dry-run locks async request shape; live workflow covers docx, empty folder, and non-empty folder async deletion | +| ✓ | drive +delete | shortcut | drive_delete_dryrun_test.go::TestDriveDeleteDryRunAsyncParams + drive_delete_workflow_test.go::TestDrive_DeleteAsyncWorkflow | `--file-token`; `--type`; fixed query `async=true`; `task_check` follow-up | dry-run locks async request shape; live workflow covers docx, empty folder, and non-empty folder deletion with async/sync/transient-failure convergence | | ✕ | drive +download | shortcut | | none | no file fixture workflow yet | | ✓ | drive +export | shortcut | drive_export_dryrun_test.go::TestDriveExportDryRun_FileNameMetadata + TestDriveExportDryRun_WikiURLPlansResolveBeforeExportTask + TestDriveExportDryRun_WikiTokenTypePlansResolveBeforeExportTask + TestDriveExportDryRun_MarkdownFetchAPI + TestDriveExportDryRun_BitableBaseOnlySchema | `--url`; `--token`; `--doc-type`; `--file-extension`; `--file-name`; `--output-dir`; `--only-schema`; Wiki URL / `--doc-type wiki` resolve step; markdown fetch omits docs fetch `extra_param` | dry-run only; no live export workflow yet | | ✕ | drive +export-download | shortcut | | none | no export-download workflow yet | diff --git a/tests/cli_e2e/drive/drive_delete_workflow_helper_test.go b/tests/cli_e2e/drive/drive_delete_workflow_helper_test.go new file mode 100644 index 0000000000..68b537cbe0 --- /dev/null +++ b/tests/cli_e2e/drive/drive_delete_workflow_helper_test.go @@ -0,0 +1,370 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDeleteAsyncAndVerify(t *testing.T) { + t.Run("sync delete without task_id skips task_result", func(t *testing.T) { + fake := mustWriteDriveDeleteWorkflowFakeCLI(t) + t.Setenv(clie2e.EnvBinaryPath, fake) + t.Setenv("FAKE_WORKFLOW_DELETE_MODE", "sync") + t.Setenv("FAKE_WORKFLOW_META_MODE", "gone") + counters := setupFakeWorkflowCounters(t) + + taskID := deleteAsyncAndVerify(t, context.Background(), "docx_sync", "docx") + assert.Empty(t, taskID) + + assert.Equal(t, "1", readFakeCounter(t, counters.deletes), "sync path must delete exactly once") + assert.Equal(t, "1", readFakeCounter(t, counters.metas), "sync path must still verify the resource is gone") + assert.Equal(t, "0", readFakeCounter(t, counters.taskResults), "sync path must not query task status") + }) + + t.Run("transient failure with resource gone is tolerated", func(t *testing.T) { + fake := mustWriteDriveDeleteWorkflowFakeCLI(t) + t.Setenv(clie2e.EnvBinaryPath, fake) + t.Setenv("FAKE_WORKFLOW_DELETE_MODE", "fail") + t.Setenv("FAKE_WORKFLOW_META_MODE", "gone") + counters := setupFakeWorkflowCounters(t) + + taskID := deleteAsyncAndVerify(t, context.Background(), "docx_transient", "docx") + assert.Empty(t, taskID) + + assert.Equal(t, "1", readFakeCounter(t, counters.deletes), "resource already gone must not trigger another delete attempt") + assert.Equal(t, "1", readFakeCounter(t, counters.metas), "transient failure must verify the terminal state") + assert.Equal(t, "0", readFakeCounter(t, counters.taskResults)) + }) + + t.Run("failed delete retries until async success", func(t *testing.T) { + fake := mustWriteDriveDeleteWorkflowFakeCLI(t) + t.Setenv(clie2e.EnvBinaryPath, fake) + t.Setenv("FAKE_WORKFLOW_DELETE_MODE", "fail-then-async") + t.Setenv("FAKE_WORKFLOW_META_MODE", "exists-then-gone") + t.Setenv("FAKE_WORKFLOW_TASK_RESULT_OK", "1") + counters := setupFakeWorkflowCounters(t) + withFastDeleteWorkflowBackoff(t) + + taskID := deleteAsyncAndVerify(t, context.Background(), "docx_retry", "docx") + assert.Equal(t, "task_123", taskID) + + assert.Equal(t, "2", readFakeCounter(t, counters.deletes)) + assert.Equal(t, "2", readFakeCounter(t, counters.metas), "one terminal-state check after the failure plus the final visibility wait") + assert.Equal(t, "1", readFakeCounter(t, counters.taskResults), "async success must verify the task result") + }) +} + +// TestIsTransientDriveDeleteFailure locks the tolerance boundary: only the one +// verified backend transient may fall through to terminal-state checking, so a +// crash, a protocol regression, or any other error keeps failing the workflow +// even when the resource happens to be gone. +func TestIsTransientDriveDeleteFailure(t *testing.T) { + t.Run("matches compact envelope", func(t *testing.T) { + result := &clie2e.Result{ + ExitCode: 1, + Stderr: "Deleting docx tok...\n{\"ok\":false,\"identity\":\"bot\",\"error\":{\"type\":\"api\",\"subtype\":\"server_error\",\"message\":\"drive task failed\"}}", + } + assert.True(t, isTransientDriveDeleteFailure(result)) + }) + + t.Run("matches pretty-printed envelope from CI", func(t *testing.T) { + result := &clie2e.Result{ + ExitCode: 1, + Stderr: "Deleting docx NTw0...Rngb...\nDelete is async, polling task schedule|7663369798226545963...\n" + + "{\n \"ok\": false,\n \"identity\": \"bot\",\n \"error\": {\n \"type\": \"api\",\n \"subtype\": \"server_error\",\n \"message\": \"drive task failed\"\n }\n}", + } + assert.True(t, isTransientDriveDeleteFailure(result)) + }) + + t.Run("rejects other server errors", func(t *testing.T) { + result := &clie2e.Result{ + ExitCode: 1, + Stderr: "{\"ok\":false,\"identity\":\"bot\",\"error\":{\"type\":\"api\",\"subtype\":\"server_error\",\"message\":\"internal error\"}}", + } + assert.False(t, isTransientDriveDeleteFailure(result)) + }) + + t.Run("rejects non-server-error subtypes", func(t *testing.T) { + result := &clie2e.Result{ + ExitCode: 1, + Stderr: "{\"ok\":false,\"identity\":\"bot\",\"error\":{\"type\":\"api\",\"subtype\":\"permission_denied\",\"message\":\"drive task failed\"}}", + } + assert.False(t, isTransientDriveDeleteFailure(result)) + }) + + t.Run("rejects non-JSON output", func(t *testing.T) { + result := &clie2e.Result{ExitCode: 2, Stderr: "panic: runtime error"} + assert.False(t, isTransientDriveDeleteFailure(result)) + }) + + t.Run("rejects nil result", func(t *testing.T) { + assert.False(t, isTransientDriveDeleteFailure(nil)) + }) +} + +// TestDeleteAsyncAndVerifyRejectsUnexpectedFailure locks the P1 boundary +// end-to-end by re-running this test binary as a subprocess that really calls +// deleteAsyncAndVerify: an unrelated non-zero exit must fail the helper +// immediately — no terminal-state check may rescue it even though meta reports +// the resource gone. Fatalf cannot be observed on the parent *testing.T, so +// the boundary is proven by the child process exiting non-zero AND the meta +// endpoint never being reached. Removing the isTransientDriveDeleteFailure +// guard from the main loop turns this test red. +func TestDeleteAsyncAndVerifyRejectsUnexpectedFailure(t *testing.T) { + fake := mustWriteDriveDeleteWorkflowFakeCLI(t) + counters := newFakeWorkflowCounterPaths(t) + + output, err := runDeleteWorkflowSubprocess(t, fake, counters, map[string]string{ + "FAKE_WORKFLOW_TOKEN": "docx_unexpected", + "FAKE_WORKFLOW_DELETE_MODE": "fail-unexpected", + "FAKE_WORKFLOW_META_MODE": "gone", + }) + require.Error(t, err, "deleteAsyncAndVerify must fail the test process on an unexpected delete error\noutput:\n%s", output) + assert.Contains(t, output, "drive +delete failed with an unexpected error", "output:\n%s", output) + + assert.Equal(t, "1", readFakeCounter(t, counters.deletes)) + assert.Equal(t, "0", readFakeCounter(t, counters.metas), "unexpected failures must not fall through to terminal-state checking") + assert.Equal(t, "0", readFakeCounter(t, counters.taskResults)) +} + +// TestDeleteAsyncAndVerifyFailsOnTaskResultFailure proves a non-zero +// drive +task_result exit fails the workflow before the final visibility +// polling: the task-result endpoint is reached once and the meta endpoint +// never. +func TestDeleteAsyncAndVerifyFailsOnTaskResultFailure(t *testing.T) { + fake := mustWriteDriveDeleteWorkflowFakeCLI(t) + counters := newFakeWorkflowCounterPaths(t) + + output, err := runDeleteWorkflowSubprocess(t, fake, counters, map[string]string{ + "FAKE_WORKFLOW_TOKEN": "docx_taskresult", + "FAKE_WORKFLOW_DELETE_MODE": "async", + "FAKE_WORKFLOW_META_MODE": "gone", + // FAKE_WORKFLOW_TASK_RESULT_OK stays unset: +task_result exits 2. + }) + require.Error(t, err, "deleteAsyncAndVerify must fail the test process when +task_result fails\noutput:\n%s", output) + assert.Contains(t, output, "drive +task_result failed", "output:\n%s", output) + + assert.Equal(t, "1", readFakeCounter(t, counters.deletes)) + assert.Equal(t, "1", readFakeCounter(t, counters.taskResults)) + assert.Equal(t, "0", readFakeCounter(t, counters.metas), "task-result failure must abort before visibility polling") +} + +// TestDeleteAsyncAndVerifyStopsAfterExhaustedRetries proves the transient +// tolerance is bounded: with the resource still present, exactly +// deleteWorkflowMaxAttempts delete attempts (each followed by one terminal +// state check) run before the workflow fails for good. +func TestDeleteAsyncAndVerifyStopsAfterExhaustedRetries(t *testing.T) { + fake := mustWriteDriveDeleteWorkflowFakeCLI(t) + counters := newFakeWorkflowCounterPaths(t) + + output, err := runDeleteWorkflowSubprocess(t, fake, counters, map[string]string{ + "FAKE_WORKFLOW_TOKEN": "docx_exhausted", + "FAKE_WORKFLOW_DELETE_MODE": "fail", + "FAKE_WORKFLOW_META_MODE": "exists", + "FAKE_WORKFLOW_FAST_BACKOFF": "1", + }) + require.Error(t, err, "deleteAsyncAndVerify must fail the test process after exhausting retries\noutput:\n%s", output) + assert.Contains(t, output, "drive +delete failed 3 times", "output:\n%s", output) + + assert.Equal(t, "3", readFakeCounter(t, counters.deletes)) + assert.Equal(t, "3", readFakeCounter(t, counters.metas)) + assert.Equal(t, "0", readFakeCounter(t, counters.taskResults)) +} + +// runDeleteWorkflowSubprocess re-runs this test binary anchored to the child +// entry point below with the fake CLI and counter files wired in via env. +func runDeleteWorkflowSubprocess(t *testing.T, fake string, counters fakeWorkflowCounters, env map[string]string) (string, error) { + t.Helper() + + cmd := exec.Command(os.Args[0], "-test.run=TestDeleteAsyncAndVerifySubprocess$", "-test.v") + cmd.Env = append(os.Environ(), + "FAKE_WORKFLOW_SUBPROCESS=1", + clie2e.EnvBinaryPath+"="+fake, + "FAKE_WORKFLOW_DELETE_STATE="+counters.deletes, + "FAKE_WORKFLOW_META_STATE="+counters.metas, + "FAKE_WORKFLOW_TASK_RESULT_STATE="+counters.taskResults, + ) + for k, v := range env { + cmd.Env = append(cmd.Env, k+"="+v) + } + output, err := cmd.CombinedOutput() + return string(output), err +} + +// TestDeleteAsyncAndVerifySubprocess is the child entry point driven by +// runDeleteWorkflowSubprocess. It does nothing in a normal test run. +func TestDeleteAsyncAndVerifySubprocess(t *testing.T) { + if os.Getenv("FAKE_WORKFLOW_SUBPROCESS") != "1" { + return + } + if os.Getenv("FAKE_WORKFLOW_FAST_BACKOFF") == "1" { + deleteWorkflowRetryBackoff = time.Millisecond + } + deleteAsyncAndVerify(t, context.Background(), os.Getenv("FAKE_WORKFLOW_TOKEN"), "docx") +} + +type fakeWorkflowCounters struct { + deletes string + metas string + taskResults string +} + +func newFakeWorkflowCounterPaths(t *testing.T) fakeWorkflowCounters { + t.Helper() + + dir := t.TempDir() + return fakeWorkflowCounters{ + deletes: filepath.Join(dir, "delete-attempts"), + metas: filepath.Join(dir, "meta-calls"), + taskResults: filepath.Join(dir, "task-result-calls"), + } +} + +// setupFakeWorkflowCounters wires per-endpoint call counters into the fake CLI +// so tests can assert exactly which commands ran. +func setupFakeWorkflowCounters(t *testing.T) fakeWorkflowCounters { + t.Helper() + + counters := newFakeWorkflowCounterPaths(t) + t.Setenv("FAKE_WORKFLOW_DELETE_STATE", counters.deletes) + t.Setenv("FAKE_WORKFLOW_META_STATE", counters.metas) + t.Setenv("FAKE_WORKFLOW_TASK_RESULT_STATE", counters.taskResults) + return counters +} + +func readFakeCounter(t *testing.T, path string) string { + t.Helper() + + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return "0" + } + require.NoError(t, err) + return string(data) +} + +func withFastDeleteWorkflowBackoff(t *testing.T) { + t.Helper() + + original := deleteWorkflowRetryBackoff + deleteWorkflowRetryBackoff = time.Millisecond + t.Cleanup(func() { + deleteWorkflowRetryBackoff = original + }) +} + +// mustWriteDriveDeleteWorkflowFakeCLI writes a fake lark-cli that emulates the +// drive delete outcomes exercised by deleteAsyncAndVerify. Every endpoint +// bumps a per-endpoint counter when its FAKE_WORKFLOW_*_STATE env is set, so +// tests can assert call contracts. +task_result rejects every call unless +// FAKE_WORKFLOW_TASK_RESULT_OK=1, which proves the sync path never queries +// task status. +func mustWriteDriveDeleteWorkflowFakeCLI(t *testing.T) string { + t.Helper() + + script := `#!/bin/sh +bump_counter() { + state="$1" + count=0 + if [ -f "$state" ]; then + count="$(cat "$state")" + fi + next=$((count + 1)) + printf '%s' "$next" > "$state" + echo "$count" +} + +if [ "$1" = "drive" ] && [ "$2" = "+delete" ]; then + count=0 + if [ -n "$FAKE_WORKFLOW_DELETE_STATE" ]; then + count="$(bump_counter "$FAKE_WORKFLOW_DELETE_STATE")" + fi + case "$FAKE_WORKFLOW_DELETE_MODE" in + sync) + echo '{"ok":true,"identity":"bot","data":{"deleted":true,"file_token":"tok","type":"docx"}}' + exit 0 + ;; + fail) + echo "Deleting docx tok..." >&2 + echo '{"ok":false,"identity":"bot","error":{"type":"api","subtype":"server_error","message":"drive task failed"}}' >&2 + exit 1 + ;; + fail-unexpected) + echo '{"ok":false,"identity":"bot","error":{"type":"api","subtype":"invalid_request","message":"file token not found"}}' >&2 + exit 1 + ;; + async) + echo '{"ok":true,"identity":"bot","data":{"task_id":"task_123","status":"success","file_token":"tok","type":"docx"}}' + exit 0 + ;; + fail-then-async) + if [ "$count" -lt 1 ]; then + echo '{"ok":false,"identity":"bot","error":{"type":"api","subtype":"server_error","message":"drive task failed"}}' >&2 + exit 1 + fi + echo '{"ok":true,"identity":"bot","data":{"task_id":"task_123","status":"success","file_token":"tok","type":"docx"}}' + exit 0 + ;; + esac + echo "unexpected FAKE_WORKFLOW_DELETE_MODE: $FAKE_WORKFLOW_DELETE_MODE" >&2 + exit 2 +fi + +if [ "$1" = "drive" ] && [ "$2" = "+task_result" ]; then + if [ -n "$FAKE_WORKFLOW_TASK_RESULT_STATE" ]; then + bump_counter "$FAKE_WORKFLOW_TASK_RESULT_STATE" > /dev/null + fi + if [ "${FAKE_WORKFLOW_TASK_RESULT_OK:-0}" != "1" ]; then + echo "unexpected +task_result call: $*" >&2 + exit 2 + fi + echo '{"ok":true,"identity":"bot","data":{"task_id":"task_123","status":"success","failed":false}}' + exit 0 +fi + +if [ "$1" = "api" ] && [ "$2" = "post" ] && [ "$3" = "/open-apis/drive/v1/metas/batch_query" ]; then + count=0 + if [ -n "$FAKE_WORKFLOW_META_STATE" ]; then + count="$(bump_counter "$FAKE_WORKFLOW_META_STATE")" + fi + case "$FAKE_WORKFLOW_META_MODE" in + gone) + echo '{"ok":true,"data":{"metas":[]}}' + exit 0 + ;; + exists) + echo '{"ok":true,"data":{"metas":[{"url":"https://example.com/still-visible"}]}}' + exit 0 + ;; + exists-then-gone) + if [ "$count" -lt 1 ]; then + echo '{"ok":true,"data":{"metas":[{"url":"https://example.com/still-visible"}]}}' + exit 0 + fi + echo '{"ok":true,"data":{"metas":[]}}' + exit 0 + ;; + esac + echo "unexpected FAKE_WORKFLOW_META_MODE: $FAKE_WORKFLOW_META_MODE" >&2 + exit 2 +fi + +echo "unexpected fake CLI args: $*" >&2 +exit 2 +` + + binaryPath := filepath.Join(t.TempDir(), "fake-lark-cli") + require.NoError(t, os.WriteFile(binaryPath, []byte(script), 0o755)) + return binaryPath +} diff --git a/tests/cli_e2e/drive/drive_delete_workflow_test.go b/tests/cli_e2e/drive/drive_delete_workflow_test.go index 0ac0102e81..138972bafb 100644 --- a/tests/cli_e2e/drive/drive_delete_workflow_test.go +++ b/tests/cli_e2e/drive/drive_delete_workflow_test.go @@ -5,6 +5,7 @@ package drive import ( "context" + "strings" "testing" "time" @@ -65,30 +66,117 @@ func createDeleteWorkflowDoc(t *testing.T, ctx context.Context, folderToken, tit return docToken } +const deleteWorkflowMaxAttempts = 3 + +// deleteWorkflowRetryBackoff paces delete retries after a non-retryable +// failure whose target still exists. Unit tests shrink it. +var deleteWorkflowRetryBackoff = driveDeleteVisibilityPoll + +// deleteAsyncAndVerify deletes token and converges every server outcome to the +// real postcondition: the resource is gone. Async deletes (non-empty task_id) +// additionally verify the task via drive +task_result; sync deletes (empty +// task_id) skip task polling; non-retryable delete failures (e.g. a transient +// "drive task failed") pass when the resource is already gone and are retried +// up to deleteWorkflowMaxAttempts times otherwise. func deleteAsyncAndVerify(t *testing.T, ctx context.Context, token, docType string) string { t.Helper() - result, err := clie2e.RunCmdWithRetry(ctx, clie2e.Request{ - Args: []string{"drive", "+delete", "--file-token", token, "--type", docType, "--yes"}, - DefaultAs: "bot", - }, driveDeleteRetry) - require.NoError(t, err) - result.AssertExitCode(t, 0) - result.AssertStdoutStatus(t, true) + var lastResult *clie2e.Result + for attempt := 1; attempt <= deleteWorkflowMaxAttempts; attempt++ { + result, err := clie2e.RunCmdWithRetry(ctx, clie2e.Request{ + Args: []string{"drive", "+delete", "--file-token", token, "--type", docType, "--yes"}, + DefaultAs: "bot", + }, driveDeleteRetry) + require.NoError(t, err) + lastResult = result + + if result.ExitCode == 0 { + result.AssertStdoutStatus(t, true) + taskID := gjson.Get(result.Stdout, "data.task_id").String() + if taskID == "" { + // Sync completion: the server deleted the resource inline and + // returned no task to poll. + require.True(t, gjson.Get(result.Stdout, "data.deleted").Bool(), "sync delete must report deleted=true\nstdout:\n%s", result.Stdout) + t.Logf("drive +delete completed synchronously for %s %s (no task_id)", docType, token) + } else { + assertDriveDeleteTaskSucceeded(t, ctx, taskID) + } + require.NoError(t, waitDriveResourceDeleted(ctx, token, docType, "bot", driveDeleteVisibilityWait)) + return taskID + } + + // Only the one verified backend transient may fall through to + // terminal-state checking; any other failure is a real regression and + // must not be rescued by the resource happening to be gone. + if !isTransientDriveDeleteFailure(result) { + t.Fatalf("drive +delete failed with an unexpected error on attempt %d\nstdout:\n%s\nstderr:\n%s", + attempt, result.Stdout, result.Stderr) + } + + // The failed delete task may still have removed the resource + // server-side, so check the real terminal state before retrying. + deleted, verifyErr := IsDriveResourceDeleted(ctx, token, docType, "bot") + require.NoError(t, verifyErr, "verify %s %s after failed delete attempt %d", docType, token, attempt) + if deleted { + t.Logf("drive +delete attempt %d failed transiently but %s %s is gone: stderr=%s", attempt, docType, token, result.Stderr) + return "" + } + if attempt < deleteWorkflowMaxAttempts { + t.Logf("drive +delete attempt %d failed and %s %s still exists; retrying: stderr=%s", attempt, docType, token, result.Stderr) + time.Sleep(deleteWorkflowRetryBackoff) + } + } + + t.Fatalf("drive +delete failed %d times and %s %s still exists\nstdout:\n%s\nstderr:\n%s", + deleteWorkflowMaxAttempts, docType, token, lastResult.Stdout, lastResult.Stderr) + return "" +} - taskID := gjson.Get(result.Stdout, "data.task_id").String() - require.NotEmpty(t, taskID, "delete must return async task_id\nstdout:\n%s", result.Stdout) +func assertDriveDeleteTaskSucceeded(t *testing.T, ctx context.Context, taskID string) { + t.Helper() taskResult, err := clie2e.RunCmd(ctx, clie2e.Request{ Args: []string{"drive", "+task_result", "--scenario", "task_check", "--task-id", taskID}, DefaultAs: "bot", }) require.NoError(t, err) - taskResult.AssertExitCode(t, 0) + require.NotNil(t, taskResult) + // Fatal exit-code gate first: the non-fatal assert flavor would cascade + // into misleading empty-stdout failures, exactly what this fix removes. + require.Equal(t, 0, taskResult.ExitCode, "drive +task_result failed\nstdout:\n%s\nstderr:\n%s", taskResult.Stdout, taskResult.Stderr) taskResult.AssertStdoutStatus(t, true) require.Equal(t, taskID, gjson.Get(taskResult.Stdout, "data.task_id").String(), "stdout:\n%s", taskResult.Stdout) - require.False(t, gjson.Get(taskResult.Stdout, "data.failed").Bool(), "stdout:\n%s", taskResult.Stdout) + // gjson returns false for an absent field too, so require presence or a + // malformed task envelope would pass validation. + failedField := gjson.Get(taskResult.Stdout, "data.failed") + require.True(t, failedField.Exists(), "task result must report data.failed\nstdout:\n%s", taskResult.Stdout) + require.False(t, failedField.Bool(), "stdout:\n%s", taskResult.Stdout) +} - require.NoError(t, waitDriveResourceDeleted(ctx, token, docType, "bot", driveDeleteVisibilityWait)) - return taskID +// isTransientDriveDeleteFailure reports whether a failed drive +delete carries +// the one backend error this workflow tolerates: the async delete task +// transiently reporting a terminal "fail" state (observed as flake in CI; the +// resource is usually deleted regardless). Everything else — crashes, protocol +// regressions, auth or parameter errors — stays fatal. +func isTransientDriveDeleteFailure(result *clie2e.Result) bool { + if result == nil { + return false + } + for _, raw := range []string{result.Stderr, result.Stdout} { + idx := strings.Index(raw, "{") + if idx < 0 { + continue + } + payload := raw[idx:] + if !gjson.Valid(payload) { + continue + } + errObj := gjson.Get(payload, "error") + if errObj.Get("type").String() == "api" && + errObj.Get("subtype").String() == "server_error" && + errObj.Get("message").String() == "drive task failed" { + return true + } + } + return false }