diff --git a/RewindTests/FavoritesModelTests.swift b/RewindTests/FavoritesModelTests.swift new file mode 100644 index 0000000..51cd35c --- /dev/null +++ b/RewindTests/FavoritesModelTests.swift @@ -0,0 +1,112 @@ +// +// FavoritesModelTests.swift +// RewindTests +// +// FavoritesModel dedups add/remove by Model.Image's cid-only equality (Model.Image.== compares +// only `cid`, ignoring every other field). These tests pin that behavior with two DISTINCT +// instances sharing a cid but differing in every other field, so a port that widened equality to +// the full struct (or compared some other key) would fail: it would either grow a duplicate entry +// on re-favoriting the same photo from a different source screen, or fail to find/remove the +// stored entry when the caller only has a partially-populated instance (e.g. one rehydrated from +// persisted Storage.Image, which carries fewer fields than a freshly-fetched Model.Image). +// + +import CoreLocation +@testable import Rewind +import Testing +import VGSL + +@MainActor +struct FavoritesModelTests { + /// Adding an image whose cid already exists in favorites is a no-op: the original entry is kept + /// (not replaced), and the synchronous storage effect is not re-invoked for the duplicate. + @Test func addToFavoritesDedupesByCidKeepingTheFirstEntry() { + let storage = StorageSpy() + let model = makeFavoritesModel(storage: storage.property) + + let first = makeImage(cid: 1, title: "A", imagePath: "a.jpg", year: 1900) + let sameCidDifferentEverythingElse = makeImage( + cid: 1, + title: "B", + imagePath: "b.jpg", + year: 1950 + ) + + model(.addToFavorites(first)) + model(.addToFavorites(sameCidDifferentEverythingElse)) + + #expect(model.state.count == 1) + #expect(model.state.first?.title == "A") + #expect(model.state.first?.imagePath == "a.jpg") + + // The second dispatch hit the `guard !state.contains(image)` and returned before enqueuing + // the persistence effect at all — not just "persisted the same thing twice". + #expect(storage.writes.count == 1) + #expect(storage.writes.last?.map(\.title) == ["A"]) + } + + /// Removing an image finds the stored entry by cid alone: an instance with the same cid but + /// different title/imagePath/coordinate still matches and is removed; an unrelated cid is a + /// no-op that neither changes state nor re-invokes the storage effect. + @Test func removeFromFavoritesMatchesStoredEntryByCidAlone() { + let toRemove = makeImage(cid: 1, title: "A", imagePath: "a.jpg", year: 1900) + let toKeep = makeImage(cid: 2, title: "B", imagePath: "b.jpg", year: 1950) + let storage = StorageSpy(initial: [toRemove, toKeep]) + let model = makeFavoritesModel(storage: storage.property) + + let differentInstanceSameCid = modified( + makeImage(cid: 1, title: "Z", imagePath: "z.jpg", year: 2020), + ) { + $0.coordinate = CLLocationCoordinate2D(latitude: 10, longitude: 20) + } + model(.removeFromFavorites(differentInstanceSameCid)) + + #expect(model.state.count == 1) + #expect(model.state.first?.cid == 2) + #expect(model.state.first?.title == "B") + #expect(storage.writes.count == 1) + #expect(storage.writes.last?.map(\.title) == ["B"]) + + // An unrelated cid finds no index (`firstIndex(of:)` returns nil) — state and storage are + // both left untouched, not just "unchanged after another identical write". + model(.removeFromFavorites(makeImage(cid: 99, title: "Nope", imagePath: "nope.jpg", year: 1))) + #expect(model.state.count == 1) + #expect(storage.writes.count == 1) + } +} + +// MARK: - Fixtures + +private func makeImage( + cid: Int, + title: String, + imagePath: String, + year: Int, +) -> Model.Image { + modified(Model.Image.mock) { + $0.cid = cid + $0.title = title + $0.imagePath = imagePath + $0.date = ImageDate(year: year, year2: year) + } +} + +@MainActor +private final class StorageSpy { + private(set) var writes: [[Model.Image]] = [] + private var value: [Model.Image] + + init(initial: [Model.Image] = []) { + value = initial + } + + var property: Property<[Model.Image]> { + Property( + getter: { [weak self] in self?.value ?? [] }, + setter: { [weak self] newValue in + self?.value = newValue + self?.writes.append(newValue) + }, + ) + } +} diff --git a/RewindTests/ImageDetailsModelTests.swift b/RewindTests/ImageDetailsModelTests.swift new file mode 100644 index 0000000..12a1cf0 --- /dev/null +++ b/RewindTests/ImageDetailsModelTests.swift @@ -0,0 +1,109 @@ +// +// ImageDetailsModelTests.swift +// RewindTests +// +// Tests the description-link routing in the ImageDetails reducer: a link to a pastvu photo +// (https://pastvu.com/p/) recurses into a nested ImageDetails screen loaded from the remote, +// while any other link (external host, or a non-photo pastvu path) is handed to the url opener. +// The routing hinges on a branchy URL parse (host + /p/ path + integer cid), which is exactly the +// kind of quirk worth pinning; the raw button/url string tables are not tested. +// + +import Foundation +@testable import Rewind +import Testing +import VGSL + +@MainActor +struct ImageDetailsModelTests { + /// A link to a pastvu photo loads that photo's details from the remote and presents them as a + /// nested details screen; loadingAnotherImage flips true while the load is in flight, and the + /// external url opener is never called. + @Test func pastvuPhotoLinkOpensNestedDetails() async { + let harness = Harness() + let model = harness.makeModel() + + let linkedCid = 2_223_969 + model(.descriptionLink(pastvuPhotoURL(linkedCid))) + #expect(model.state.loadingAnotherImage) // set synchronously, before the load returns + + #expect(await eventually { model.state.anotherImageModel != nil }) + #expect(!model.state.loadingAnotherImage) // cleared once the nested model is presented + #expect(harness.requestedCids == [linkedCid]) // the remote was asked for the linked photo + #expect(harness.openedURLs.isEmpty) // recursion, not a browser hand-off + } + + /// A link to an external host is opened in the browser rather than recursing. + @Test func externalLinkOpensInBrowser() async throws { + let harness = Harness() + let model = harness.makeModel() + + let external = try #require(URL(string: "https://example.com/gallery")) + model(.descriptionLink(external)) + + #expect(await eventually { harness.openedURLs == [external] }) + #expect(!model.state.loadingAnotherImage) + #expect(model.state.anotherImageModel == nil) + #expect(harness.requestedCids.isEmpty) + } + + /// A pastvu link that is not a photo page (e.g. a user profile) is opened in the browser, not + /// mistaken for a photo to recurse into. + @Test func nonPhotoPastvuLinkOpensInBrowser() async throws { + let harness = Harness() + let model = harness.makeModel() + + let userPage = try #require(URL(string: "https://pastvu.com/u/someone")) + model(.descriptionLink(userPage)) + + #expect(await eventually { harness.openedURLs == [userPage] }) + #expect(model.state.anotherImageModel == nil) + #expect(harness.requestedCids.isEmpty) + } +} + +private func pastvuPhotoURL(_ cid: Int) -> URL { + URL(string: "https://pastvu.com/p/\(cid)")! +} + +@MainActor +private final class Harness { + private(set) var openedURLs: [URL] = [] + private(set) var requestedCids: [Int] = [] + + // Both closures below are non-Sendable and formed in this @MainActor context, so they inherit + // main-actor isolation — their bodies hop back to the main actor before touching harness state. + func makeModel() -> ImageDetailsModel { + makeImageDetailsModel( + modelImage: .mock, + remote: Remote { [weak self] cid in + self?.requestedCids.append(cid) + return modified(.mock) { $0.cid = cid } + }, + openSource: "", + favoritesModel: .mock, + showOnMap: { _ in }, + canOpenURL: { _ in true }, + urlOpener: { [weak self] in self?.openedURLs.append($0) }, + setOrientationLock: { _ in }, + streetViewAvailability: .mock(.unavailable), + translate: .mock("translated"), + extractModelImage: { _ in .mock }, + ) + } +} + +// MARK: - Async helpers (mirrors MapModelTests: async effects run in a Task we can't await) + +@MainActor +private func eventually( + timeout: Duration = .seconds(2), + _ condition: () -> Bool, +) async -> Bool { + let deadline = ContinuousClock().now.advanced(by: timeout) + while !condition() { + if ContinuousClock().now >= deadline { return false } + try? await Task.sleep(for: .milliseconds(5)) + } + return true +}