components/offline_pages/background/request_coordinator_unittest.cc - Issue 2425873003: [Offline Pages] Add evaluation test support in RequestCoordinator.

Unified Diff: components/offline_pages/background/request_coordinator_unittest.cc

Issue 2425873003: [Offline Pages] Add evaluation test support in RequestCoordinator. (Closed)

Patch Set: Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: components/offline_pages/background/request_coordinator_unittest.cc

diff --git a/components/offline_pages/background/request_coordinator_unittest.cc b/components/offline_pages/background/request_coordinator_unittest.cc

index 6a2c95d5a2de562bac380d1b8c128462634f70e4..b8ebf26e66f66546160bfe5150771332b3c843a0 100644

--- a/components/offline_pages/background/request_coordinator_unittest.cc

+++ b/components/offline_pages/background/request_coordinator_unittest.cc

@@ -68,7 +68,6 @@ class SchedulerStub : public Scheduler {

conditions_ = trigger_conditions;

}

// Unschedules the currently scheduled task, if any.

void Unschedule() override {

unschedule_called_ = true;

@@ -240,6 +239,7 @@ class RequestCoordinatorTest

// Empty callback function.

void EmptyCallbackFunction(bool result) {

Pete Williamson 2016/10/18 19:24:32 This should be renamed consistent with whatever na

romax 2016/10/18 20:31:51 Done.

+ last_empty_callback_called_ = true;

}

// Callback function which releases a wait for it.

@@ -326,6 +326,10 @@ class RequestCoordinatorTest

ObserverStub observer() { return observer_; }

+ bool last_empty_callback_called() const {

dougarnett 2016/10/18 17:26:32 don't like this naming - maybe immediate_schedule_

romax 2016/10/18 20:31:51 Done.

+ return last_empty_callback_called_;

+ }

private:

RequestQueue::GetRequestsResult last_get_requests_result_;

MultipleItemStatuses last_remove_results_;

@@ -337,6 +341,7 @@ class RequestCoordinatorTest

OfflinerStub* offliner_;

base::WaitableEvent waiter_;

ObserverStub observer_;

+ bool last_empty_callback_called_;

};

RequestCoordinatorTest::RequestCoordinatorTest()

@@ -345,7 +350,8 @@ RequestCoordinatorTest::RequestCoordinatorTest()

task_runner_handle_(task_runner_),

offliner_(nullptr),

waiter_(base::WaitableEvent::ResetPolicy::MANUAL,

- base::WaitableEvent::InitialState::NOT_SIGNALED) {}

+ base::WaitableEvent::InitialState::NOT_SIGNALED),

+ last_empty_callback_called_(false) {}

RequestCoordinatorTest::~RequestCoordinatorTest() {}

@@ -411,6 +417,8 @@ TEST_F(RequestCoordinatorTest, StartProcessingWithNoRequests) {

&RequestCoordinatorTest::EmptyCallbackFunction,

base::Unretained(this));

EXPECT_TRUE(coordinator()->StartProcessing(device_conditions, callback));

+ PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

}

TEST_F(RequestCoordinatorTest, StartProcessingWithRequestInProgress) {

@@ -435,24 +443,86 @@ TEST_F(RequestCoordinatorTest, StartProcessingWithRequestInProgress) {

EXPECT_TRUE(coordinator()->StartProcessing(device_conditions, callback));

PumpLoop();

EXPECT_TRUE(is_busy());

+ // Since the offliner is disabled, this callback should not be called.

+ EXPECT_FALSE(last_empty_callback_called());

// Now trying to start processing on another request should return false.

EXPECT_FALSE(coordinator()->StartProcessing(device_conditions, callback));

}

TEST_F(RequestCoordinatorTest, SavePageLater) {

+ // Set up device conditions for the test and enable the offliner.

+ DeviceConditions device_conditions(false, 75,

+ net::NetworkChangeNotifier::CONNECTION_3G);

+ SetDeviceConditionsForTest(device_conditions);

+ SetEffectiveConnectionTypeForTest(

+ net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_3G);

+ EnableOfflinerCallback(true);

+ base::Callback<void(bool)> callback = base::Bind(

+ &RequestCoordinatorTest::EmptyCallbackFunction, base::Unretained(this));

+ // The user-requested request which gets processed by SavePageLater

+ // would invoke user request callback.

+ coordinator()->SetUserRequestCallbackForTest(callback);

EXPECT_NE(

coordinator()->SavePageLater(

kUrl1, kClientId1, kUserRequested,

RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER), 0);

// Expect that a request got placed on the queue.

+ coordinator()->queue()->GetRequests(base::Bind(

+ &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));

+ // Wait for callbacks to finish, both request queue and offliner.

+ PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

+ // Check the request queue is as expected.

+ EXPECT_EQ(1UL, last_requests().size());

+ EXPECT_EQ(kUrl1, last_requests().at(0)->url());

+ EXPECT_EQ(kClientId1, last_requests().at(0)->client_id());

+ // Expect that the scheduler got notified.

+ SchedulerStub* scheduler_stub =

+ reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());

+ EXPECT_TRUE(scheduler_stub->schedule_called());

+ EXPECT_EQ(coordinator()

+ ->GetTriggerConditions(last_requests()[0]->user_requested())

+ .minimum_battery_percentage,

+ scheduler_stub->conditions()->minimum_battery_percentage);

+ // Check that the observer got the notification that a page is available

+ EXPECT_TRUE(observer().added_called());

+TEST_F(RequestCoordinatorTest, SavePageLaterFailed) {

+ // Set up device conditions for the test and enable the offliner.

+ DeviceConditions device_conditions(false, 75,

+ net::NetworkChangeNotifier::CONNECTION_3G);

+ SetDeviceConditionsForTest(device_conditions);

+ SetEffectiveConnectionTypeForTest(

+ net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_3G);

+ EnableOfflinerCallback(true);

+ base::Callback<void(bool)> callback = base::Bind(

+ &RequestCoordinatorTest::EmptyCallbackFunction, base::Unretained(this));

+ // The user-requested request which gets processed by SavePageLater

+ // would invoke user request callback.

+ coordinator()->SetUserRequestCallbackForTest(callback);

+ EXPECT_TRUE(

+ coordinator()->SavePageLater(

+ kUrl1, kClientId1, kUserRequested,

+ RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER) != 0);

+ // Expect that a request got placed on the queue.

coordinator()->queue()->GetRequests(

base::Bind(&RequestCoordinatorTest::GetRequestsDone,

base::Unretained(this)));

// Wait for callbacks to finish, both request queue and offliner.

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

// Check the request queue is as expected.

EXPECT_EQ(1UL, last_requests().size());

@@ -499,6 +569,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestSucceeded) {

// for callbacks.

SendOfflinerDoneCallback(request, Offliner::RequestStatus::SAVED);

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

// Verify the request gets removed from the queue, and wait for callbacks.

coordinator()->queue()->GetRequests(

@@ -554,6 +625,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {

SendOfflinerDoneCallback(request,

Offliner::RequestStatus::PRERENDERING_FAILED);

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

// TODO(dougarnett): Consider injecting mock RequestPicker for this test

// and verifying that there is no attempt to pick another request following

@@ -607,6 +679,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoRetryFailure) {

SendOfflinerDoneCallback(

request, Offliner::RequestStatus::PRERENDERING_FAILED_NO_RETRY);

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

// TODO(dougarnett): Consider injecting mock RequestPicker for this test

// and verifying that there is as attempt to pick another request following

@@ -650,6 +723,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneForegroundCancel) {

SendOfflinerDoneCallback(request,

Offliner::RequestStatus::FOREGROUND_CANCELED);

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

// Verify the request is not removed from the queue, and wait for callbacks.

coordinator()->queue()->GetRequests(base::Bind(

@@ -687,6 +761,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDonePrerenderingCancel) {

SendOfflinerDoneCallback(request,

Offliner::RequestStatus::PRERENDERING_CANCELED);

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

// Verify the request is not removed from the queue, and wait for callbacks.

coordinator()->queue()->GetRequests(base::Bind(

@@ -743,6 +818,7 @@ TEST_F(RequestCoordinatorTest, RequestNotPickedNonUserRequestedItemsRemain) {

PumpLoop();

EXPECT_FALSE(is_starting());

+ EXPECT_TRUE(last_empty_callback_called());

// The scheduler should have been called to schedule the non-user requested

// task.

@@ -814,6 +890,7 @@ TEST_F(RequestCoordinatorTest, StartProcessingWithLoadingDisabled) {

// Let the async callbacks in the request coordinator run.

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

EXPECT_FALSE(is_starting());

EXPECT_EQ(Offliner::PRERENDERING_NOT_STARTED, last_offlining_status());

@@ -845,6 +922,7 @@ TEST_F(RequestCoordinatorTest, StartProcessingThenStopProcessingImmediately) {

// Let the async callbacks in the request coordinator run.

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

EXPECT_FALSE(is_starting());

@@ -882,6 +960,8 @@ TEST_F(RequestCoordinatorTest, StartProcessingThenStopProcessingLater) {

// Let all the async parts of the start processing pipeline run to completion.

PumpLoop();

+ // Since the offliner is disabled, this callback should not be called.

+ EXPECT_FALSE(last_empty_callback_called());

// Coordinator should now be busy.

EXPECT_TRUE(is_busy());

@@ -926,6 +1006,8 @@ TEST_F(RequestCoordinatorTest, RemoveInflightRequest) {

// Let all the async parts of the start processing pipeline run to completion.

PumpLoop();

+ // Since the offliner is disabled, this callback should not be called.

+ EXPECT_FALSE(last_empty_callback_called());

// Remove the request while it is processing.

std::vector<int64_t> request_ids{kRequestId1};

@@ -962,6 +1044,7 @@ TEST_F(RequestCoordinatorTest, MarkRequestCompleted) {

// Call the method under test, making sure we send SUCCESS to the observer.

coordinator()->MarkRequestCompleted(request_id);

PumpLoop();

+ EXPECT_TRUE(last_empty_callback_called());

// Our observer should have seen SUCCESS instead of REMOVED.

EXPECT_EQ(RequestCoordinator::BackgroundSavePageResult::SUCCESS,

« components/offline_pages/background/request_coordinator.cc ('K') | « components/offline_pages/background/request_coordinator.cc ('k') | no next file » | no next file with comments »