Skip to content

Commit 29da398

Browse files
authored
Fix storage large download timeout. (#1513)
* Handle large downloads timing out without failing the test. * Additional work in adding timeout to large file download. * Increase timeout to 2 minutes.
1 parent 05049d2 commit 29da398

File tree

1 file changed

+83
-24
lines changed

1 file changed

+83
-24
lines changed

storage/integration_test/src/integration_test.cc

Lines changed: 83 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ namespace firebase_testapp_automated {
5656

5757
using app_framework::LogDebug;
5858
using app_framework::LogError;
59+
using app_framework::LogWarning;
5960

6061
// You can customize the Storage URL here.
6162
const char* kStorageUrl = nullptr;
@@ -1209,7 +1210,8 @@ class StorageListener : public firebase::storage::Listener {
12091210
: on_paused_was_called_(false),
12101211
on_progress_was_called_(false),
12111212
resume_succeeded_(false),
1212-
last_bytes_transferred_(-1) {}
1213+
last_bytes_transferred_(-1),
1214+
timeout_time_(0) {}
12131215

12141216
// Tracks whether OnPaused was ever called and resumes the transfer.
12151217
void OnPaused(firebase::storage::Controller* controller) override {
@@ -1231,6 +1233,14 @@ class StorageListener : public firebase::storage::Listener {
12311233
}
12321234

12331235
void OnProgress(firebase::storage::Controller* controller) override {
1236+
// Check for timeout.
1237+
if (timeout_time_ > 0) {
1238+
if (app_framework::GetCurrentTimeInMicroseconds() >= timeout_time_) {
1239+
timeout_time_ = -1;
1240+
controller->Cancel();
1241+
}
1242+
}
1243+
12341244
int64_t bytes_transferred = controller->bytes_transferred();
12351245
// Only update when the byte count changed, to avoid spamming the log.
12361246
if (last_bytes_transferred_ != bytes_transferred) {
@@ -1245,11 +1255,22 @@ class StorageListener : public firebase::storage::Listener {
12451255
bool on_progress_was_called() const { return on_progress_was_called_; }
12461256
bool resume_succeeded() const { return resume_succeeded_; }
12471257

1258+
void SetTimeoutSeconds(int seconds_from_now) {
1259+
int64_t microseconds_from_now =
1260+
static_cast<int64_t>(seconds_from_now) * 1000000L;
1261+
timeout_time_ =
1262+
app_framework::GetCurrentTimeInMicroseconds() + microseconds_from_now;
1263+
}
1264+
1265+
bool DidTimeout() { return (timeout_time_ == -1); }
1266+
12481267
public:
12491268
bool on_paused_was_called_;
12501269
bool on_progress_was_called_;
12511270
bool resume_succeeded_;
12521271
int64_t last_bytes_transferred_;
1272+
1273+
int64_t timeout_time_;
12531274
};
12541275

12551276
// Contents of a large file, "X" will be replaced with a different character
@@ -1332,21 +1353,40 @@ TEST_F(FirebaseStorageTest, TestLargeFilePauseResumeAndDownloadCancel) {
13321353
EXPECT_EQ(metadata->size_bytes(), kLargeFileSize);
13331354

13341355
FLAKY_TEST_SECTION_END();
1356+
const int kDownloadTimeoutSeconds = 120;
13351357

13361358
// Download the file and confirm it's correct.
13371359
{
13381360
std::vector<char> buffer(kLargeFileSize);
13391361
memset(&buffer[0], 0, kLargeFileSize);
13401362
LogDebug("Downloading large file for comparison.");
13411363
StorageListener listener;
1342-
firebase::Future<size_t> future = RunWithRetry<size_t>(
1343-
[&]() { return ref.GetBytes(&buffer[0], kLargeFileSize, &listener); });
1344-
WaitForCompletion(future, "GetBytes");
1345-
ASSERT_NE(future.result(), nullptr);
1346-
size_t file_size = *future.result();
1347-
EXPECT_EQ(file_size, kLargeFileSize) << "Read size did not match";
1348-
EXPECT_TRUE(memcmp(kLargeTestFile.c_str(), &buffer[0], kLargeFileSize) == 0)
1349-
<< "Read large file failed, contents did not match.";
1364+
firebase::storage::Controller controller;
1365+
firebase::Future<size_t> future;
1366+
1367+
FLAKY_TEST_SECTION_BEGIN();
1368+
1369+
future = ref.GetBytes(&buffer[0], kLargeFileSize, &listener, &controller);
1370+
listener.SetTimeoutSeconds(kDownloadTimeoutSeconds);
1371+
ASSERT_TRUE(controller.is_valid());
1372+
WaitForCompletionAnyResult(future, "GetBytes");
1373+
if (!listener.DidTimeout()) {
1374+
EXPECT_EQ(future.error(), 0);
1375+
}
1376+
1377+
FLAKY_TEST_SECTION_END();
1378+
1379+
if (!listener.DidTimeout()) {
1380+
ASSERT_NE(future.result(), nullptr);
1381+
size_t file_size = *future.result();
1382+
EXPECT_EQ(file_size, kLargeFileSize) << "Read size did not match";
1383+
EXPECT_TRUE(memcmp(kLargeTestFile.c_str(), &buffer[0], kLargeFileSize) ==
1384+
0)
1385+
<< "Read large file failed, contents did not match.";
1386+
} else {
1387+
LogWarning("Download timed out after %d seconds.",
1388+
kDownloadTimeoutSeconds);
1389+
}
13501390
}
13511391
#if FIREBASE_PLATFORM_DESKTOP
13521392
FLAKY_TEST_SECTION_BEGIN();
@@ -1374,18 +1414,24 @@ TEST_F(FirebaseStorageTest, TestLargeFilePauseResumeAndDownloadCancel) {
13741414
FAIL() << "Pause failed";
13751415
}
13761416

1377-
WaitForCompletion(future, "GetBytes");
1417+
listener.SetTimeoutSeconds(kDownloadTimeoutSeconds);
1418+
WaitForCompletionAnyResult(future, "GetBytes");
13781419

13791420
LogDebug("Download complete.");
13801421

13811422
// Ensure the progress and pause callbacks were called.
13821423
EXPECT_TRUE(listener.on_paused_was_called());
13831424
EXPECT_TRUE(listener.on_progress_was_called());
13841425
EXPECT_TRUE(listener.resume_succeeded());
1385-
EXPECT_NE(future.result(), nullptr);
1386-
size_t file_size = *future.result();
1387-
EXPECT_EQ(file_size, kLargeFileSize);
1388-
EXPECT_EQ(memcmp(kLargeTestFile.c_str(), &buffer[0], kLargeFileSize), 0);
1426+
if (!listener.DidTimeout()) {
1427+
EXPECT_EQ(future.error(), 0);
1428+
EXPECT_NE(future.result(), nullptr);
1429+
size_t file_size = *future.result();
1430+
EXPECT_EQ(file_size, kLargeFileSize);
1431+
EXPECT_EQ(memcmp(kLargeTestFile.c_str(), &buffer[0], kLargeFileSize), 0);
1432+
} else {
1433+
LogWarning("Download timed out after %d seconds.", kDownloadTimeoutSeconds);
1434+
}
13891435

13901436
FLAKY_TEST_SECTION_END();
13911437
#else
@@ -1397,23 +1443,36 @@ TEST_F(FirebaseStorageTest, TestLargeFilePauseResumeAndDownloadCancel) {
13971443
LogDebug("Downloading large file.");
13981444
StorageListener listener;
13991445
firebase::storage::Controller controller;
1400-
firebase::Future<size_t> future = RunWithRetry<size_t>([&]() {
1401-
return ref.GetBytes(&buffer[0], kLargeFileSize, &listener, &controller);
1402-
});
1446+
firebase::Future<size_t> future;
1447+
1448+
FLAKY_TEST_SECTION_BEGIN();
1449+
1450+
future = ref.GetBytes(&buffer[0], kLargeFileSize, &listener, &controller);
1451+
listener.SetTimeoutSeconds(kDownloadTimeoutSeconds);
14031452
ASSERT_TRUE(controller.is_valid());
1453+
WaitForCompletionAnyResult(future, "GetBytes");
1454+
if (!listener.DidTimeout()) {
1455+
EXPECT_EQ(future.error(), 0);
1456+
}
1457+
1458+
FLAKY_TEST_SECTION_END();
14041459

1405-
WaitForCompletion(future, "GetBytes");
14061460
LogDebug("Download complete.");
14071461

14081462
// Ensure the progress callback was called.
14091463
EXPECT_TRUE(listener.on_progress_was_called());
14101464
EXPECT_FALSE(listener.on_paused_was_called());
1411-
1412-
ASSERT_NE(future.result(), nullptr);
1413-
size_t file_size = *future.result();
1414-
EXPECT_EQ(file_size, kLargeFileSize) << "Read size did not match";
1415-
EXPECT_TRUE(memcmp(kLargeTestFile.c_str(), &buffer[0], kLargeFileSize) == 0)
1416-
<< "Read large file failed, contents did not match.";
1465+
if (!listener.DidTimeout()) {
1466+
ASSERT_NE(future.result(), nullptr);
1467+
size_t file_size = *future.result();
1468+
EXPECT_EQ(file_size, kLargeFileSize) << "Read size did not match";
1469+
EXPECT_TRUE(memcmp(kLargeTestFile.c_str(), &buffer[0], kLargeFileSize) ==
1470+
0)
1471+
<< "Read large file failed, contents did not match.";
1472+
} else {
1473+
LogWarning("Download timed out after %d seconds.",
1474+
kDownloadTimeoutSeconds);
1475+
}
14171476
}
14181477
#endif // FIREBASE_PLATFORM_DESKTOP
14191478

0 commit comments

Comments
 (0)