Improve and simplify remote flag tests

Author: joshheald

Modules/Sources/Yosemite/Tools/POS/POSLocalCatalogEligibilityService.swift

@@ -37,8 +37,9 @@ public actor POSLocalCatalogEligibilityService: POSLocalCatalogEligibilityServic
         self.isLocalCatalogFeatureFlagEnabled = isLocalCatalogFeatureFlagEnabled
         self.remoteFeatureFlagProvider = remoteFeatureFlagProvider
         self.catalogSizeLimit = catalogSizeLimit ?? Constants.defaultCatalogSizeLimit
+        // Eagerly start fetching the remote flag in the background
         Task {
-            _ = await isRemoteCatalogFeatureFlagEnabled()
+            await self.fetchRemoteFlag()
         }
     }
 
@@ -53,22 +54,15 @@ public actor POSLocalCatalogEligibilityService: POSLocalCatalogEligibilityServic
     }
 
     /// Fetch and cache the remote feature flag value
-    /// Returns cached value if available, otherwise returns true without waiting for network
+    /// Returns cached value if available, otherwise returns true (assumes eligible)
     private func isRemoteCatalogFeatureFlagEnabled() async -> Bool {
-        if let cached = cachedRemoteFeatureFlag {
-            return cached
-        }
-        // No cached value yet - assume eligible (true) without waiting for network
-        // Kick off the fetch in the background to cache for next time
-        Task { [weak self] in
-            let value = await self?.remoteFeatureFlagProvider() ?? true
-            await self?.cacheRemoteFeatureFlag(value)
-        }
-        return true
+        // Return cached value if we have one
+        return cachedRemoteFeatureFlag ?? true
     }
 
-    /// Cache the remote feature flag value (actor-isolated)
-    private func cacheRemoteFeatureFlag(_ value: Bool) {
+    /// Fetch the remote feature flag value and cache it (actor-isolated)
+    private func fetchRemoteFlag() async {
+        let value = await remoteFeatureFlagProvider()
         cachedRemoteFeatureFlag = value
     }
 

Modules/Tests/YosemiteTests/Tools/POS/POSLocalCatalogEligibilityServiceTests.swift

@@ -242,7 +242,7 @@ struct POSLocalCatalogEligibilityServiceTests {
         #expect(sizeChecker.checkCatalogSizeCallCount == 0)
     }
 
-    @Test("Remote feature flag disabled returns ineligible")
+    @Test("Remote feature flag disabled returns ineligible after refresh")
     func testRemoteFeatureFlagDisabledReturnsIneligible() async throws {
         let sizeChecker = MockPOSCatalogSizeChecker(
             sizeToReturn: .success(POSCatalogSize(productCount: 500, variationCount: 400))
@@ -257,10 +257,14 @@ struct POSLocalCatalogEligibilityServiceTests {
         )
         try await service.updatePOSEligibility(isEligible: true, for: siteID)
 
-        let state = try await service.catalogEligibility(for: siteID)
+        // First refresh might check catalog (using default true before fetch completes)
+        _ = try? await service.refreshEligibilityState(for: siteID)
+
+        // Second refresh should use the fetched remote flag value (false)
+        let state = try await service.refreshEligibilityState(for: siteID)
 
         guard case .ineligible(let reason) = state else {
-            Issue.record("Expected ineligible state")
+            Issue.record("Expected ineligible state after remote flag fetch")
             return
         }
 
@@ -269,8 +273,9 @@ struct POSLocalCatalogEligibilityServiceTests {
             return
         }
 
-        // Should not have checked catalog size
-        #expect(sizeChecker.checkCatalogSizeCallCount == 0)
+        // Second refresh should not have checked catalog size (short-circuited by flag)
+        // First refresh might have checked it (count could be 0 or 1)
+        #expect(sizeChecker.checkCatalogSizeCallCount <= 1)
     }
 
     @Test("Both feature flags required for eligibility")