diff --git a/Yattee/Core/InstancesManager.swift b/Yattee/Core/InstancesManager.swift index ee35ed0e..fb935402 100644 --- a/Yattee/Core/InstancesManager.swift +++ b/Yattee/Core/InstancesManager.swift @@ -26,7 +26,7 @@ enum InstanceStatus: Equatable { final class InstancesManager { // MARK: - Storage - private let localDefaults = UserDefaults.standard + private let localDefaults: UserDefaults private let ubiquitousStore = NSUbiquitousKeyValueStore.default private let instancesKey = "configuredInstances" private let activeInstanceKey = "activeInstanceID" @@ -45,8 +45,13 @@ final class InstancesManager { // MARK: - Initialization - init(settingsManager: SettingsManager? = nil) { + /// - Parameter defaults: where the configured instances are read and + /// written. The app's own defaults, except under test: a manager built on + /// the shared ones edits the list the app on that device is using, which + /// is how test fixtures ended up in it. + init(settingsManager: SettingsManager? = nil, defaults: UserDefaults = .standard) { self.settingsManager = settingsManager + self.localDefaults = defaults loadInstances() loadActiveInstance() diff --git a/Yattee/Models/Video.swift b/Yattee/Models/Video.swift index c6e9b295..0ef560c5 100644 --- a/Yattee/Models/Video.swift +++ b/Yattee/Models/Video.swift @@ -85,6 +85,41 @@ struct Video: Identifiable, Codable, Sendable { self.relatedVideos = relatedVideos } + // MARK: - Standing in for metadata + + /// A video that is nothing but its own ID, for a list that knows a video was + /// watched before it knows anything else about it. + /// + /// An account's server-side history is a list of IDs; the rows have to be + /// on screen while their metadata is still being fetched, and one whose + /// fetch fails is still part of that history. + static func metadataPlaceholder(for videoID: String) -> Video { + Video( + id: .global(videoID), + title: videoID, + description: nil, + author: Author(id: "", name: ""), + duration: 0, + publishedAt: nil, + publishedText: nil, + viewCount: nil, + likeCount: nil, + thumbnails: [], + isLive: false, + isUpcoming: false, + scheduledStartTime: nil + ) + } + + /// Whether the title is only the video's own ID standing in for one. + /// + /// Anything showing this video beside details it fetched itself should + /// prefer the fetched title: a bare ID is not a name anybody chose, and the + /// screen would otherwise keep it under a real description and thumbnail. + var hasPlaceholderTitle: Bool { + title == id.videoID + } + // MARK: - Computed Properties var bestThumbnail: Thumbnail? { @@ -429,17 +464,39 @@ extension Video { // MARK: - Equatable & Hashable extension Video: Equatable { - /// Custom Equatable implementation that only compares by ID for performance. - /// Two videos with the same ID are considered equal during SwiftUI diffing. - /// This avoids expensive comparisons of all properties (including nested Author and [Thumbnail]). + /// Compares what the video *is*, not only which video it is. + /// + /// Being the same video is not the same as being the same *description* of + /// it. One ID can arrive first as a placeholder standing in for metadata + /// nobody has fetched yet and again as the real thing, and SwiftUI decides + /// whether a view needs redrawing by comparing the values it was given — so + /// equality by ID alone leaves a list showing the placeholder it was handed + /// first, however many times it is given the answer. + /// + /// The ID is compared first, which is what nearly every comparison differs + /// in and settles it in one step. static func == (lhs: Video, rhs: Video) -> Bool { lhs.id == rhs.id + && lhs.title == rhs.title + && lhs.author == rhs.author + && lhs.duration == rhs.duration + && lhs.viewCount == rhs.viewCount + && lhs.likeCount == rhs.likeCount + && lhs.publishedAt == rhs.publishedAt + && lhs.publishedText == rhs.publishedText + && lhs.isLive == rhs.isLive + && lhs.isUpcoming == rhs.isUpcoming + && lhs.scheduledStartTime == rhs.scheduledStartTime + && lhs.thumbnails == rhs.thumbnails + && lhs.description == rhs.description + && lhs.relatedVideos == rhs.relatedVideos } } extension Video: Hashable { - /// Custom Hashable implementation consistent with Equatable. - /// Only hashes the ID since equality is determined by ID alone. + /// Hashed by ID alone, which stays coarser than equality on purpose: it is + /// what every lookup is by, and hashing equal for values that turn out to + /// differ is exactly what a hash is allowed to do. func hash(into hasher: inout Hasher) { hasher.combine(id) } diff --git a/Yattee/Views/Home/HistoryListView.swift b/Yattee/Views/Home/HistoryListView.swift index 5ba36f4e..850360b3 100644 --- a/Yattee/Views/Home/HistoryListView.swift +++ b/Yattee/Views/Home/HistoryListView.swift @@ -72,6 +72,26 @@ struct HistoryListView: View { @State private var hydrationTasks: [String: Task] = [:] /// The rows waiting for a fetch slot, in the order they asked for one. @State private var hydrationQueue: [String] = [] + /// Every row that has asked for metadata and has not been given any. + /// + /// Unlike ``hydrationAttempted`` this survives a reload, and it is what a + /// reload owes a fetch to afterwards: the work it cancelled would otherwise + /// be nobody's to start again. + @State private var hydrationPending: Set = [] + /// The rows on screen right now. + /// + /// A row asks for its metadata as it appears, and that is the only moment + /// it asks — so an appearance that lands while the account's history is not + /// what the screen is reading (a status still settling, an instance not yet + /// arrived) asks for nothing, and nothing would ever ask again. Remembering + /// what is on screen lets the next load make good on it. + @State private var visibleRowIDs: Set = [] + /// Which set of fetches the slots belong to. + /// + /// Bumped whenever the in-flight fetches are dropped, so a task from before + /// that cannot free a slot the load after it has since given to a fetch of + /// the same video. + @State private var hydrationEpoch = 0 /// How many IDs one page of server history asks for. private static let serverPageSize = 60 @@ -361,8 +381,15 @@ struct HistoryListView: View { } // Selecting another account — or local — changes whose history this is, // and the previous rows may not be shown for a moment longer. + // + // Compared rather than taken on trust: the selection is announced by + // more than the switcher — a repair, an arriving sync, the first + // resolution of the stored choice — and most of those settle on the + // context that is already on screen. Reloading for one of those would + // throw away rows and half-finished metadata to fetch the same history + // again. .onReceive(NotificationCenter.default.publisher(for: .dataContextDidChange)) { _ in - startLoad() + reloadIfEffectiveContextMoved() } // The same, without the context moving: the credential behind the // account was replaced, so the history may now be someone else's. @@ -531,7 +558,10 @@ struct HistoryListView: View { loadMoreVideos: loadMoreHistoryCallback ) .onAppear { - hydrateIfNeeded(row) + rowDidAppear(row) + } + .onDisappear { + visibleRowIDs.remove(row.videoID) } } @@ -572,7 +602,10 @@ struct HistoryListView: View { loadMoreVideos: loadMoreHistoryCallback ) .onAppear { - hydrateIfNeeded(row) + rowDidAppear(row) + } + .onDisappear { + visibleRowIDs.remove(row.videoID) } } } @@ -626,13 +659,24 @@ struct HistoryListView: View { // Recorded on every load, not only on a context change: the first load // of a session has to establish what is on screen, or the first status // change would look like a move and reload for nothing. - loadedContext = appEnvironment.dataContextValidator.effectiveContext + let context = appEnvironment.dataContextValidator.effectiveContext + let movedContext = loadedContext != nil && loadedContext != context + + loadedContext = context loadGeneration += 1 serverErrorMessage = nil isLoadingPage = false hydrationAttempted.removeAll() pageFloor = 1 + // What the rows are owed, and which of them are on screen, only means + // anything while they are the same rows. Another history's IDs would + // never match a row again. + if movedContext { + hydrationPending.removeAll() + visibleRowIDs.removeAll() + } + guard case let .server(account) = displayMode else { hasMorePages = false loadHistory() @@ -693,6 +737,12 @@ struct HistoryListView: View { // view — which, being filtered out, they never are. if !searchText.isEmpty { hydrateAllLoaded(account: account) + } else { + // Rows that were already asking for metadata when this load + // replaced them. They are back, unchanged and still without a + // title, and nothing is going to announce their appearance a + // second time. + resumeHydration(account: account) } } catch is CancellationError { // Another load is taking over — it owns the state now. @@ -738,35 +788,18 @@ struct HistoryListView: View { return HistoryRow(videoID: id, video: entry.toVideo(), entry: entry, isHydrated: true) } - return HistoryRow(videoID: id, video: Self.placeholder(for: id), entry: nil, isHydrated: false) - } - - /// The row a server ID stands as until its metadata arrives — or instead of - /// it, when the fetch fails. - /// - /// The entry is never dropped for want of a title: the account watched this - /// video, and a history missing rows it cannot describe is a wrong history. - private static func placeholder(for videoID: String) -> Video { - Video( - id: .global(videoID), - title: videoID, - description: nil, - author: Author(id: "", name: ""), - duration: 0, - publishedAt: nil, - publishedText: nil, - viewCount: nil, - likeCount: nil, - thumbnails: [], - isLive: false, - isUpcoming: false, - scheduledStartTime: nil - ) + // The entry is never dropped for want of a title: the account watched + // this video, and a history missing rows it cannot describe is a wrong + // history. + return HistoryRow(videoID: id, video: .metadataPlaceholder(for: id), entry: nil, isHydrated: false) } // MARK: - Hydration - private func hydrateIfNeeded(_ row: HistoryRow) { + /// A row came on screen: it is noted as visible, and asks for its metadata. + private func rowDidAppear(_ row: HistoryRow) { + visibleRowIDs.insert(row.videoID) + guard case let .server(account) = displayMode else { return } hydrate(row, account: account) } @@ -784,6 +817,21 @@ struct HistoryListView: View { } } + /// Makes good on the rows a load left without metadata. + /// + /// The ones whose fetch the reload cancelled, and the ones on screen that + /// never got to ask — a row appearing is the only thing that asks, and an + /// appearance during a moment when this was not the account's history at + /// all asks for nothing. Bounded by what has actually been looked at, and + /// each row is still only ever fetched once per load. + private func resumeHydration(account: Account) { + for row in rows + where !row.isHydrated + && (hydrationPending.contains(row.videoID) || visibleRowIDs.contains(row.videoID)) { + hydrate(row, account: account) + } + } + /// Asks for one row's metadata, as the row appears. /// /// The request itself waits its turn: the row joins the queue, and the @@ -796,6 +844,7 @@ struct HistoryListView: View { else { return } hydrationAttempted.insert(row.videoID) + hydrationPending.insert(row.videoID) hydrationQueue.append(row.videoID) pumpHydration(account: account) } @@ -809,7 +858,8 @@ struct HistoryListView: View { let instance = appEnvironment.accountsManager.instance(for: account) else { return } - let generation = loadGeneration + let epoch = hydrationEpoch + let accountID = account.id while hydrationTasks.count < Self.hydrationConcurrency, !hydrationQueue.isEmpty { let videoID = hydrationQueue.removeFirst() @@ -817,8 +867,13 @@ struct HistoryListView: View { hydrationTasks[videoID] = Task { do { let video = try await appEnvironment.contentService.video(id: videoID, instance: instance) - guard generation == loadGeneration else { return } - apply(video, to: videoID) + // Whose history this describes is what decides whether it + // may be shown — not which load asked for it. A reload of + // the same account puts the same rows back, and metadata + // fetched for one of them is still that row's metadata. + if isShowing(accountID: accountID) { + apply(video, to: videoID) + } } catch { // The row keeps its placeholder. A video the account watched // is part of its history whether or not this instance can @@ -826,16 +881,22 @@ struct HistoryListView: View { } // A finished fetch frees its slot for the next row — but only - // for the load it belongs to. A later one has already emptied - // this queue and started its own, and giving back a slot there - // would drop a task that is still running. - guard generation == loadGeneration else { return } + // among the fetches it started with. A later set has already + // emptied this queue and started its own, and giving back a + // slot there would drop a task that is still running. + guard epoch == hydrationEpoch else { return } hydrationTasks[videoID] = nil pumpHydration(account: account) } } } + /// Whether the rows on screen are still that account's history. + private func isShowing(accountID: UUID) -> Bool { + guard case let .server(account) = displayMode else { return false } + return account.id == accountID + } + /// Drops everything the current load was still going to fetch, and stops /// what it had already started. private func cancelHydration() { @@ -845,9 +906,12 @@ struct HistoryListView: View { hydrationTasks.removeAll() hydrationQueue.removeAll() + hydrationEpoch += 1 } private func apply(_ video: Video, to videoID: String) { + hydrationPending.remove(videoID) + guard let index = rows.firstIndex(where: { $0.videoID == videoID }) else { return } rows[index].video = video diff --git a/Yattee/Views/Video/VideoInfoView.swift b/Yattee/Views/Video/VideoInfoView.swift index fde1a99e..8053abe7 100644 --- a/Yattee/Views/Video/VideoInfoView.swift +++ b/Yattee/Views/Video/VideoInfoView.swift @@ -883,7 +883,13 @@ struct VideoInfoView: View { // Prefer freshly loaded details for thumbnail (so expired proxy URLs get replaced) and avatar. // Keep `video` as the card's stable identity so title/channel don't flicker. let detailedVideo = loadedVideoDetails[video.id.videoID] - videoCard(for: video, thumbnailFrom: detailedVideo, authorFrom: detailedVideo, isLoadingMore: isLoadingMoreVideos && isCurrent, showTitle: true, isCurrent: isCurrent) + // Unless the list never had a title to keep: a row + // from an account's server history arrives as its + // own video ID, and holding on to that would leave + // the card named after a string of characters under + // the real channel and description. + let titleVideo = video.hasPlaceholderTitle ? (detailedVideo ?? video) : video + videoCard(for: titleVideo, thumbnailFrom: detailedVideo, authorFrom: detailedVideo, isLoadingMore: isLoadingMoreVideos && isCurrent, showTitle: true, isCurrent: isCurrent) .opacity(isCurrent ? 1.0 : peekOpacity) .containerRelativeFrame(.horizontal) .id(index) diff --git a/YatteeTests/AccountServiceTests.swift b/YatteeTests/AccountServiceTests.swift index 4237b73e..510afccc 100644 --- a/YatteeTests/AccountServiceTests.swift +++ b/YatteeTests/AccountServiceTests.swift @@ -277,7 +277,7 @@ private final class AccountServiceHarness { var credentialSwapOnNextRead: String? init(type: InstanceType = .invidious, isEnabled: Bool = true) { - instances = InstancesManager() + instances = makeTestInstancesManager() instance = Instance( type: type, url: URL(string: "https://accounts-\(UUID().uuidString).example.com")!, diff --git a/YatteeTests/AccountSyncTests.swift b/YatteeTests/AccountSyncTests.swift index e2520da4..b7d41e85 100644 --- a/YatteeTests/AccountSyncTests.swift +++ b/YatteeTests/AccountSyncTests.swift @@ -691,7 +691,7 @@ struct AccountSyncTests { @Test("A repair made mid-batch is never uploaded over the server's older context") func repairIsNotPublishedForAStaleServerContext() async { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = Instance(type: .invidious, url: URL(string: "https://inv-\(UUID().uuidString).example.com")!) instances.add(instance) defer { instances.remove(instance) } @@ -732,7 +732,7 @@ struct AccountSyncTests { @Test("An account discarded for its tombstone still repairs the context") func discardedAccountKeepsItsInstance() async { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = Instance(type: .invidious, url: URL(string: "https://inv-\(UUID().uuidString).example.com")!) instances.add(instance) defer { instances.remove(instance) } @@ -765,7 +765,7 @@ struct AccountSyncTests { @Test("A context fetched ahead of the records it depends on is still applied last") func stagedContextIsAppliedAfterTheAccountRecords() async { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = Instance(type: .invidious, url: URL(string: "https://inv-\(UUID().uuidString).example.com")!) instances.add(instance) defer { instances.remove(instance) } diff --git a/YatteeTests/AccountsManagerTests.swift b/YatteeTests/AccountsManagerTests.swift index 490f8db4..e5236a37 100644 --- a/YatteeTests/AccountsManagerTests.swift +++ b/YatteeTests/AccountsManagerTests.swift @@ -406,7 +406,7 @@ struct AccountsManagerTests { @Test("An unresolved identity never migrates a legacy login") func unresolvedIdentityDoesNotMigrate() { let defaults = makeDefaults() - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeLegacyInstance() instances.add(instance) defer { instances.remove(instance) } @@ -438,7 +438,7 @@ struct AccountsManagerTests { @Test("A scope that dips to unresolved and returns writes nothing in between") func scopeRoundTripLeavesTheUnresolvedNamespaceEmpty() async throws { let defaults = makeDefaults() - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeLegacyInstance() instances.add(instance) defer { instances.remove(instance) } @@ -570,7 +570,7 @@ struct DataContextValidatorTests { @Test("A switched-off instance makes its context unavailable, not removed") func disabledInstanceIsUnavailable() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() var instance = Instance(type: .invidious, url: URL(string: "https://inv.example.com")!) instances.add(instance) @@ -598,7 +598,7 @@ struct DataContextValidatorTests { @Test("Only a deletion the app can point at moves the context") func correctionRequiresATombstone() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() var instance = Instance(type: .invidious, url: URL(string: "https://inv-correct.example.com")!) instances.add(instance) defer { instances.remove(instance) } @@ -639,7 +639,7 @@ struct DataContextValidatorTests { @Test("A tombstoned selection is repaired without being published") func correctionFollowsTheTombstone() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = Instance(type: .invidious, url: URL(string: "https://inv-tombstone.example.com")!) instances.add(instance) defer { instances.remove(instance) } @@ -668,7 +668,7 @@ struct DataContextValidatorTests { @Test("Nothing is repaired while the namespace is reloading") func correctionIsFencedToTheLoadedNamespace() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = Instance(type: .invidious, url: URL(string: "https://inv-reload.example.com")!) instances.add(instance) defer { instances.remove(instance) } @@ -715,7 +715,7 @@ struct DataContextValidatorTests { @Test("Only accounts the app can actually resolve are offered") func availableContextsAreFiltered() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let enabled = Instance(type: .invidious, url: URL(string: "https://inv-a.example.com")!) let disabled = Instance(type: .invidious, url: URL(string: "https://inv-b.example.com")!, isEnabled: false) // No account concept on PeerTube, so a context there could never be @@ -756,7 +756,7 @@ struct DataContextRepairTests { @Test("Deleting the selected account falls back to a signed-in sibling") func deletionPrefersSignedInSibling() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -779,7 +779,7 @@ struct DataContextRepairTests { @Test("Without a signed-in sibling the repair lands on local") func deletionWithoutSiblingFallsBackToLocal() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -800,7 +800,7 @@ struct DataContextRepairTests { @Test("A repair made while a sibling was still being checked never comes back") func repairIsNotReconsideredWhenStatusesSettle() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -829,7 +829,7 @@ struct DataContextRepairTests { @Test("A signed-in account on another server is never the repair") func repairNeverCrossesInstances() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let first = makeInstance("inv-first") let second = makeInstance("inv-second") instances.add(first) @@ -852,7 +852,7 @@ struct DataContextRepairTests { @Test("A synced context naming a deleted account repairs to its sibling") func syncedTombstoneRepairPrefersSibling() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -878,7 +878,7 @@ struct DataContextRepairTests { @Test("A synced context for a deleted account with no sibling lands on local") func syncedTombstoneRepairFallsBackToLocal() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -900,7 +900,7 @@ struct DataContextRepairTests { @Test("Adopting a repaired synced context drops what the replaced choice owed") func syncedTombstoneRepairIsNeverPublished() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -929,7 +929,7 @@ struct DataContextRepairTests { @Test("A local choice that outranks a repair keeps its upload") func repairThatLosesLeavesTheChoiceDirty() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -956,7 +956,7 @@ struct DataContextRepairTests { @Test("A repair is never pushed back over the server's older context") func repairIsNeverPushedBackOverAnOlderContext() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -1002,7 +1002,7 @@ struct DataContextRepairTests { @Test("A repair read back from disk still yields rather than publishing") func reloadedRepairStillYieldsToTheServersContext() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -1035,7 +1035,7 @@ struct DataContextRepairTests { @Test("A repair yields to an explicit context it ties with") func repairYieldsToAnExplicitContextOnATie() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -1064,7 +1064,7 @@ struct DataContextRepairTests { @Test("A repair yields to an older context naming an account that is still there") func repairYieldsToAnOlderContextNamingALiveAccount() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -1096,7 +1096,7 @@ struct DataContextRepairTests { @Test("An older context naming the deleted account repairs again rather than landing on it") func repairYieldsToAnOlderContextNamingTheDeletedAccount() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -1130,7 +1130,7 @@ struct DataContextRepairTests { @Test("A repair recomputes statuses before it picks a sibling") func repairRefreshesStatusesBeforeChoosing() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } @@ -1160,7 +1160,7 @@ struct DataContextRepairTests { @Test("An account dominated by a tombstone still says where it lived") func discardedAccountKeepsItsInstanceForRepair() { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance("inv") instances.add(instance) defer { instances.remove(instance) } diff --git a/YatteeTests/AccountsScreenTests.swift b/YatteeTests/AccountsScreenTests.swift index c2870870..b7c89e0f 100644 --- a/YatteeTests/AccountsScreenTests.swift +++ b/YatteeTests/AccountsScreenTests.swift @@ -30,7 +30,7 @@ struct OrphanedAccountRemovalTests { @Test("An account outlives its source, and knows nothing about it") func orphanHasNothingToResolveThrough() { let scope = makeScope() - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance(.invidious) instances.add(instance) @@ -59,7 +59,7 @@ struct OrphanedAccountRemovalTests { let invidious = InvidiousCredentialsManager(identityScope: scope) let piped = PipedCredentialsManager(identityScope: scope) - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance(.invidious) instances.add(instance) @@ -96,7 +96,7 @@ struct OrphanedAccountRemovalTests { let invidious = InvidiousCredentialsManager(identityScope: scope) let piped = PipedCredentialsManager(identityScope: scope) - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance(.piped) instances.add(instance) @@ -126,7 +126,7 @@ struct OrphanedAccountRemovalTests { @Test("A deletion queued for the server survives the removal") func removalQueuesTheTombstone() { let scope = makeScope() - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance(.invidious) instances.add(instance) @@ -155,7 +155,7 @@ struct OrphanedAccountRemovalTests { @Test("The deletion is durable before the secret is destroyed") func removalRecordsTheTombstoneFirst() { let scope = makeScope() - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance(.invidious) instances.add(instance) @@ -186,7 +186,7 @@ struct OrphanedAccountRemovalTests { let scope = makeScope() let invidious = InvidiousCredentialsManager(identityScope: scope) - let instances = InstancesManager() + let instances = makeTestInstancesManager() let instance = makeInstance(.invidious) instances.add(instance) diff --git a/YatteeTests/CredentialProbeTests.swift b/YatteeTests/CredentialProbeTests.swift index a37683c2..c86064fe 100644 --- a/YatteeTests/CredentialProbeTests.swift +++ b/YatteeTests/CredentialProbeTests.swift @@ -47,7 +47,7 @@ private final class ProbeHarness { cooldown: TimeInterval = 0, unreachableRetryInterval: TimeInterval = 60 ) { - instances = InstancesManager() + instances = makeTestInstancesManager() instance = Instance( type: type, url: URL(string: "https://probe-\(UUID().uuidString).example.com")!, diff --git a/YatteeTests/InstancesManagerTests.swift b/YatteeTests/InstancesManagerTests.swift index a62c9430..673aee63 100644 --- a/YatteeTests/InstancesManagerTests.swift +++ b/YatteeTests/InstancesManagerTests.swift @@ -147,3 +147,15 @@ struct InstanceIdentityTests { #expect(instance.id == originalID) } } + +// MARK: - Test Support + +/// An instances manager with defaults of its own. +/// +/// `InstancesManager()` reads and writes `UserDefaults.standard`, which under a +/// test host is the app's own container: an instance a fixture adds becomes one +/// the app on that device is configured with, and it stays there after the run. +@MainActor +func makeTestInstancesManager() -> InstancesManager { + InstancesManager(defaults: UserDefaults(suiteName: "yattee.tests.instances.\(UUID().uuidString)")!) +} diff --git a/YatteeTests/SubscriptionDestinationTests.swift b/YatteeTests/SubscriptionDestinationTests.swift index b04a3356..500a5472 100644 --- a/YatteeTests/SubscriptionDestinationTests.swift +++ b/YatteeTests/SubscriptionDestinationTests.swift @@ -117,7 +117,7 @@ struct SubscriptionDestinationRoutingTests { } private func makeFixture() throws -> Fixture { - let instances = InstancesManager() + let instances = makeTestInstancesManager() let made = (0 ..< 2).map { index in Instance( type: .invidious, diff --git a/YatteeTests/WatchHistorySyncTests.swift b/YatteeTests/WatchHistorySyncTests.swift index f1e1e604..1b146c1f 100644 --- a/YatteeTests/WatchHistorySyncTests.swift +++ b/YatteeTests/WatchHistorySyncTests.swift @@ -35,7 +35,7 @@ private final class WatchHistorySyncHarness { var credentials: [UUID: String] = [:] init(type: InstanceType = .invidious) { - instances = InstancesManager() + instances = makeTestInstancesManager() instance = Instance( type: type, url: URL(string: "https://history-\(UUID().uuidString).example.com")!,