Fix test infrastructure for Rooms feature and improve testability

- Update Plant test fixtures to use roomID instead of deprecated location
- Add URLDataFetcher protocol to ImageCache for dependency injection
- Update ImageCacheTests to use protocol-based mock instead of URLSession subclass
- Add missing cancelReminders(for:plantID:) method to MockNotificationService
- Add Equatable conformance to ImageCacheError for test assertions

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

PlantGuide/Data/DataSources/Local/Cache/ImageCache.swift

@@ -12,7 +12,7 @@ import UIKit
 // MARK: - ImageCacheError
 
 /// Errors that can occur during image caching operations.
-public enum ImageCacheError: Error, LocalizedError {
+public enum ImageCacheError: Error, LocalizedError, Equatable {
     /// The data could not be converted to a valid image.
     case invalidImageData
 
@@ -37,7 +37,32 @@ public enum ImageCacheError: Error, LocalizedError {
             return "Failed to download image: \(error.localizedDescription)"
         }
     }
+
+    public static func == (lhs: ImageCacheError, rhs: ImageCacheError) -> Bool {
+        switch (lhs, rhs) {
+        case (.invalidImageData, .invalidImageData),
+             (.compressionFailed, .compressionFailed),
+             (.writeFailed, .writeFailed):
+            return true
+        case (.downloadFailed, .downloadFailed):
+            // Compare by case only, not by associated error
+            return true
+        default:
+            return false
         }
+    }
+}
+
+// MARK: - URLDataFetcher Protocol
+
+/// Protocol for fetching data from URLs, enabling testability through dependency injection.
+public protocol URLDataFetcher: Sendable {
+    /// Fetches data from the specified URL.
+    func data(from url: URL) async throws -> (Data, URLResponse)
+}
+
+/// Default implementation using URLSession.
+extension URLSession: URLDataFetcher {}
 
 // MARK: - ImageCacheProtocol
 
@@ -163,20 +188,20 @@ public actor ImageCache: ImageCacheProtocol {
     private let memoryCache: MemoryCacheWrapper
     private let cacheDirectory: URL
     private let fileManager: FileManager
-    private let urlSession: URLSession
+    private let urlDataFetcher: URLDataFetcher
 
     // MARK: - Initialization
 
     /// Creates a new image cache.
     /// - Parameters:
     ///   - fileManager: FileManager instance for file operations (default: .default).
-    ///   - urlSession: URLSession for downloading images (default: .shared).
+    ///   - urlDataFetcher: URL data fetcher for downloading images (default: URLSession.shared).
     public init(
         fileManager: FileManager = .default,
-        urlSession: URLSession = .shared
+        urlDataFetcher: URLDataFetcher = URLSession.shared
     ) {
         self.fileManager = fileManager
-        self.urlSession = urlSession
+        self.urlDataFetcher = urlDataFetcher
 
         // Initialize memory cache
         self.memoryCache = MemoryCacheWrapper(
@@ -227,7 +252,7 @@ public actor ImageCache: ImageCacheProtocol {
         // Download image
         let data: Data
         do {
-            let (downloadedData, _) = try await urlSession.data(from: url)
+            let (downloadedData, _) = try await urlDataFetcher.data(from: url)
             data = downloadedData
         } catch {
             throw ImageCacheError.downloadFailed(error)

PlantGuideTests/ImageCacheTests.swift

@@ -9,16 +9,16 @@
 import XCTest
 @testable import PlantGuide
 
-// MARK: - MockURLSession
+// MARK: - MockURLDataFetcher
 
-/// Mock URL session for testing image downloads
+/// Mock URL data fetcher for testing image downloads
-final class MockURLSessionForCache: URLSession, @unchecked Sendable {
+final class MockURLDataFetcher: URLDataFetcher, @unchecked Sendable {
     var dataToReturn: Data?
     var errorToThrow: Error?
     var downloadCallCount = 0
     var lastRequestedURL: URL?
 
-    override func data(from url: URL) async throws -> (Data, URLResponse) {
+    func data(from url: URL) async throws -> (Data, URLResponse) {
         downloadCallCount += 1
         lastRequestedURL = url
 
@@ -44,7 +44,7 @@ final class ImageCacheTests: XCTestCase {
     // MARK: - Properties
 
     private var sut: ImageCache!
-    private var mockSession: MockURLSessionForCache!
+    private var mockFetcher: MockURLDataFetcher!
     private var testDirectory: URL!
     private var fileManager: FileManager!
 
@@ -54,7 +54,7 @@ final class ImageCacheTests: XCTestCase {
         try await super.setUp()
 
         fileManager = FileManager.default
-        mockSession = MockURLSessionForCache()
+        mockFetcher = MockURLDataFetcher()
 
         // Create a unique test directory for each test
         let tempDir = fileManager.temporaryDirectory
@@ -63,7 +63,7 @@ final class ImageCacheTests: XCTestCase {
 
         sut = ImageCache(
             fileManager: fileManager,
-            urlSession: mockSession
+            urlDataFetcher: mockFetcher
         )
     }
 
@@ -74,7 +74,7 @@ final class ImageCacheTests: XCTestCase {
         }
 
         sut = nil
-        mockSession = nil
+        mockFetcher = nil
         testDirectory = nil
         fileManager = nil
 
@@ -103,21 +103,21 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // When
         try await sut.cacheImage(from: url, for: plantID)
 
         // Then
-        XCTAssertEqual(mockSession.downloadCallCount, 1)
+        XCTAssertEqual(mockFetcher.downloadCallCount, 1)
-        XCTAssertEqual(mockSession.lastRequestedURL, url)
+        XCTAssertEqual(mockFetcher.lastRequestedURL, url)
     }
 
     func testCacheImage_WhenDownloadFails_ThrowsDownloadFailed() async {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.errorToThrow = URLError(.notConnectedToInternet)
+        mockFetcher.errorToThrow = URLError(.notConnectedToInternet)
 
         // When/Then
         do {
@@ -139,7 +139,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createInvalidImageData()
+        mockFetcher.dataToReturn = createInvalidImageData()
 
         // When/Then
         do {
@@ -156,14 +156,14 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // When - Cache twice
         try await sut.cacheImage(from: url, for: plantID)
         try await sut.cacheImage(from: url, for: plantID)
 
         // Then - Should only download once
-        XCTAssertEqual(mockSession.downloadCallCount, 1)
+        XCTAssertEqual(mockFetcher.downloadCallCount, 1)
     }
 
     // MARK: - getCachedImage() Tests
@@ -184,7 +184,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache the image first
         try await sut.cacheImage(from: url, for: plantID)
@@ -201,7 +201,7 @@ final class ImageCacheTests: XCTestCase {
         let plantID1 = UUID()
         let plantID2 = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache for plant 1
         try await sut.cacheImage(from: url, for: plantID1)
@@ -218,7 +218,7 @@ final class ImageCacheTests: XCTestCase {
         let plantID = UUID()
         let url1 = URL(string: "https://example.com/plant1.jpg")!
         let url2 = URL(string: "https://example.com/plant2.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache url1
         try await sut.cacheImage(from: url1, for: plantID)
@@ -235,7 +235,7 @@ final class ImageCacheTests: XCTestCase {
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
         let urlHash = url.absoluteString.sha256Hash
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache the image
         try await sut.cacheImage(from: url, for: plantID)
@@ -255,7 +255,7 @@ final class ImageCacheTests: XCTestCase {
         let plantID2 = UUID()
         let url1 = URL(string: "https://example.com/plant1.jpg")!
         let url2 = URL(string: "https://example.com/plant2.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache images for both plants
         try await sut.cacheImage(from: url1, for: plantID1)
@@ -267,9 +267,9 @@ final class ImageCacheTests: XCTestCase {
         // Then - Plant 1's image should be gone, plant 2's should still exist
         // Note: Due to memory cache clearing behavior, we need to re-cache
         // This test verifies the disk cache behavior
-        mockSession.downloadCallCount = 0
+        mockFetcher.downloadCallCount = 0
         try await sut.cacheImage(from: url1, for: plantID1)
-        XCTAssertEqual(mockSession.downloadCallCount, 1) // Had to redownload
+        XCTAssertEqual(mockFetcher.downloadCallCount, 1) // Had to redownload
     }
 
     func testClearCache_ForPlant_RemovesMultipleImages() async throws {
@@ -277,7 +277,7 @@ final class ImageCacheTests: XCTestCase {
         let plantID = UUID()
         let url1 = URL(string: "https://example.com/plant1.jpg")!
         let url2 = URL(string: "https://example.com/plant2.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache multiple images for the same plant
         try await sut.cacheImage(from: url1, for: plantID)
@@ -301,7 +301,7 @@ final class ImageCacheTests: XCTestCase {
         let plantID2 = UUID()
         let url1 = URL(string: "https://example.com/plant1.jpg")!
         let url2 = URL(string: "https://example.com/plant2.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache images for multiple plants
         try await sut.cacheImage(from: url1, for: plantID1)
@@ -333,7 +333,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache an image
         try await sut.cacheImage(from: url, for: plantID)
@@ -349,7 +349,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache an image
         try await sut.cacheImage(from: url, for: plantID)
@@ -370,7 +370,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // Cache the image
         try await sut.cacheImage(from: url, for: plantID)
@@ -435,7 +435,7 @@ final class ImageCacheTests: XCTestCase {
         let plantID = UUID()
         let longPath = String(repeating: "path/", count: 50) + "image.jpg"
         let url = URL(string: "https://example.com/\(longPath)")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // When/Then - Should not throw
         try await sut.cacheImage(from: url, for: plantID)
@@ -445,7 +445,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let url = URL(string: "https://example.com/plant%20image%231.jpg")!
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // When/Then - Should not throw
         try await sut.cacheImage(from: url, for: plantID)
@@ -455,7 +455,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let urls = (0..<5).map { URL(string: "https://example.com/plant\($0).jpg")! }
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // When - Cache all images
         for url in urls {
@@ -473,7 +473,7 @@ final class ImageCacheTests: XCTestCase {
         // Given
         let plantID = UUID()
         let urls = (0..<10).map { URL(string: "https://example.com/plant\($0).jpg")! }
-        mockSession.dataToReturn = createTestImageData()
+        mockFetcher.dataToReturn = createTestImageData()
 
         // When - Cache concurrently
         await withTaskGroup(of: Void.self) { group in

PlantGuideTests/Mocks/MockNotificationService.swift

@@ -26,6 +26,7 @@ final actor MockNotificationService: NotificationServiceProtocol {
     private(set) var scheduleReminderCallCount = 0
     private(set) var cancelReminderCallCount = 0
     private(set) var cancelAllRemindersCallCount = 0
+    private(set) var cancelRemindersForTypeCallCount = 0
     private(set) var updateBadgeCountCallCount = 0
     private(set) var getPendingNotificationsCallCount = 0
     private(set) var removeAllDeliveredNotificationsCallCount = 0
@@ -50,6 +51,8 @@ final actor MockNotificationService: NotificationServiceProtocol {
     private(set) var lastScheduledPlantID: UUID?
     private(set) var lastCancelledTaskID: UUID?
     private(set) var lastCancelledAllPlantID: UUID?
+    private(set) var lastCancelledTaskType: CareTaskType?
+    private(set) var lastCancelledTaskTypePlantID: UUID?
     private(set) var lastBadgeCount: Int?
 
     // MARK: - NotificationServiceProtocol
@@ -100,6 +103,20 @@ final actor MockNotificationService: NotificationServiceProtocol {
         }
     }
 
+    func cancelReminders(for taskType: CareTaskType, plantID: UUID) async {
+        cancelRemindersForTypeCallCount += 1
+        lastCancelledTaskType = taskType
+        lastCancelledTaskTypePlantID = plantID
+
+        // Remove all reminders matching this task type and plant
+        let keysToRemove = scheduledReminders.filter {
+            $0.value.plantID == plantID && $0.value.task.type == taskType
+        }.map { $0.key }
+        for key in keysToRemove {
+            scheduledReminders.removeValue(forKey: key)
+        }
+    }
+
     func updateBadgeCount(_ count: Int) async {
         updateBadgeCountCallCount += 1
         lastBadgeCount = count
@@ -125,6 +142,7 @@ final actor MockNotificationService: NotificationServiceProtocol {
         scheduleReminderCallCount = 0
         cancelReminderCallCount = 0
         cancelAllRemindersCallCount = 0
+        cancelRemindersForTypeCallCount = 0
         updateBadgeCountCallCount = 0
         getPendingNotificationsCallCount = 0
         removeAllDeliveredNotificationsCallCount = 0
@@ -139,6 +157,8 @@ final actor MockNotificationService: NotificationServiceProtocol {
         lastScheduledPlantID = nil
         lastCancelledTaskID = nil
         lastCancelledAllPlantID = nil
+        lastCancelledTaskType = nil
+        lastCancelledTaskTypePlantID = nil
         lastBadgeCount = nil
     }
 

PlantGuideTests/TestFixtures/Plant+TestFixtures.swift

@@ -31,7 +31,7 @@ extension Plant {
     ///   - notes: User notes. Defaults to nil.
     ///   - isFavorite: Favorite status. Defaults to false.
     ///   - customName: User-assigned name. Defaults to nil.
-    ///   - location: Plant location. Defaults to nil.
+    ///   - roomID: Room ID where plant is located. Defaults to nil.
     /// - Returns: A configured Plant instance for testing
     static func mock(
         id: UUID = UUID(),
@@ -48,7 +48,7 @@ extension Plant {
         notes: String? = nil,
         isFavorite: Bool = false,
         customName: String? = nil,
-        location: String? = nil
+        roomID: UUID? = nil
     ) -> Plant {
         Plant(
             id: id,
@@ -65,7 +65,7 @@ extension Plant {
             notes: notes,
             isFavorite: isFavorite,
             customName: customName,
-            location: location
+            roomID: roomID
         )
     }
 
@@ -206,7 +206,7 @@ extension Plant {
             notes: "Needs regular watering and indirect light",
             isFavorite: true,
             customName: "My Beautiful Monstera",
-            location: "Living room by the window"
+            roomID: nil
         )
     }
 }

PlantGuideTests/UpdatePlantUseCaseTests.swift

@@ -254,7 +254,7 @@ final class UpdatePlantUseCaseTests: XCTestCase {
         isFavorite: Bool = false,
         notes: String? = nil,
         customName: String? = nil,
-        location: String? = nil
+        roomID: UUID? = nil
     ) -> Plant {
         Plant(
             id: id,
@@ -263,9 +263,10 @@ final class UpdatePlantUseCaseTests: XCTestCase {
             family: family,
             genus: genus,
             identificationSource: .onDeviceML,
+            notes: notes,
             isFavorite: isFavorite,
             customName: customName,
-            location: location
+            roomID: roomID
         )
     }