From dc42a9b89238eff0a0ef7815196e8af1c1a7e92d Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 24 Jan 2024 10:40:09 -0800 Subject: [PATCH 1/3] Move SetConsent test to the end, and ensure that GetAnalyticsInstanceId isn't tested after it's called on Android. --- .../integration_test/src/integration_test.cc | 132 +++++++++++------- 1 file changed, 80 insertions(+), 52 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index e950415599..5f7942dc2a 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -100,6 +100,21 @@ TEST_F(FirebaseAnalyticsTest, TestSetSessionTimeoutDuraction) { } TEST_F(FirebaseAnalyticsTest, TestGetAnalyticsInstanceID) { + // On Android, if SetConsent was tested, this test will fail, since the app + // needs to be restarted after consent is denied or it won't generate a new + // sessionID. To not break the tests, skip this test in that case. +#if defined(__ANDROID__) + // Log the Google Play services version for debugging in case this test fails. + LogInfo("Google Play services version: %d", GetGooglePlayServicesVersion()); + if (did_test_setconsent_) { + LogInfo( + "Skipping %s after TestSetConsent, as the test may fail until the app is restarted.", + ::testing::UnitTest::GetInstance()->current_test_info()->name()); + GTEST_SKIP(); + return; + } +#endif + FLAKY_TEST_SECTION_BEGIN(); firebase::Future future = @@ -124,11 +139,10 @@ TEST_F(FirebaseAnalyticsTest, TestGetSessionID) { #if defined(__ANDROID__) // Log the Google Play services version for debugging in case this test fails. LogInfo("Google Play services version: %d", GetGooglePlayServicesVersion()); - if (did_test_setconsent_) { LogInfo( - "Skipping TestGetSessionID after TestSetConsent, as GetSessionId() " - "will fail until the app is restarted."); + "Skipping %s after TestSetConsent, as the test may fail until the app is restarted.", + ::testing::UnitTest::GetInstance()->current_test_info()->name()); GTEST_SKIP(); return; } @@ -158,39 +172,38 @@ TEST_F(FirebaseAnalyticsTest, TestGetSessionID) { } } -TEST_F(FirebaseAnalyticsTest, TestSetConsent) { - // Can't confirm that these do anything but just run them all to ensure the - // app doesn't crash. - std::map - consent_settings_allow = { - {firebase::analytics::kConsentTypeAnalyticsStorage, - firebase::analytics::kConsentStatusGranted}, - {firebase::analytics::kConsentTypeAdStorage, - firebase::analytics::kConsentStatusGranted}, - {firebase::analytics::kConsentTypeAdUserData, - firebase::analytics::kConsentStatusGranted}, - {firebase::analytics::kConsentTypeAdPersonalization, - firebase::analytics::kConsentStatusGranted}}; - std::map - consent_settings_deny = { - {firebase::analytics::kConsentTypeAnalyticsStorage, - firebase::analytics::kConsentStatusDenied}, - {firebase::analytics::kConsentTypeAdStorage, - firebase::analytics::kConsentStatusDenied}, - {firebase::analytics::kConsentTypeAdUserData, - firebase::analytics::kConsentStatusDenied}, - {firebase::analytics::kConsentTypeAdPersonalization, - firebase::analytics::kConsentStatusDenied}}; - std::map - consent_settings_empty; - firebase::analytics::SetConsent(consent_settings_empty); - ProcessEvents(1000); - firebase::analytics::SetConsent(consent_settings_deny); - ProcessEvents(1000); - firebase::analytics::SetConsent(consent_settings_allow); - ProcessEvents(1000); +TEST_F(FirebaseAnalyticsTest, TestResettingGivesNewInstanceId) { + // On Android, if SetConsent was tested, this test will fail, since the app + // needs to be restarted after consent is denied or it won't generate a new + // sessionID. To not break the tests, skip this test in that case. +#if defined(__ANDROID__) + // Log the Google Play services version for debugging in case this test fails. + LogInfo("Google Play services version: %d", GetGooglePlayServicesVersion()); + if (did_test_setconsent_) { + LogInfo( + "Skipping %s after TestSetConsent, as the test may fail until the app is restarted.", + ::testing::UnitTest::GetInstance()->current_test_info()->name()); + GTEST_SKIP(); + return; + } +#endif + FLAKY_TEST_SECTION_BEGIN(); - did_test_setconsent_ = true; + firebase::Future future = + firebase::analytics::GetAnalyticsInstanceId(); + WaitForCompletion(future, "GetAnalyticsInstanceId"); + EXPECT_FALSE(future.result()->empty()); + std::string instance_id = *future.result(); + + firebase::analytics::ResetAnalyticsData(); + + future = firebase::analytics::GetAnalyticsInstanceId(); + WaitForCompletion(future, "GetAnalyticsInstanceId after ResetAnalyticsData"); + std::string new_instance_id = *future.result(); + EXPECT_FALSE(future.result()->empty()); + EXPECT_NE(instance_id, new_instance_id); + + FLAKY_TEST_SECTION_END(); } TEST_F(FirebaseAnalyticsTest, TestSetProperties) { @@ -235,24 +248,39 @@ TEST_F(FirebaseAnalyticsTest, TestLogEventWithMultipleParameters) { sizeof(kLevelUpParameters) / sizeof(kLevelUpParameters[0])); } -TEST_F(FirebaseAnalyticsTest, TestResettingGivesNewInstanceId) { - FLAKY_TEST_SECTION_BEGIN(); - - firebase::Future future = - firebase::analytics::GetAnalyticsInstanceId(); - WaitForCompletion(future, "GetAnalyticsInstanceId"); - EXPECT_FALSE(future.result()->empty()); - std::string instance_id = *future.result(); - - firebase::analytics::ResetAnalyticsData(); - - future = firebase::analytics::GetAnalyticsInstanceId(); - WaitForCompletion(future, "GetAnalyticsInstanceId after ResetAnalyticsData"); - std::string new_instance_id = *future.result(); - EXPECT_FALSE(future.result()->empty()); - EXPECT_NE(instance_id, new_instance_id); +TEST_F(FirebaseAnalyticsTest, TestSetConsent) { + // Can't confirm that these do anything but just run them all to ensure the + // app doesn't crash. + std::map + consent_settings_allow = { + {firebase::analytics::kConsentTypeAnalyticsStorage, + firebase::analytics::kConsentStatusGranted}, + {firebase::analytics::kConsentTypeAdStorage, + firebase::analytics::kConsentStatusGranted}, + {firebase::analytics::kConsentTypeAdUserData, + firebase::analytics::kConsentStatusGranted}, + {firebase::analytics::kConsentTypeAdPersonalization, + firebase::analytics::kConsentStatusGranted}}; + std::map + consent_settings_deny = { + {firebase::analytics::kConsentTypeAnalyticsStorage, + firebase::analytics::kConsentStatusDenied}, + {firebase::analytics::kConsentTypeAdStorage, + firebase::analytics::kConsentStatusDenied}, + {firebase::analytics::kConsentTypeAdUserData, + firebase::analytics::kConsentStatusDenied}, + {firebase::analytics::kConsentTypeAdPersonalization, + firebase::analytics::kConsentStatusDenied}}; + std::map + consent_settings_empty; + firebase::analytics::SetConsent(consent_settings_empty); + ProcessEvents(1000); + firebase::analytics::SetConsent(consent_settings_deny); + ProcessEvents(1000); + firebase::analytics::SetConsent(consent_settings_allow); + ProcessEvents(1000); - FLAKY_TEST_SECTION_END(); + did_test_setconsent_ = true; } } // namespace firebase_testapp_automated From 591fedbdfb2b00d8093f9cb9a9669c01e9383443 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 25 Jan 2024 15:47:13 -0800 Subject: [PATCH 2/3] Add explanatory comment. --- analytics/integration_test/src/integration_test.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index 5f7942dc2a..6d82a25a92 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -249,6 +249,15 @@ TEST_F(FirebaseAnalyticsTest, TestLogEventWithMultipleParameters) { } TEST_F(FirebaseAnalyticsTest, TestSetConsent) { + // On Android, this test must be performed at the end, after all the tests for + // session ID and instance ID. This is because once you call SetConsent to + // deny consent on Android, calling it again to grant consent may not take + // effect until the app restarts, thus breaking any of those tests that are + // run after this one. + // + // If this test does happen to run earlier (due to randomizing test order, for + // example), the tests that could fail will be skipped (on Android). + // Can't confirm that these do anything but just run them all to ensure the // app doesn't crash. std::map From 9e3772bf3e17b6a152b9e7ba0cf7a5c3d17bea48 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 25 Jan 2024 16:00:38 -0800 Subject: [PATCH 3/3] Format. --- analytics/integration_test/src/integration_test.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index 6d82a25a92..7f7a244b05 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -108,7 +108,8 @@ TEST_F(FirebaseAnalyticsTest, TestGetAnalyticsInstanceID) { LogInfo("Google Play services version: %d", GetGooglePlayServicesVersion()); if (did_test_setconsent_) { LogInfo( - "Skipping %s after TestSetConsent, as the test may fail until the app is restarted.", + "Skipping %s after TestSetConsent, as the test may fail until the app " + "is restarted.", ::testing::UnitTest::GetInstance()->current_test_info()->name()); GTEST_SKIP(); return; @@ -141,7 +142,8 @@ TEST_F(FirebaseAnalyticsTest, TestGetSessionID) { LogInfo("Google Play services version: %d", GetGooglePlayServicesVersion()); if (did_test_setconsent_) { LogInfo( - "Skipping %s after TestSetConsent, as the test may fail until the app is restarted.", + "Skipping %s after TestSetConsent, as the test may fail until the app " + "is restarted.", ::testing::UnitTest::GetInstance()->current_test_info()->name()); GTEST_SKIP(); return; @@ -181,7 +183,8 @@ TEST_F(FirebaseAnalyticsTest, TestResettingGivesNewInstanceId) { LogInfo("Google Play services version: %d", GetGooglePlayServicesVersion()); if (did_test_setconsent_) { LogInfo( - "Skipping %s after TestSetConsent, as the test may fail until the app is restarted.", + "Skipping %s after TestSetConsent, as the test may fail until the app " + "is restarted.", ::testing::UnitTest::GetInstance()->current_test_info()->name()); GTEST_SKIP(); return;