From f628dbc46e4aaf1920c52a7c845bb6b8f537ead6 Mon Sep 17 00:00:00 2001 From: Nick Porter <88012362+porter-stripe@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:22:20 -0600 Subject: [PATCH] Send analytics events on simulator but not during tests (#3501) ## Summary - Updates the `STPAnalyticsClient` to only send analytics if we are not in a test. Now includes sending when running from a simulator. - Intentionally does not update the V2 analytics client as that is used outside the scope of our team. - Left in place the `STPAnalyticsClientDelegate` so our playground can still inspect analytics logs easily. ## Motivation https://jira.corp.stripe.com/browse/MOBILESDK-1903 ## Testing Manual ## Changelog N/A --- .../Source/STPAnalyticsClient+BasicUI.swift | 3 --- .../STPAnalyticsClientPaymentSheetTest.swift | 3 ++- .../STPAnalyticsClientPaymentsTest.swift | 5 ++++- .../Source/Analytics/STPAnalyticsClient.swift | 17 +++++++++-------- .../Analytics/STPAnalyticsClientTest.swift | 4 ++-- .../Analytics/STPAnalyticsClient+Address.swift | 4 ---- .../STPAnalyticsClient+PaymentSheet.swift | 4 ---- 7 files changed, 17 insertions(+), 23 deletions(-) diff --git a/Stripe/StripeiOS/Source/STPAnalyticsClient+BasicUI.swift b/Stripe/StripeiOS/Source/STPAnalyticsClient+BasicUI.swift index f205d68f0f8..f4c2bd7e2e2 100644 --- a/Stripe/StripeiOS/Source/STPAnalyticsClient+BasicUI.swift +++ b/Stripe/StripeiOS/Source/STPAnalyticsClient+BasicUI.swift @@ -100,9 +100,6 @@ extension STPPaymentContext { } var params: [String: Any] = ["selected_lpm": paymentMethodType] - if STPAnalyticsClient.isSimulatorOrTest { - params["is_development"] = true - } if let error { params["error_message"] = error.makeSafeLoggingString() } diff --git a/Stripe/StripeiOSTests/STPAnalyticsClientPaymentSheetTest.swift b/Stripe/StripeiOSTests/STPAnalyticsClientPaymentSheetTest.swift index 0134fd2f991..f523ea747d0 100644 --- a/Stripe/StripeiOSTests/STPAnalyticsClientPaymentSheetTest.swift +++ b/Stripe/StripeiOSTests/STPAnalyticsClientPaymentSheetTest.swift @@ -187,7 +187,7 @@ class STPAnalyticsClientPaymentSheetTest: XCTestCase { let payload = client.payload(from: analytic, apiClient: apiClient) // verify - XCTAssertEqual(16, payload.count) + XCTAssertEqual(17, payload.count) XCTAssertNotNil(payload["device_type"] as? String) XCTAssertEqual("Wi-Fi", payload["network_type"] as? String) // In xctest, this is the version of Xcode @@ -208,6 +208,7 @@ class STPAnalyticsClientPaymentSheetTest: XCTestCase { XCTAssertEqual(STPAPIClient.STPSDKVersion, payload["bindings_version"] as? String) XCTAssertEqual("testVal", payload["testKey"] as? String) XCTAssertEqual("X", payload["install"] as? String) + XCTAssertTrue(payload["is_development"] as? Bool ?? false) let additionalInfo = try XCTUnwrap(payload["additional_info"] as? [String]) XCTAssertEqual(1, additionalInfo.count) diff --git a/Stripe/StripeiOSTests/STPAnalyticsClientPaymentsTest.swift b/Stripe/StripeiOSTests/STPAnalyticsClientPaymentsTest.swift index 68b34fa97e8..60b98fba4f2 100644 --- a/Stripe/StripeiOSTests/STPAnalyticsClientPaymentsTest.swift +++ b/Stripe/StripeiOSTests/STPAnalyticsClientPaymentsTest.swift @@ -44,7 +44,7 @@ class STPAnalyticsClientPaymentsTest: XCTestCase { let mockAnalytic = MockAnalytic() let payload = client.payload(from: mockAnalytic) - XCTAssertEqual(payload.count, 14) + XCTAssertEqual(payload.count, 15) // Verify event name is included XCTAssertEqual(payload["event"] as? String, mockAnalytic.event.rawValue) @@ -61,6 +61,9 @@ class STPAnalyticsClientPaymentsTest: XCTestCase { // Verify install method is Xcode XCTAssertEqual(payload["install"] as? String, "X") + + // Verify is_development + XCTAssertTrue(payload["is_development"] as? Bool ?? false) } // MARK: - Error tests diff --git a/StripeCore/StripeCore/Source/Analytics/STPAnalyticsClient.swift b/StripeCore/StripeCore/Source/Analytics/STPAnalyticsClient.swift index 49bf3152781..429fd126636 100644 --- a/StripeCore/StripeCore/Source/Analytics/STPAnalyticsClient.swift +++ b/StripeCore/StripeCore/Source/Analytics/STPAnalyticsClient.swift @@ -70,12 +70,12 @@ import UIKit #if targetEnvironment(simulator) return true #else - return NSClassFromString("XCTest") != nil + return isUnitOrUITest #endif } - @objc public class func shouldCollectAnalytics() -> Bool { - return !isSimulatorOrTest + static var isUnitOrUITest: Bool { + return NSClassFromString("XCTest") != nil || ProcessInfo.processInfo.environment["UITesting"] != nil } public func additionalInfo() -> [String] { @@ -115,11 +115,9 @@ import UIKit delegate?.analyticsClientDidLog(analyticsClient: self, payload: payload) #endif - guard type(of: self).shouldCollectAnalytics() else { - // Don't send the analytic, but add it to `_testLogHistory` if we're in a test. - if NSClassFromString("XCTest") != nil { - _testLogHistory.append(payload) - } + // If in testing, don't log analytic, instead append payload to log history + guard !STPAnalyticsClient.isUnitOrUITest else { + _testLogHistory.append(payload) return } @@ -150,6 +148,9 @@ extension STPAnalyticsClient { payload["network_type"] = NetworkDetector.getConnectionType() payload["install"] = InstallMethod.current.rawValue payload["publishable_key"] = apiClient.sanitizedPublishableKey ?? "unknown" + if STPAnalyticsClient.isSimulatorOrTest { + payload["is_development"] = true + } return payload } diff --git a/StripeCore/StripeCoreTests/Analytics/STPAnalyticsClientTest.swift b/StripeCore/StripeCoreTests/Analytics/STPAnalyticsClientTest.swift index 1fcad5cba9e..bd0b1a755f3 100644 --- a/StripeCore/StripeCoreTests/Analytics/STPAnalyticsClientTest.swift +++ b/StripeCore/StripeCoreTests/Analytics/STPAnalyticsClientTest.swift @@ -13,8 +13,8 @@ import XCTest class STPAnalyticsClientTest: XCTestCase { - func testShouldCollectAnalytics_alwaysFalseInTest() { - XCTAssertFalse(STPAnalyticsClient.shouldCollectAnalytics()) + func testIsUnitOrUITest_alwaysTrueInTest() { + XCTAssertTrue(STPAnalyticsClient.isUnitOrUITest) } func testShouldRedactLiveKeyFromLog() { diff --git a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/STPAnalyticsClient+Address.swift b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/STPAnalyticsClient+Address.swift index b5d6163f076..efb02a34b01 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/STPAnalyticsClient+Address.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/STPAnalyticsClient+Address.swift @@ -17,10 +17,6 @@ extension STPAnalyticsClient { apiClient: STPAPIClient ) { var additionalParams = [:] as [String: Any] - if Self.isSimulatorOrTest { - additionalParams["is_development"] = true - } - additionalParams["address_data_blob"] = addressAnalyticData?.analyticsPayload let analytic = AddressAnalytic(event: event, diff --git a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/STPAnalyticsClient+PaymentSheet.swift b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/STPAnalyticsClient+PaymentSheet.swift index 8bf90aa156f..ecc947a4f3c 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/STPAnalyticsClient+PaymentSheet.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/STPAnalyticsClient+PaymentSheet.swift @@ -304,10 +304,6 @@ extension STPAnalyticsClient { apiClient: STPAPIClient = .shared ) { var additionalParams = [:] as [String: Any] - if Self.isSimulatorOrTest { - additionalParams["is_development"] = true - } - additionalParams["duration"] = duration additionalParams["link_enabled"] = linkEnabled additionalParams["active_link_session"] = activeLinkSession