Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 1f2089b

Browse files
Fix a few flaky tests in several products (Auth, Analytics, Storage, RTDB, Firestore) (#1520)
* Fix flaky tests on Auth, Analytics, and GMA (with additional debug output). * Revert GMA changes, no longer needed. * Add logging for storage timestamp * Fix build error. * Change timestamp logic. * Deflake the Storage timestamp test by getting the timestamp from the remote server. * Add retries. * Increase timeout on Android FTL tests. * Handle Analytics test flakiness on Android. * Add remote timestamp verification to RTDB test as well. * Fix extra semicolon. * Change UpdateChildren test to use remote timestamp.
1 parent 7374b76 commit 1f2089b

File tree

5 files changed

+86
-17
lines changed

5 files changed

+86
-17
lines changed

‎.github/workflows/integration_tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ jobs:
10471047
- id: ftl_test
10481048
if: steps.device-info.outputs.device_type == 'ftl'
10491049
uses: FirebaseExtended/github-actions/firebase-test-lab@v1.4
1050-
timeout-minutes: 90
1050+
timeout-minutes: 120
10511051
with:
10521052
credentials_json: ${{ secrets.FIREBASE_SERVICE_ACCOUNT_CREDENTIALS }}
10531053
testapp_dir: testapps

‎analytics/integration_test/src/integration_test.cc

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,14 @@ TEST_F(FirebaseAnalyticsTest, TestSetSessionTimeoutDuraction) {
100100
}
101101

102102
TEST_F(FirebaseAnalyticsTest, TestGetAnalyticsInstanceID) {
103+
FLAKY_TEST_SECTION_BEGIN();
104+
103105
firebase::Future<std::string> future =
104106
firebase::analytics::GetAnalyticsInstanceId();
105107
WaitForCompletion(future, "GetAnalyticsInstanceId");
106108
EXPECT_FALSE(future.result()->empty());
109+
110+
FLAKY_TEST_SECTION_END();
107111
}
108112

109113
TEST_F(FirebaseAnalyticsTest, TestGetSessionID) {
@@ -232,10 +236,7 @@ TEST_F(FirebaseAnalyticsTest, TestLogEventWithMultipleParameters) {
232236
}
233237

234238
TEST_F(FirebaseAnalyticsTest, TestResettingGivesNewInstanceId) {
235-
// Test is flaky on iPhone due to a known issue in iOS. See b/143656277.
236-
#if TARGET_OS_IPHONE
237239
FLAKY_TEST_SECTION_BEGIN();
238-
#endif // TARGET_OS_IPHONE
239240

240241
firebase::Future<std::string> future =
241242
firebase::analytics::GetAnalyticsInstanceId();
@@ -251,9 +252,7 @@ TEST_F(FirebaseAnalyticsTest, TestResettingGivesNewInstanceId) {
251252
EXPECT_FALSE(future.result()->empty());
252253
EXPECT_NE(instance_id, new_instance_id);
253254

254-
#if TARGET_OS_IPHONE
255255
FLAKY_TEST_SECTION_END();
256-
#endif // TARGET_OS_IPHONE
257256
}
258257

259258
} // namespace firebase_testapp_automated

‎auth/integration_test/src/integration_test.cc

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,10 @@ TEST_F(FirebaseAuthTest, TestUpdateEmailAndPassword_DEPRECATED) {
948948
}
949949

950950
TEST_F(FirebaseAuthTest, TestLinkAnonymousUserWithEmailCredential) {
951-
SignInAnonymously();
951+
FLAKY_TEST_SECTION_BEGIN();
952+
953+
WaitForCompletion(auth_->SignInAnonymously(), "SignInAnonymously");
954+
952955
firebase::auth::User user = auth_->current_user();
953956
EXPECT_TRUE(user.is_valid());
954957
std::string email = GenerateEmailAddress();
@@ -958,7 +961,7 @@ TEST_F(FirebaseAuthTest, TestLinkAnonymousUserWithEmailCredential) {
958961
WaitForCompletion(user.LinkWithCredential(credential), "LinkWithCredential");
959962
WaitForCompletion(user.Unlink(credential.provider().c_str()), "Unlink");
960963
SignOut();
961-
SignInAnonymously();
964+
WaitForCompletion(auth_->SignInAnonymously(), "SignInAnonymously");
962965
user = auth_->current_user();
963966
EXPECT_TRUE(user.is_valid());
964967
std::string email1 = GenerateEmailAddress();
@@ -979,10 +982,16 @@ TEST_F(FirebaseAuthTest, TestLinkAnonymousUserWithEmailCredential) {
979982
firebase::auth::kAuthErrorProviderAlreadyLinked);
980983
WaitForCompletion(user.Unlink(credential.provider().c_str()), "Unlink 2");
981984
DeleteUser();
985+
986+
// In case any operations failed, force signout before retrying the test.
987+
SignOut();
988+
FLAKY_TEST_SECTION_END();
982989
}
983990

984991
TEST_F(FirebaseAuthTest, TestLinkAnonymousUserWithEmailCredential_DEPRECATED) {
985-
SignInAnonymously_DEPRECATED();
992+
FLAKY_TEST_SECTION_BEGIN();
993+
994+
WaitForCompletion(auth_->SignInAnonymously_DEPRECATED(), "SignInAnonymously");
986995
ASSERT_NE(auth_->current_user_DEPRECATED(), nullptr);
987996
firebase::auth::User* user = auth_->current_user_DEPRECATED();
988997
std::string email = GenerateEmailAddress();
@@ -994,7 +1003,7 @@ TEST_F(FirebaseAuthTest, TestLinkAnonymousUserWithEmailCredential_DEPRECATED) {
9941003
WaitForCompletion(user->Unlink_DEPRECATED(credential.provider().c_str()),
9951004
"Unlink");
9961005
SignOut();
997-
SignInAnonymously_DEPRECATED();
1006+
WaitForCompletion(auth_->SignInAnonymously_DEPRECATED(), "SignInAnonymously");
9981007
EXPECT_NE(auth_->current_user_DEPRECATED(), nullptr);
9991008
std::string email1 = GenerateEmailAddress();
10001009
firebase::auth::Credential credential1 =
@@ -1012,6 +1021,9 @@ TEST_F(FirebaseAuthTest, TestLinkAnonymousUserWithEmailCredential_DEPRECATED) {
10121021
WaitForCompletion(user->Unlink_DEPRECATED(credential.provider().c_str()),
10131022
"Unlink_DEPRECATED 2");
10141023
DeleteUser_DEPRECATED();
1024+
1025+
SignOut();
1026+
FLAKY_TEST_SECTION_END();
10151027
}
10161028

10171029
TEST_F(FirebaseAuthTest, TestLinkAnonymousUserWithBadCredential) {

‎database/integration_test/src/integration_test.cc

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ namespace firebase_testapp_automated {
4747
using app_framework::LogDebug;
4848
using app_framework::LogError;
4949
using app_framework::LogInfo;
50+
using app_framework::LogWarning;
5051

5152
using app_framework::ProcessEvents;
5253
using firebase_test_framework::FirebaseTest;
@@ -146,6 +147,8 @@ class FirebaseDatabaseTest : public FirebaseTest {
146147
// Shut down Firebase Database.
147148
void TerminateDatabase();
148149

150+
int64_t GetRemoteTimeInMilliseconds();
151+
149152
firebase::database::DatabaseReference CreateWorkingPath(
150153
bool suppress_cleanup = false);
151154

@@ -365,6 +368,30 @@ firebase::database::DatabaseReference FirebaseDatabaseTest::CreateWorkingPath(
365368
return ref;
366369
}
367370

371+
int64_t FirebaseDatabaseTest::GetRemoteTimeInMilliseconds() {
372+
firebase::database::DatabaseReference ref =
373+
CreateWorkingPath().Child("timestamp");
374+
WaitForCompletionAnyResult(
375+
ref.SetValue(firebase::database::ServerTimestamp()),
376+
"GetRemoteTime_SetValue");
377+
firebase::Future<firebase::database::DataSnapshot> future = ref.GetValue();
378+
WaitForCompletionAnyResult(future, "GetRemoteTime_GetValue");
379+
int64_t timestamp = 0;
380+
if (future.error() == 0 && future.result() &&
381+
future.result()->value().is_int64()) {
382+
timestamp = future.result()->value().int64_value();
383+
}
384+
if (timestamp > 0) {
385+
LogDebug("Got server timestamp: %lld", timestamp);
386+
LogDebug(" Local timestamp: %lld",
387+
app_framework::GetCurrentTimeInMicroseconds() / 1000L);
388+
return timestamp;
389+
} else {
390+
LogWarning("Couldn't get remote timestamp, using local time");
391+
return app_framework::GetCurrentTimeInMicroseconds() / 1000L;
392+
}
393+
}
394+
368395
// Test cases below.
369396
TEST_F(FirebaseDatabaseTest, TestInitializeAndTerminate) {
370397
// Already tested via SetUp() and TearDown().
@@ -460,8 +487,7 @@ TEST_F(FirebaseDatabaseTest, TestSetAndGetSimpleValues) {
460487
WaitForCompletion(f7, "GetLongDouble");
461488

462489
// Get the current time to compare to the Timestamp.
463-
int64_t current_time_milliseconds =
464-
static_cast<int64_t>(time(nullptr)) * 1000L;
490+
int64_t current_time_milliseconds = GetRemoteTimeInMilliseconds();
465491

466492
EXPECT_EQ(f1.result()->value().AsString(), kSimpleString);
467493
EXPECT_EQ(f2.result()->value().AsInt64(), kSimpleInt);
@@ -672,8 +698,8 @@ TEST_F(FirebaseDatabaseTest, TestUpdateChildren) {
672698
WaitForCompletion(update_future, "UpdateChildren");
673699
read_future = ref.Child(test_name).GetValue();
674700
WaitForCompletion(read_future, "GetValue 2");
675-
int64_t current_time_milliseconds =
676-
static_cast<int64_t>(time(nullptr)) * 1000L;
701+
int64_t current_time_milliseconds =GetRemoteTimeInMilliseconds();
702+
677703
EXPECT_THAT(
678704
read_future.result()->value().map(),
679705
UnorderedElementsAre(

‎storage/integration_test/src/integration_test.cc

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ class FirebaseStorageTest : public FirebaseTest {
118118
// Create a unique working folder and return a reference to it.
119119
firebase::storage::StorageReference CreateFolder();
120120

121+
int64_t GetRemoteTimeInMilliseconds();
122+
121123
static firebase::App* shared_app_;
122124
static firebase::auth::Auth* shared_auth_;
123125

@@ -324,6 +326,30 @@ firebase::storage::StorageReference FirebaseStorageTest::CreateFolder() {
324326
return storage_->GetReference(kRootNodeName).Child(saved_url_);
325327
}
326328

329+
int64_t FirebaseStorageTest::GetRemoteTimeInMilliseconds() {
330+
SignIn();
331+
332+
firebase::storage::StorageReference ref =
333+
CreateFolder().Child("timestamp.txt");
334+
firebase::Future<firebase::storage::Metadata> future =
335+
RunWithRetry<firebase::storage::Metadata>(
336+
[&]() { return ref.PutBytes("TS00", 4); });
337+
WaitForCompletionAnyResult(future, "GetRemoteTime_PutBytes");
338+
if (future.error() == 0 && future.result() != nullptr &&
339+
future.result()->creation_time() > 0) {
340+
int64_t timestamp = future.result()->creation_time();
341+
WaitForCompletionAnyResult(RunWithRetry([&]() { return ref.Delete(); }),
342+
"GetRemoteTime_Delete");
343+
LogDebug("Got server timestamp: %lld", timestamp);
344+
LogDebug(" Local timestamp: %lld",
345+
app_framework::GetCurrentTimeInMicroseconds() / 1000L);
346+
return timestamp;
347+
} else {
348+
LogWarning("Couldn't get remote timestamp, using local time");
349+
return app_framework::GetCurrentTimeInMicroseconds() / 1000L;
350+
}
351+
}
352+
327353
// Test cases below.
328354

329355
TEST_F(FirebaseStorageTest, TestInitializeAndTerminate) {
@@ -497,17 +523,23 @@ TEST_F(FirebaseStorageTest, TestWriteAndReadFileWithCustomMetadata) {
497523
ASSERT_NE(metadata, nullptr);
498524

499525
// Get the current time to compare to the Timestamp.
500-
int64_t current_time_seconds = static_cast<int64_t>(time(nullptr));
526+
int64_t current_time_seconds = GetRemoteTimeInMilliseconds() / 1000L;
501527
int64_t updated_time_milliseconds = metadata->updated_time();
502-
int64_t updated_time_seconds = updated_time_milliseconds / 1000;
528+
int64_t updated_time_seconds = updated_time_milliseconds / 1000L;
503529
int64_t time_difference_seconds =
504530
updated_time_seconds - current_time_seconds;
531+
if (time_difference_seconds < 0)
532+
time_difference_seconds = -time_difference_seconds;
505533
// As long as our timestamp is within a day, it's correct enough for
506534
// our purposes.
507535
const int64_t kAllowedTimeDifferenceSeconds = 60L * 60L * 24L;
508536
EXPECT_TRUE(time_difference_seconds < kAllowedTimeDifferenceSeconds &&
509537
time_difference_seconds > -kAllowedTimeDifferenceSeconds)
510-
<< "Bad timestamp in metadata.";
538+
<< "Bad timestamp in metadata. Metadata timestamp is "
539+
<< updated_time_seconds << ", current time is " << current_time_seconds
540+
<< ", difference = " << time_difference_seconds
541+
<< " (allowed difference " << kAllowedTimeDifferenceSeconds << ").";
542+
511543
EXPECT_EQ(metadata->size_bytes(), kSimpleTestFile.size());
512544
EXPECT_EQ(metadata->content_type(), content_type);
513545
ASSERT_NE(metadata->custom_metadata(), nullptr);

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /