Commit f3e549cc authored by Matt Menke's avatar Matt Menke Committed by Commit Bot

Attempt to fix SimpleURLLoaderTest.OnUploadProgressCallback.

Sometimes the write-to-file tests timeout on the bots. Since the other
write to file tests all pass (though they write less), and there have
been no reports of the code hanging in the wild, seems likely an issue
with bot performance, rather than a bug in production code. Just reduce
the amount of data the tests write to disk in the write-to-file case.

Bug: 1035127
Change-Id: I1199d22c5b65110f070f7c000881a564c8a44649
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036630
Auto-Submit: Matt Menke <mmenke@chromium.org>
Reviewed-by: default avatarMaksim Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737957}
parent 3953668f
...@@ -136,6 +136,12 @@ class SimpleLoaderTestHelper : public SimpleURLLoaderStreamConsumer { ...@@ -136,6 +136,12 @@ class SimpleLoaderTestHelper : public SimpleURLLoaderStreamConsumer {
std::move(on_destruction_callback_).Run(); std::move(on_destruction_callback_).Run();
} }
// Returns true if the DownloadType indicates the response body is being saved
// to disk. Writing to disk can sometimes be slow on the bots.
static bool IsDownloadTypeToFile(DownloadType type) {
return type == DownloadType::TO_FILE || type == DownloadType::TO_TEMP_FILE;
}
// File path that will be written to. // File path that will be written to.
const base::FilePath& dest_path() const { const base::FilePath& dest_path() const {
DCHECK_EQ(DownloadType::TO_FILE, download_type_); DCHECK_EQ(DownloadType::TO_FILE, download_type_);
...@@ -3167,11 +3173,19 @@ TEST_F(SimpleURLLoaderMockTimeTest, TimeoutAfterRetryTriggered) { ...@@ -3167,11 +3173,19 @@ TEST_F(SimpleURLLoaderMockTimeTest, TimeoutAfterRetryTriggered) {
} }
TEST_P(SimpleURLLoaderTest, OnUploadProgressCallback) { TEST_P(SimpleURLLoaderTest, OnUploadProgressCallback) {
// The size of the payload cannot be bigger than std::string long_string;
// net::test_server::<anonymous>::kRequestSizeLimit which is if (SimpleLoaderTestHelper::IsDownloadTypeToFile(GetParam())) {
// 64Mb. We set a pretty large value in order to ensure multiple // Use a smaller upload body when writing to disk - sometimes creating a
// progress update calls even on fast machines. // large file takes a while on the bots (and, strangely, sometimes it
std::string long_string = GetLongUploadBody(31 * 1024 * 1024); // performs fine on the exact same bot).
long_string = GetLongUploadBody();
} else {
// The size of the payload cannot be bigger than
// net::test_server::<anonymous>::kRequestSizeLimit which is
// 64Mb. We set a pretty large value in order to ensure multiple
// progress update calls even on fast machines.
long_string = GetLongUploadBody(31 * 1024 * 1024);
}
std::unique_ptr<SimpleLoaderTestHelper> test_helper = std::unique_ptr<SimpleLoaderTestHelper> test_helper =
CreateHelperForURL(test_server_.GetURL("/echo"), "POST"); CreateHelperForURL(test_server_.GetURL("/echo"), "POST");
test_helper->simple_url_loader()->AttachStringForUpload(long_string, test_helper->simple_url_loader()->AttachStringForUpload(long_string,
...@@ -3179,14 +3193,11 @@ TEST_P(SimpleURLLoaderTest, OnUploadProgressCallback) { ...@@ -3179,14 +3193,11 @@ TEST_P(SimpleURLLoaderTest, OnUploadProgressCallback) {
uint64_t progress = 0; uint64_t progress = 0;
test_helper->simple_url_loader()->SetOnUploadProgressCallback( test_helper->simple_url_loader()->SetOnUploadProgressCallback(
base::BindRepeating( base::BindLambdaForTesting([&](uint64_t current, uint64_t total) {
[](uint64_t* progress, uint64_t current, uint64_t total) { EXPECT_GT(current, progress);
EXPECT_GT(current, *progress); EXPECT_GE(total, current);
EXPECT_GE(total, current); progress = current;
*progress = current; }));
},
base::Unretained(&progress)));
test_helper->StartSimpleLoaderAndWait(url_loader_factory_.get()); test_helper->StartSimpleLoaderAndWait(url_loader_factory_.get());
EXPECT_EQ(net::OK, test_helper->simple_url_loader()->NetError()); EXPECT_EQ(net::OK, test_helper->simple_url_loader()->NetError());
EXPECT_EQ(long_string.size(), progress); EXPECT_EQ(long_string.size(), progress);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment