Skip to content

Commit

Permalink
Add more EXPECTations to CancelPushIfCached tests.
Browse files Browse the repository at this point in the history
These tests flake under Fuchsia, suggesting that the ordering of NetLog
events assumed by the tests may not always be valid, e.g. due to
concurrency in resource delivery over QUIC. The additional EXPECTations
will cause more detail to be reported about what was actually NetLog'd.

The tests are also renamed to better express their purpose.

Bug: 775122
Change-Id: Ia1c8aa898d27db2faafd2e79d705b724a7704843
Reviewed-on: https://chromium-review.googlesource.com/806760
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521489}
  • Loading branch information
Wez authored and Commit Bot committed Dec 4, 2017
1 parent 9839538 commit 92be9ad
Showing 1 changed file with 57 additions and 4 deletions.
61 changes: 57 additions & 4 deletions net/url_request/url_request_quic_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ TEST_F(URLRequestQuicTest, TestGetRequest) {
EXPECT_EQ(kHelloBodyValue, delegate.data_received());
}

TEST_F(URLRequestQuicTest, CancelPushIfCached) {
TEST_F(URLRequestQuicTest, CancelPushIfCached_SomeCached) {
base::RunLoop run_loop;
Init();

Expand Down Expand Up @@ -292,7 +292,7 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached) {
net::TestNetLogEntry::List entries;
ExtractNetLog(NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION, &entries);

EXPECT_EQ(4u, entries.size());
ASSERT_EQ(4u, entries.size());

std::string value;
int net_error;
Expand All @@ -301,22 +301,51 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached) {
std::string push_url_2 =
base::StringPrintf("https://%s%s", kTestServerHost, "/favicon.ico");

ASSERT_EQ(entries[0].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[0].phase, net::NetLogEventPhase::BEGIN);
EXPECT_EQ(entries[0].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_TRUE(entries[0].params);
EXPECT_TRUE(entries[0].GetStringValue("push_url", &value));
EXPECT_EQ(value, push_url_1);

ASSERT_EQ(entries[1].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[1].phase, net::NetLogEventPhase::BEGIN);
EXPECT_EQ(entries[1].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_TRUE(entries[1].params);
EXPECT_TRUE(entries[1].GetStringValue("push_url", &value));
EXPECT_EQ(value, push_url_2);

ASSERT_EQ(entries[2].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[2].phase, net::NetLogEventPhase::END);
EXPECT_EQ(entries[2].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[2].source.id, entries[1].source.id);
EXPECT_TRUE(entries[2].params);
// Net error code -400 is found for this lookup transaction, the push is not
// found in the cache.
EXPECT_TRUE(entries[2].GetIntegerValue("net_error", &net_error));
EXPECT_EQ(net_error, -400);

ASSERT_EQ(entries[3].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[3].phase, net::NetLogEventPhase::END);
EXPECT_EQ(entries[3].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[3].source.id, entries[0].source.id);
EXPECT_FALSE(entries[3].params);
// No net error code for this lookup transaction, the push is found.
EXPECT_FALSE(entries[3].GetIntegerValue("net_error", &net_error));

// Verify the reset error count received on the server side.
EXPECT_LE(1u, GetRstErrorCountReceivedByServer(QUIC_STREAM_CANCELLED));
}

TEST_F(URLRequestQuicTest, CancelPushIfCached2) {
TEST_F(URLRequestQuicTest, CancelPushIfCached_AllCached) {
base::RunLoop run_loop;
Init();

Expand Down Expand Up @@ -357,7 +386,7 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached2) {
EXPECT_TRUE(request_1->status().is_success());

// Send a request to /index2.html which pushes /kitten-1.jpg and /favicon.ico.
// Should cancel push for /kitten-1.jpg.
// Should cancel push for both pushed resources, since they're already cached.
CheckLoadTimingDelegate delegate(true);
std::string url =
base::StringPrintf("https://%s%s", kTestServerHost, "/index2.html");
Expand Down Expand Up @@ -387,15 +416,39 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached2) {
std::string push_url_2 =
base::StringPrintf("https://%s%s", kTestServerHost, "/favicon.ico");

ASSERT_EQ(entries[0].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[0].phase, net::NetLogEventPhase::BEGIN);
EXPECT_EQ(entries[0].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_TRUE(entries[0].params);
EXPECT_TRUE(entries[0].GetStringValue("push_url", &value));
EXPECT_EQ(value, push_url_1);

ASSERT_EQ(entries[1].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[1].phase, net::NetLogEventPhase::BEGIN);
EXPECT_EQ(entries[1].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_TRUE(entries[1].params);
EXPECT_TRUE(entries[1].GetStringValue("push_url", &value));
EXPECT_EQ(value, push_url_2);

ASSERT_EQ(entries[2].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[2].phase, net::NetLogEventPhase::END);
EXPECT_EQ(entries[2].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_FALSE(entries[2].params);
// No net error code for this lookup transaction, the push is found.
EXPECT_FALSE(entries[2].GetIntegerValue("net_error", &net_error));

ASSERT_EQ(entries[3].type,
net::NetLogEventType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_EQ(entries[3].phase, net::NetLogEventPhase::END);
EXPECT_EQ(entries[3].source.type,
net::NetLogSourceType::SERVER_PUSH_LOOKUP_TRANSACTION);
EXPECT_FALSE(entries[3].params);
// No net error code for this lookup transaction, the push is found.
EXPECT_FALSE(entries[3].GetIntegerValue("net_error", &net_error));

Expand Down

0 comments on commit 92be9ad

Please sign in to comment.