From f6add1598eb716b09b79e2314e11923d94b782f1 Mon Sep 17 00:00:00 2001
From: Jerry Yan <792602257@qq.com>
Date: Wed, 6 May 2026 08:14:47 +0800
Subject: [PATCH] test(jobs/runners): cover partial-storage-success, list-error
 isolation, DB-delete-error branches

---
 .../jobs/runners/attachment_cleanup_test.go   | 77 +++++++++++++++++--
 1 file changed, 70 insertions(+), 7 deletions(-)

diff --git a/internal/jobs/runners/attachment_cleanup_test.go b/internal/jobs/runners/attachment_cleanup_test.go
index f6b758e..1a2a96d 100644
--- a/internal/jobs/runners/attachment_cleanup_test.go
+++ b/internal/jobs/runners/attachment_cleanup_test.go
@@ -14,31 +14,45 @@ import (
 )
 
 type fakeAttachmentRepo struct {
-	softDel    []runners.AttachmentRow
-	orphan     []runners.AttachmentRow
-	deletedIDs []uuid.UUID
+	softDel        []runners.AttachmentRow
+	orphan         []runners.AttachmentRow
+	softDelListErr error
+	orphanListErr  error
+	deleteErr      error
+	deletedIDs     []uuid.UUID
+	softDelCalled  bool
+	orphanCalled   bool
 }
 
 func (f *fakeAttachmentRepo) ListAttachmentsForSoftDeletedConversations(_ context.Context, _ time.Duration) ([]runners.AttachmentRow, error) {
-	return f.softDel, nil
+	f.softDelCalled = true
+	return f.softDel, f.softDelListErr
 }
 func (f *fakeAttachmentRepo) ListExpiredOrphans(_ context.Context, _ time.Duration) ([]runners.AttachmentRow, error) {
-	return f.orphan, nil
+	f.orphanCalled = true
+	return f.orphan, f.orphanListErr
 }
 func (f *fakeAttachmentRepo) DeleteAttachments(_ context.Context, ids []uuid.UUID) error {
+	if f.deleteErr != nil {
+		return f.deleteErr
+	}
 	f.deletedIDs = append(f.deletedIDs, ids...)
 	return nil
 }
 
 type fakeStorage struct {
-	deleted []string
-	err     error
+	deleted   []string
+	err       error
+	failPaths map[string]error
 }
 
 func (s *fakeStorage) Delete(_ context.Context, path string) error {
 	if s.err != nil {
 		return s.err
 	}
+	if e, ok := s.failPaths[path]; ok {
+		return e
+	}
 	s.deleted = append(s.deleted, path)
 	return nil
 }
@@ -71,3 +85,52 @@ func TestAttachmentCleanup_StorageErrorDoesNotDeleteFromDB(t *testing.T) {
 	require.Empty(t, st.deleted, "storage error → no path recorded")
 	require.Empty(t, repo.deletedIDs, "storage error → no DB delete batch")
 }
+
+func TestAttachmentCleanup_PartialStorageSuccessOnlyDeletesSuccessfulIDs(t *testing.T) {
+	t.Parallel()
+	succID := uuid.New()
+	failID := uuid.New()
+	repo := &fakeAttachmentRepo{
+		softDel: []runners.AttachmentRow{
+			{ID: succID, StoragePath: "ok"},
+			{ID: failID, StoragePath: "bad"},
+		},
+	}
+	st := &fakeStorage{failPaths: map[string]error{"bad": errors.New("permission denied")}}
+	r := runners.NewAttachmentCleanup(repo, st, runners.AttachmentCleanupConfig{Retention: 30 * 24 * time.Hour}, discardLogger())
+	r.Run(context.Background())
+	assert.ElementsMatch(t, []string{"ok"}, st.deleted, "only successful storage delete recorded")
+	assert.ElementsMatch(t, []uuid.UUID{succID}, repo.deletedIDs, "DB delete batch only contains successfully-deleted-from-storage IDs")
+}
+
+func TestAttachmentCleanup_SoftDelListErrorDoesNotPreventOrphanPhase(t *testing.T) {
+	t.Parallel()
+	orphanID := uuid.New()
+	repo := &fakeAttachmentRepo{
+		softDelListErr: errors.New("db blip"),
+		orphan:         []runners.AttachmentRow{{ID: orphanID, StoragePath: "p"}},
+	}
+	st := &fakeStorage{}
+	r := runners.NewAttachmentCleanup(repo, st, runners.AttachmentCleanupConfig{Retention: 30 * 24 * time.Hour}, discardLogger())
+	r.Run(context.Background())
+	assert.True(t, repo.softDelCalled, "soft-del list was attempted")
+	assert.True(t, repo.orphanCalled, "orphan list still ran after soft-del list error")
+	assert.ElementsMatch(t, []string{"p"}, st.deleted, "orphan storage delete proceeded")
+	assert.ElementsMatch(t, []uuid.UUID{orphanID}, repo.deletedIDs)
+}
+
+func TestAttachmentCleanup_DBDeleteErrorIsLoggedNotPanicked(t *testing.T) {
+	t.Parallel()
+	id := uuid.New()
+	repo := &fakeAttachmentRepo{
+		softDel:   []runners.AttachmentRow{{ID: id, StoragePath: "p"}},
+		deleteErr: errors.New("constraint violation"),
+	}
+	st := &fakeStorage{}
+	r := runners.NewAttachmentCleanup(repo, st, runners.AttachmentCleanupConfig{Retention: 30 * 24 * time.Hour}, discardLogger())
+	require.NotPanics(t, func() {
+		r.Run(context.Background())
+	}, "DB delete error must not panic")
+	assert.ElementsMatch(t, []string{"p"}, st.deleted, "storage delete proceeded before DB error")
+	assert.Empty(t, repo.deletedIDs, "deleteErr means no IDs accumulated in fake")
+}