Skip to content

Commit

Permalink
Add FXIOS-10881 [Sent from Firefox] Add ability to override a user's …
Browse files Browse the repository at this point in the history 
…preference for a nimbus feature in unit tests (#23790)

* Slightly alter our implementation of the preexisting Nimbus test override flag to ensure it only overrides the user's preference setting in tests. Make behaviour more explicit and add documentation. Update handling for tests using the old code.

* Add an additional ShareManager tests which check if the ShareManager respects the user's Sent from Firefox preference when checking whether WhatsApp share treatment should or should not be appended.
  • Loading branch information
@ih-codes
ih-codes authored Dec 18, 2024
1 parent 243602c commit 87f330c
Show file tree
Hide file tree
Showing 8 changed files with 154 additions and 16 deletions.
4 changes: 2 additions & 2 deletions BrowserKit/Sources/Shared/Prefs.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ public struct PrefsKeys {
// Representing whether or not the last user session was private
public static let LastSessionWasPrivate = "wasLastSessionPrivate"

// Only used to force nimbus features to true with tests
public static let NimbusFeatureTestsOverride = "NimbusFeatureTestsOverride"
// Only used in unit tests to override the user's setting for nimbus features
public static let NimbusUserEnabledFeatureTestsOverride = "NimbusUserEnabledFeatureTestsOverride"

// Only used to force faster transition of tabs to the inactive state (10 seconds)
public static let FasterInactiveTabsOverride = "FasterInactiveTabsOverride"
Expand Down
4 changes: 4 additions & 0 deletions firefox-ios/Client.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1868,6 +1868,7 @@
EDD3348E2D109458004516D0 /* ShareTelemetryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDD3348D2D109458004516D0 /* ShareTelemetryTests.swift */; };
EDD334902D109A5B004516D0 /* ShareTelemetryActivityItemProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDD3348F2D109A5B004516D0 /* ShareTelemetryActivityItemProviderTests.swift */; };
EDD334932D109AE6004516D0 /* MockShareTelemetry.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDD334922D109AE6004516D0 /* MockShareTelemetry.swift */; };
EDD334952D10BEEB004516D0 /* UserDefaults+valueExists.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDD334942D10BEEB004516D0 /* UserDefaults+valueExists.swift */; };
EDDF340A2CDD159F008BB6A4 /* SearchEngineModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDDF34092CDD159F008BB6A4 /* SearchEngineModel.swift */; };
EDDF340B2CDD1B7C008BB6A4 /* SearchEngineModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDDF34092CDD159F008BB6A4 /* SearchEngineModel.swift */; };
EDF567A02C8B51DC00FDB09D /* SiteImageView in Frameworks */ = {isa = PBXBuildFile; productRef = EDF5679F2C8B51DC00FDB09D /* SiteImageView */; };
Expand Down Expand Up @@ -9413,6 +9414,7 @@
EDD3348D2D109458004516D0 /* ShareTelemetryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareTelemetryTests.swift; sourceTree = "<group>"; };
EDD3348F2D109A5B004516D0 /* ShareTelemetryActivityItemProviderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareTelemetryActivityItemProviderTests.swift; sourceTree = "<group>"; };
EDD334922D109AE6004516D0 /* MockShareTelemetry.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockShareTelemetry.swift; sourceTree = "<group>"; };
EDD334942D10BEEB004516D0 /* UserDefaults+valueExists.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UserDefaults+valueExists.swift"; sourceTree = "<group>"; };
EDDF34092CDD159F008BB6A4 /* SearchEngineModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchEngineModel.swift; sourceTree = "<group>"; };
EDEE4CC7BD65892525632A4E /* ko */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ko; path = ko.lproj/Today.strings; sourceTree = "<group>"; };
EE294A97B40C4FDBD67AE536 /* hr */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = hr; path = hr.lproj/ErrorPages.strings; sourceTree = "<group>"; };
Expand Down Expand Up @@ -10291,6 +10293,7 @@
E1442FC5294782D7003680B0 /* UIAlertController+Extension.swift */,
8A395551299AF83300B2AFBB /* UIControl+Extension.swift */,
E18EA56E28AD3279003F97FC /* UIDevice+Extension.swift */,
EDD334942D10BEEB004516D0 /* UserDefaults+valueExists.swift */,
E12BD0AF28AC3A7E0029AAF0 /* UIEdgeInsets+Extension.swift */,
C80E1A0F2A0943640025B9E1 /* UIFont+Extension.swift */,
E12BD0AD28AC38480029AAF0 /* UIImage+Extension.swift */,
Expand Down Expand Up @@ -16274,6 +16277,7 @@
DF5D47522A9381C700D6AE74 /* FakespotSettingsCardView.swift in Sources */,
C88E7A592A0553440072E638 /* OnboardingCardInfoModel.swift in Sources */,
B2DFB7E32B619E2B0004CEA5 /* AddressCellView.swift in Sources */,
EDD334952D10BEEB004516D0 /* UserDefaults+valueExists.swift in Sources */,
AB6FEA202AEA5CA200E7B2F2 /* FakespotAdView.swift in Sources */,
C87A121A28C2451A0097ED51 /* WallpaperMigrationUtility.swift in Sources */,
AB9CBC032C53B64C00102610 /* TrackingProtectionMiddleware.swift in Sources */,
Expand Down
16 changes: 16 additions & 0 deletions firefox-ios/Client/Extensions/UserDefaults+valueExists.swift
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import Foundation

extension UserDefaults {
/// Returns `true` if this value exists, and `false` if the value is `nil`.
///
/// @discussion:
/// Calling `.bool(forKey:)` for a value that does NOT exist always returns false. However, sometimes we want to check
/// that the value has actually been set first.
func valueExists(forKey key: String) -> Bool {
return object(forKey: key) != nil
}
}
15 changes: 9 additions & 6 deletions firefox-ios/Client/FeatureFlags/NimbusFlaggableFeature.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,6 @@ struct NimbusFlaggableFeature: HasNimbusSearchBar {

// MARK: - Public methods
public func isNimbusEnabled(using nimbusLayer: NimbusFeatureFlagLayer) -> Bool {
// Provide a way to override nimbus feature enabled for tests
if AppConstants.isRunningUnitTest, UserDefaults.standard.bool(forKey: PrefsKeys.NimbusFeatureTestsOverride) {
return true
}

return nimbusLayer.checkNimbusConfigFor(featureID)
}

Expand All @@ -166,7 +161,15 @@ struct NimbusFlaggableFeature: HasNimbusSearchBar {
public func isUserEnabled(using nimbusLayer: NimbusFeatureFlagLayer) -> Bool {
guard let optionsKey = featureKey,
let option = profile.prefs.boolForKey(optionsKey)
else { return isNimbusEnabled(using: nimbusLayer) }
else {
// In unit tests only, we provide a way to return an override value to simulate a user's preference for a feature
if AppConstants.isRunningUnitTest,
UserDefaults.standard.valueExists(forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride) {
return UserDefaults.standard.bool(forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)
}

return isNimbusEnabled(using: nimbusLayer)
}

return option
}
Expand Down
4 changes: 2 additions & 2 deletions ...efox-ios-tests/Tests/ClientTests/Coordinators/LaunchView/LaunchScreenViewModelTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ final class LaunchScreenViewModelTests: XCTestCase {
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: profile)
messageManager = MockGleanPlumbMessageManagerProtocol()

UserDefaults.standard.set(true, forKey: PrefsKeys.NimbusFeatureTestsOverride)
UserDefaults.standard.set(true, forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)
}

override func tearDown() {
Expand All @@ -31,7 +31,7 @@ final class LaunchScreenViewModelTests: XCTestCase {
messageManager = nil
delegate = nil

UserDefaults.standard.set(false, forKey: PrefsKeys.NimbusFeatureTestsOverride)
UserDefaults.standard.removeObject(forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)
}

func testLaunchDoesntCallLoadedIfNotStarted() {
Expand Down
5 changes: 0 additions & 5 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/FeatureFlagManagerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ class FeatureFlagManagerTests: XCTestCase, FeatureFlaggable {
let mockProfile = MockProfile(databasePrefix: "FeatureFlagsManagerTests_")
mockProfile.prefs.clearAll()
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: mockProfile)
UserDefaults.standard.set(false, forKey: PrefsKeys.NimbusFeatureTestsOverride)
}

override func tearDown() {
UserDefaults.standard.removeObject(forKey: PrefsKeys.NimbusFeatureTestsOverride)
}

// MARK: - Tests
Expand Down
3 changes: 2 additions & 1 deletion firefox-ios/firefox-ios-tests/Tests/ClientTests/OnboardingTests/UpdateViewModelTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class UpdateViewModelTests: XCTestCase {
override func tearDown() {
super.tearDown()
profile = nil
UserDefaults.standard.removeObject(forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)
}

// MARK: Enable cards
Expand Down Expand Up @@ -104,7 +105,7 @@ class UpdateViewModelTests: XCTestCase {
func testShouldNotShowCoverSheetForSameVersion() {
let subject = createSubject()
let currentTestAppVersion = "22.0"
UserDefaults.standard.set(true, forKey: PrefsKeys.NimbusFeatureTestsOverride)
UserDefaults.standard.set(true, forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)

// Setting clean install to false
profile.prefs.setString(currentTestAppVersion, forKey: PrefsKeys.AppVersion.Latest)
Expand Down
119 changes: 119 additions & 0 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/Sharing/ShareManagerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import UniformTypeIdentifiers
import XCTest
import Shared

@testable import Client

final class ShareManagerTests: XCTestCase {
Expand All @@ -23,6 +25,7 @@ final class ShareManagerTests: XCTestCase {

override func tearDown() {
testTab = nil
UserDefaults.standard.removeObject(forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)
super.tearDown()
}

Expand Down Expand Up @@ -510,6 +513,122 @@ final class ShareManagerTests: XCTestCase {
XCTAssertTrue(itemForShareActivity is NSNull)
}

/// This test ensures that the `ShareManager` does not enforce Sent from Firefox treatment for users enrolled in the
/// experiment who have explicitly opted out (for example, using the "Include Firefox Download Link on WhatsApp Shares"
/// toggle on the general settings screen).
func testGetActivityItems_forTab_withSentFromFirefoxEnabled_respectsUserPreferencesToOptOut() throws {
// Setup Nimbus to emulate a user enrolled in Sent from Firefox with the Treatment A branch
setupNimbusSentFromFirefoxTesting(isEnabled: true, isTreatmentA: true)

// Simulate the user having disabled the Sent from Firefox toggle in the Settings
UserDefaults.standard.set(false, forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)

let whatsAppActivityIdentifier = "net.whatsapp.WhatsApp.ShareExtension"
let whatsAppActivity = UIActivity.ActivityType(rawValue: whatsAppActivityIdentifier)

let activityItems = ShareManager.getActivityItems(
forShareType: .tab(url: testWebURL, tab: testTab),
withExplicitShareMessage: nil
)

// Check we get all types of share items for tabs below:
let urlActivityItemProvider = try XCTUnwrap(activityItems[safe: 0] as? URLActivityItemProvider)
let urlDataIdentifier = urlActivityItemProvider.activityViewController(
createStubActivityViewController(),
dataTypeIdentifierForActivityType: whatsAppActivity
)
let itemForActivity = urlActivityItemProvider.activityViewController(
createStubActivityViewController(),
itemForActivityType: whatsAppActivity
)

// The rest of the content should be unchanged from other tests:
_ = try XCTUnwrap(activityItems[safe: 1] as? TabPrintPageRenderer)

_ = try XCTUnwrap(activityItems[safe: 2] as? TabWebView)

let titleActivityItemProvider = try XCTUnwrap(activityItems[safe: 3] as? TitleActivityItemProvider)
let itemForTitleActivity = titleActivityItemProvider.activityViewController(
createStubActivityViewController(),
itemForActivityType: whatsAppActivity
)

let telemetryActivityItemProvider = try XCTUnwrap(activityItems[safe: 4] as? ShareTelemetryActivityItemProvider)
let itemForShareActivity = telemetryActivityItemProvider.activityViewController(
createStubActivityViewController(),
itemForActivityType: whatsAppActivity
)

XCTAssertEqual(activityItems.count, 5)
XCTAssertEqual(urlDataIdentifier, UTType.url.identifier)
XCTAssertEqual(itemForActivity as? URL, testWebURL)
XCTAssertEqual(
itemForTitleActivity as? String,
testWebpageDisplayTitle,
"When no explicit share message is set, we expect to see the webpage's title."
)
XCTAssertTrue(itemForShareActivity is NSNull)
}

/// This test ensures that the `ShareManager` applies Sent from Firefox treatment for users enrolled in the experiment
/// who have explicitly opted in (for example, using the "Include Firefox Download Link on WhatsApp Shares" toggle on the
/// general settings screen).
func testGetActivityItems_forTab_withSentFromFirefoxEnabled_respectsUserPreferencesToOptIn() throws {
let expectedShareContentA = "https://mozilla.org Sent from Firefox 🦊 Try the mobile browser: https://mzl.la/4fOWPpd"

// Setup Nimbus to emulate a user enrolled in Sent from Firefox with the Treatment A branch
setupNimbusSentFromFirefoxTesting(isEnabled: true, isTreatmentA: true)

// Simulate the user having enabled the Sent from Firefox toggle in the Settings
UserDefaults.standard.set(true, forKey: PrefsKeys.NimbusUserEnabledFeatureTestsOverride)

let whatsAppActivityIdentifier = "net.whatsapp.WhatsApp.ShareExtension"
let whatsAppActivity = UIActivity.ActivityType(rawValue: whatsAppActivityIdentifier)

let activityItems = ShareManager.getActivityItems(
forShareType: .tab(url: testWebURL, tab: testTab),
withExplicitShareMessage: nil
)

// Check we get all types of share items for tabs below:
let urlActivityItemProvider = try XCTUnwrap(activityItems[safe: 0] as? URLActivityItemProvider)
let urlDataIdentifier = urlActivityItemProvider.activityViewController(
createStubActivityViewController(),
dataTypeIdentifierForActivityType: whatsAppActivity
)
let itemForActivity = urlActivityItemProvider.activityViewController(
createStubActivityViewController(),
itemForActivityType: whatsAppActivity
)

// The rest of the content should be unchanged from other tests:
_ = try XCTUnwrap(activityItems[safe: 1] as? TabPrintPageRenderer)

_ = try XCTUnwrap(activityItems[safe: 2] as? TabWebView)

let titleActivityItemProvider = try XCTUnwrap(activityItems[safe: 3] as? TitleActivityItemProvider)
let itemForTitleActivity = titleActivityItemProvider.activityViewController(
createStubActivityViewController(),
itemForActivityType: whatsAppActivity
)

let telemetryActivityItemProvider = try XCTUnwrap(activityItems[safe: 4] as? ShareTelemetryActivityItemProvider)
let itemForShareActivity = telemetryActivityItemProvider.activityViewController(
createStubActivityViewController(),
itemForActivityType: whatsAppActivity
)

XCTAssertEqual(activityItems.count, 5)
XCTAssertEqual(urlDataIdentifier, UTType.plainText.identifier)
XCTAssertEqual(itemForActivity as? String, expectedShareContentA)
XCTAssertEqual(
itemForTitleActivity as? String,
testWebpageDisplayTitle,
"When no explicit share message is set, we expect to see the webpage's title."
)
XCTAssertTrue(itemForShareActivity is NSNull)
}

// MARK: - Helpers

private func createStubActivityViewController() -> UIActivityViewController {
Expand Down

0 comments on commit 87f330c

Please sign in to comment.