diff --git a/Strand/System/TestCentre.swift b/Strand/System/TestCentre.swift index 12f3e9dbb2..8259a37970 100644 --- a/Strand/System/TestCentre.swift +++ b/Strand/System/TestCentre.swift @@ -2,19 +2,17 @@ import Foundation import StrandAnalytics /// The Test Centre orchestration surface: per-domain activation, a SINGLE consolidated prefs namespace, -/// and a one-time read-through migration that gathers the scattered legacy keys behind one accessor -/// WITHOUT renaming them (spec section 10). `active(_:)` is the zero-cost gate engines check before +/// preserving legacy experimental settings in their original namespaces (spec section 10). +/// `active(_:)` is the zero-cost gate engines check before /// emitting a tagged line, so an emitter on the GATT/analytics queue pays one Bool read when a mode is /// off. The Kotlin twin is TestCentre.kt, backed by a single "noop_testcentre" SharedPreferences file. public enum TestCentre { - // SINGLE namespace for all NEW Test Centre flags. The migrated LEGACY keys keep their original - // names (see migrate()), so no user loses a setting. + // Single namespace for new Test Centre flags; legacy keys keep their original names. private static let activePrefix = "testcentre.active." // + domain.id -> Bool private static let startedPrefix = "testcentre.startedAt." // + domain.id -> Double (unix) private static let guidedTargetPrefix = "testcentre.target." // + domain.id -> Int (nights/days) private static let answersPrefix = "testcentre.answers." // + domain.id -> [String:String] (Data) - private static let migratedKey = "testcentre.migrated.v1" private static let master = TestDomain.master @@ -63,12 +61,6 @@ public enum TestCentre { return m } - @MainActor public static func setAnswers(_ m: [String: String], for d: TestDomain) { - if let data = try? JSONEncoder().encode(m) { - UserDefaults.standard.set(data, forKey: answersPrefix + d.id) - } - } - // MARK: - All-time drained-rows tally (#990) /// Key for the ALL-TIME drained (persisted) row counter. Sits in the testcentre.* namespace because @@ -93,19 +85,4 @@ public enum TestCentre { UserDefaults.standard.integer(forKey: cumulativeDrainedKey) } - /// One-time migration: fold the scattered @AppStorage / PuffinExperiment / ScheduledDebugExport keys - /// behind this surface WITHOUT renaming them (read-through). Existing keys are PRESERVED (spec section - /// 10): the experimental toggles keep their PuffinExperiment.*Key names, the scheduled export keeps its - /// debugExport.* names. This only seeds the NEW testcentre.* surface and never deletes a legacy key. - /// Idempotent, guarded by the migratedKey bool. - @MainActor public static func migrate() { - guard !UserDefaults.standard.bool(forKey: migratedKey) else { return } - // Phase 1 has no testcentre.active.* state to seed from the legacy toggles (those are advanced - // experimental flags, gathered by the IA but not domain activations), so the migration only - // stamps the guard. The legacy keys are read in place through their existing accessors: - // PuffinExperiment.defaultsKey / .deepDataKey / .broadcastHrKey / .keepRealtimeForDataKey / - // .experimentalSleepV2Key / .autoDetectWorkoutsKey, and the ScheduledDebugExport "debugExport.*" - // keys. Nothing is moved; the Test Centre screen reads them where they already live. - UserDefaults.standard.set(true, forKey: migratedKey) - } } diff --git a/StrandTests/TestCentreActivationTests.swift b/StrandTests/TestCentreActivationTests.swift index 2d67563186..72e2751f7a 100644 --- a/StrandTests/TestCentreActivationTests.swift +++ b/StrandTests/TestCentreActivationTests.swift @@ -5,23 +5,33 @@ import StrandAnalytics @MainActor final class TestCentreActivationTests: XCTestCase { - // A clean suite so the test never touches the real app defaults. - private func freshDefaults() -> UserDefaults { - let name = "TestCentreTests-\(UUID().uuidString)" - let d = UserDefaults(suiteName: name)! - d.removePersistentDomain(forName: name) - return d + private var savedDefaults: [String: Any] = [:] + private var touchedKeys: [String] { + TestDomain.allCases.flatMap { domain in + ["testcentre.active.\(domain.id)", "testcentre.startedAt.\(domain.id)", + "testcentre.answers.\(domain.id)"] + } + [PuffinExperiment.deepDataKey] } - override func setUp() { - super.setUp() - // Reset the namespace this test relies on (TestCentre reads UserDefaults.standard). - for d in TestDomain.allCases { - UserDefaults.standard.removeObject(forKey: "testcentre.active.\(d.id)") - UserDefaults.standard.removeObject(forKey: "testcentre.startedAt.\(d.id)") - UserDefaults.standard.removeObject(forKey: "testcentre.answers.\(d.id)") + override func setUp() async throws { + try await super.setUp() + savedDefaults = [:] + for key in touchedKeys { + savedDefaults[key] = UserDefaults.standard.object(forKey: key) + UserDefaults.standard.removeObject(forKey: key) } - UserDefaults.standard.removeObject(forKey: "testcentre.migrated.v1") + } + + override func tearDown() async throws { + for key in touchedKeys { + if let value = savedDefaults[key] { + UserDefaults.standard.set(value, forKey: key) + } else { + UserDefaults.standard.removeObject(forKey: key) + } + } + savedDefaults = [:] + try await super.tearDown() } func testActivateThenActiveThenDeactivate() { @@ -55,18 +65,27 @@ final class TestCentreActivationTests: XCTestCase { XCTAssertGreaterThanOrEqual(s!.timeIntervalSince1970, before.timeIntervalSince1970 - 1) } - func testAnswersRoundTrip() { + func testAnswersReadsPersistedQuestionnaire() throws { + let key = "testcentre.answers.battery" XCTAssertEqual(TestCentre.answers(.battery), [:]) - TestCentre.setAnswers(["whoopAppInstalled": "yes", "batterySaverApps": "none"], for: .battery) - XCTAssertEqual(TestCentre.answers(.battery), ["whoopAppInstalled": "yes", "batterySaverApps": "none"]) + let answers = ["whoopAppInstalled": "yes", "batterySaverApps": "none"] + UserDefaults.standard.set(try JSONEncoder().encode(answers), forKey: key) + XCTAssertEqual(TestCentre.answers(.battery), answers) + XCTAssertEqual(TestCentre.answers(.sleep), [:]) } - // Migration seeds nothing destructive and is idempotent (guarded by the v1 bool). - func testMigrationIsIdempotentAndPreservesLegacyKeys() { - UserDefaults.standard.set(true, forKey: PuffinExperiment.deepDataKey) // a legacy key set "before" - TestCentre.migrate() - TestCentre.migrate() // second call is a no-op - XCTAssertTrue(UserDefaults.standard.bool(forKey: "testcentre.migrated.v1")) - XCTAssertTrue(UserDefaults.standard.bool(forKey: PuffinExperiment.deepDataKey)) // NOT renamed/wiped + func testAnswersRejectsMalformedPersistedQuestionnaire() { + let key = "testcentre.answers.battery" + for raw in ["not JSON", "[]", "{\"answer\":null}"] { + UserDefaults.standard.set(Data(raw.utf8), forKey: key) + XCTAssertEqual(TestCentre.answers(.battery), [:], raw) + } + } + + func testActivationPreservesLegacyKeys() { + UserDefaults.standard.set(true, forKey: PuffinExperiment.deepDataKey) + TestCentre.activate(.battery) + TestCentre.deactivate(.battery) + XCTAssertTrue(UserDefaults.standard.bool(forKey: PuffinExperiment.deepDataKey)) } } diff --git a/android/app/src/main/java/com/noop/testcentre/TestCentre.kt b/android/app/src/main/java/com/noop/testcentre/TestCentre.kt index 5a3d294a63..9d007a1cf2 100644 --- a/android/app/src/main/java/com/noop/testcentre/TestCentre.kt +++ b/android/app/src/main/java/com/noop/testcentre/TestCentre.kt @@ -10,8 +10,8 @@ import org.json.JSONObject * Backed by a SINGLE SharedPreferences file "noop_testcentre" for all NEW Test Centre flags, * consolidating what used to be scattered across noop_prefs / noop_experiments / noop_debug_export and * ~10 non-reactive mutableStateOf mirrors in SettingsScreen. The legacy experiment + debug-export keys - * are PRESERVED in their own files (read through their existing accessors), so migration never loses a - * setting (spec section 10). [active] is the zero-cost gate engines check before emitting a tagged line. + * are PRESERVED in their own files (read through their existing accessors), so activation never changes a + * legacy setting (spec section 10). [active] is the zero-cost gate engines check before emitting a tagged line. * * The constructor takes a [SharedPreferences] directly so the surface is exercisable on the plain JVM * (no Robolectric, matching the rest of the suite, see DeviceRegistryTest / MoodStoreTest); production @@ -56,12 +56,6 @@ class TestCentre internal constructor(private val prefs: SharedPreferences) { }.getOrDefault(emptyMap()) } - fun setAnswers(d: TestDomain, m: Map) { - val o = JSONObject() - for ((k, v) in m) o.put(k, v) - prefs.edit().putString(ANSWERS_PREFIX + d.id, o.toString()).apply() - } - // All-time drained-rows tally (#990) - twin of the Swift TestCentre accessors. Sits in the // testcentre.* namespace because the Connection readout is its consumer, but it accrues // UNCONDITIONALLY (the Backfiller session summary is not test-mode gated), so it answers "has this @@ -80,24 +74,11 @@ class TestCentre internal constructor(private val prefs: SharedPreferences) { * count on the Connection readout (#990). */ fun cumulativeDrainedRows(): Long = prefs.getLong(CUMULATIVE_DRAINED_KEY, 0L) - /** - * One-time migration. Idempotent, guarded by the v1 bool. Phase 1 has no domain-activation state to - * seed from the legacy toggles (those are advanced experimental flags, gathered by the IA but not - * domain activations), so this only stamps the guard. The legacy PuffinExperiment (noop_experiments) - * and DebugExportSettings (noop_debug_export) keys are read in place through their existing - * accessors; nothing is moved or renamed. - */ - fun migrate() { - if (prefs.getBoolean(MIGRATED_KEY, false)) return - prefs.edit().putBoolean(MIGRATED_KEY, true).apply() - } - companion object { private const val PREFS = "noop_testcentre" private const val ACTIVE_PREFIX = "testcentre.active." private const val STARTED_PREFIX = "testcentre.startedAt." private const val ANSWERS_PREFIX = "testcentre.answers." - private const val MIGRATED_KEY = "testcentre.migrated.v1" private const val CUMULATIVE_DRAINED_KEY = "testcentre.cumulativeDrainedRows" fun from(context: Context): TestCentre = diff --git a/android/app/src/test/java/com/noop/testcentre/TestCentreTest.kt b/android/app/src/test/java/com/noop/testcentre/TestCentreTest.kt index bf1e68b748..8017b5f9e7 100644 --- a/android/app/src/test/java/com/noop/testcentre/TestCentreTest.kt +++ b/android/app/src/test/java/com/noop/testcentre/TestCentreTest.kt @@ -9,7 +9,7 @@ import org.junit.Test /** * Mirror of the Swift TestCentreActivationTests: same activation semantics, master-implies-all, - * universal-rides-any, answers round-trip, idempotent migration that preserves legacy keys. + * universal-rides-any, persisted-answer reads, and activation that preserves legacy keys. * * The project ships NO Robolectric (junit + kotlinx-coroutines-test only, see app/build.gradle.kts and * DeviceRegistryTest / MoodStoreTest), so rather than stand up a real Context we run the REAL [TestCentre] @@ -90,21 +90,31 @@ class TestCentreTest { assertTrue((tc.startedAt(TestDomain.SLEEP) ?: 0L) > 0L) } - @Test fun answersRoundTrip() { - val tc = newCentre() + @Test fun answersReadsPersistedQuestionnaire() { + val prefs = FakeSharedPreferences() + val tc = TestCentre(prefs) assertEquals(emptyMap(), tc.answers(TestDomain.BATTERY)) - tc.setAnswers(TestDomain.BATTERY, mapOf("whoopAppInstalled" to "yes", "batterySaverApps" to "none")) + prefs.edit().putString("testcentre.answers.battery", + "{\"whoopAppInstalled\":\"yes\",\"batterySaverApps\":\"none\"}").apply() assertEquals(mapOf("whoopAppInstalled" to "yes", "batterySaverApps" to "none"), tc.answers(TestDomain.BATTERY)) + assertEquals(emptyMap(), tc.answers(TestDomain.SLEEP)) } - @Test fun migrationIsIdempotentAndPreservesLegacyKeys() { - // A legacy experiments key set "before" lives in its OWN prefs store (noop_experiments). The Test - // Centre prefs file is separate; migrate() must never reach into or wipe the legacy store. - val legacy = FakeSharedPreferences() - legacy.edit().putBoolean("noopWhoop5DeepData", true).apply() - val tc = TestCentre(FakeSharedPreferences()) - tc.migrate() - tc.migrate() - assertTrue(legacy.getBoolean("noopWhoop5DeepData", false)) // NOT renamed/wiped by migration + @Test fun answersRejectsMalformedPersistedQuestionnaire() { + val prefs = FakeSharedPreferences() + val tc = TestCentre(prefs) + for (raw in listOf("not JSON", "[]")) { + prefs.edit().putString("testcentre.answers.battery", raw).apply() + assertEquals(raw, emptyMap(), tc.answers(TestDomain.BATTERY)) + } + } + + @Test fun activationPreservesUnrelatedKeys() { + val prefs = FakeSharedPreferences() + prefs.edit().putBoolean("unrelated", true).apply() + val tc = TestCentre(prefs) + tc.activate(TestDomain.BATTERY) + tc.deactivate(TestDomain.BATTERY) + assertTrue(prefs.getBoolean("unrelated", false)) } }