libchromeos: Use MockMessageLoop on FakeStream.
The added MockMessageLoop is a mockable FakeMessageLoop. By default the
mock object will behave like a fake one, but it is also possible to set
expectations on the MessageLoop methods.
The MockMessageLoop now replaces the base::TaskRunner used in
FakeStream and runs the posted callbacks from the message loop instead
of running them when they are posted.
libweave is updated to create the FakeStream using a
chromeos::MessageLoop.
BUG=chromium:499886
TEST=FEATURES=test emerge-link libchromeos libweave buffet
Reviewed-on: https://chromium-review.googlesource.com/290632
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
(cherry-pick from
https://chromium.googlesource.com/chromiumos/platform2 at
78c174362c0ce8f75c53ab37da1874533eb1a56a)
Change-Id: Ie3407b9dfc5c821615c03e9fe37ddf11fcc82e9d
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc
index 91e1d12..175614c 100644
--- a/libweave/src/device_registration_info.cc
+++ b/libweave/src/device_registration_info.cc
@@ -467,7 +467,7 @@
notification_channel_starting_ = true;
primary_notification_channel_.reset(new XmppChannel{
- config_->robot_account(), access_token_, task_runner_, network_});
+ config_->robot_account(), access_token_, network_});
primary_notification_channel_->Start(this);
}
diff --git a/libweave/src/notification/xmpp_channel.cc b/libweave/src/notification/xmpp_channel.cc
index 141fd6c..7141f29 100644
--- a/libweave/src/notification/xmpp_channel.cc
+++ b/libweave/src/notification/xmpp_channel.cc
@@ -9,6 +9,7 @@
#include <base/bind.h>
#include <chromeos/backoff_entry.h>
#include <chromeos/data_encoding.h>
+#include <chromeos/message_loops/message_loop.h>
#include <chromeos/streams/file_stream.h>
#include <chromeos/streams/tls_stream.h>
#include <weave/network.h>
@@ -92,13 +93,11 @@
XmppChannel::XmppChannel(
const std::string& account,
const std::string& access_token,
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
Network* network)
: account_{account},
access_token_{access_token},
backoff_entry_{&kDefaultBackoffPolicy},
- task_runner_{task_runner},
- iq_stanza_handler_{new IqStanzaHandler{this, task_runner}} {
+ iq_stanza_handler_{new IqStanzaHandler{this}} {
read_socket_data_.resize(4096);
if (network) {
network->AddOnConnectionChangedCallback(base::Bind(
@@ -127,7 +126,7 @@
// However, if the connection has never been established yet (e.g.
// authorization failed), do not restart right now. Wait till we get
// new credentials.
- task_runner_->PostTask(
+ chromeos::MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&XmppChannel::Restart, task_ptr_factory_.GetWeakPtr()));
} else if (delegate_) {
@@ -140,7 +139,7 @@
// from expat XML parser and some stanza could cause the XMPP stream to be
// reset and the parser to be re-initialized. We don't want to destroy the
// parser while it is performing a callback invocation.
- task_runner_->PostTask(
+ chromeos::MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&XmppChannel::HandleStanza, task_ptr_factory_.GetWeakPtr(),
base::Passed(std::move(stanza))));
@@ -396,7 +395,7 @@
} else {
VLOG(1) << "Delaying connection to XMPP server " << host << " for "
<< backoff_entry_.GetTimeUntilRelease();
- task_runner_->PostDelayedTask(
+ chromeos::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::Bind(&XmppChannel::Connect, task_ptr_factory_.GetWeakPtr(), host,
port, callback),
@@ -469,7 +468,7 @@
base::TimeDelta timeout) {
VLOG(1) << "Next XMPP ping in " << interval << " with timeout " << timeout;
ping_ptr_factory_.InvalidateWeakPtrs();
- task_runner_->PostDelayedTask(
+ chromeos::MessageLoop::current()->PostDelayedTask(
FROM_HERE, base::Bind(&XmppChannel::PingServer,
ping_ptr_factory_.GetWeakPtr(), timeout),
interval);
diff --git a/libweave/src/notification/xmpp_channel.h b/libweave/src/notification/xmpp_channel.h
index 66f7c78..cf58a5b 100644
--- a/libweave/src/notification/xmpp_channel.h
+++ b/libweave/src/notification/xmpp_channel.h
@@ -13,7 +13,6 @@
#include <base/callback_forward.h>
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
-#include <base/single_thread_task_runner.h>
#include <chromeos/backoff_entry.h>
#include <chromeos/streams/stream.h>
@@ -43,7 +42,6 @@
// so you will need to reset the XmppClient every time this happens.
XmppChannel(const std::string& account,
const std::string& access_token,
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
Network* network);
~XmppChannel() override = default;
@@ -154,7 +152,6 @@
chromeos::BackoffEntry backoff_entry_;
NotificationDelegate* delegate_{nullptr};
- scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
XmppStreamParser stream_parser_{this};
bool read_pending_{false};
bool write_pending_{false};
diff --git a/libweave/src/notification/xmpp_channel_unittest.cc b/libweave/src/notification/xmpp_channel_unittest.cc
index 74b158c..22a0b36 100644
--- a/libweave/src/notification/xmpp_channel_unittest.cc
+++ b/libweave/src/notification/xmpp_channel_unittest.cc
@@ -8,16 +8,13 @@
#include <base/test/simple_test_clock.h>
#include <chromeos/bind_lambda.h>
+#include <chromeos/message_loops/fake_message_loop.h>
#include <chromeos/streams/fake_stream.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
namespace weave {
-using ::testing::DoAll;
-using ::testing::Return;
-using ::testing::SetArgPointee;
-using ::testing::_;
namespace {
@@ -82,31 +79,11 @@
"</subscribe></iq>";
} // namespace
-// Mock-like task runner that allow the tests to inspect the calls to
-// TaskRunner::PostDelayedTask and verify the delays.
-class TestTaskRunner : public base::SingleThreadTaskRunner {
- public:
- MOCK_METHOD3(PostDelayedTask,
- bool(const tracked_objects::Location&,
- const base::Closure&,
- base::TimeDelta));
- MOCK_METHOD3(PostNonNestableDelayedTask,
- bool(const tracked_objects::Location&,
- const base::Closure&,
- base::TimeDelta));
-
- bool RunsTasksOnCurrentThread() const { return true; }
-};
-
class FakeXmppChannel : public XmppChannel {
public:
- FakeXmppChannel(
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
- base::Clock* clock)
- : XmppChannel{kAccountName, kAccessToken, task_runner, nullptr},
- fake_stream_{chromeos::Stream::AccessMode::READ_WRITE,
- task_runner,
- clock} {}
+ explicit FakeXmppChannel(base::Clock* clock)
+ : XmppChannel{kAccountName, kAccessToken, nullptr},
+ fake_stream_{chromeos::Stream::AccessMode::READ_WRITE, clock} {}
XmppState state() const { return state_; }
void set_state(XmppState state) { state_ = state; }
@@ -128,22 +105,9 @@
class XmppChannelTest : public ::testing::Test {
protected:
void SetUp() override {
- task_runner_ = new TestTaskRunner;
+ fake_loop_.SetAsCurrent();
- auto callback = [this](const tracked_objects::Location& from_here,
- const base::Closure& task,
- base::TimeDelta delay) -> bool {
- if (ignore_delayed_tasks_ && delay > base::TimeDelta::FromMilliseconds(0))
- return true;
- clock_.Advance(delay);
- message_queue_.push(task);
- return true;
- };
-
- EXPECT_CALL(*task_runner_, PostDelayedTask(_, _, _))
- .WillRepeatedly(testing::Invoke(callback));
-
- xmpp_client_.reset(new FakeXmppChannel{task_runner_, &clock_});
+ xmpp_client_.reset(new FakeXmppChannel{&clock_});
clock_.SetNow(base::Time::Now());
}
@@ -162,21 +126,18 @@
void RunTasks(size_t count) {
while (count > 0) {
- ASSERT_FALSE(message_queue_.empty());
- base::Closure task = message_queue_.front();
- message_queue_.pop();
- task.Run();
+ EXPECT_TRUE(fake_loop_.RunOnce(true /* may_block */));
count--;
}
}
- void SetIgnoreDelayedTasks(bool ignore) { ignore_delayed_tasks_ = true; }
+ void RunLoopUntilIdle() {
+ while (fake_loop_.RunOnce(false /* may_block */)) {}
+ }
std::unique_ptr<FakeXmppChannel> xmpp_client_;
base::SimpleTestClock clock_;
- scoped_refptr<TestTaskRunner> task_runner_;
- std::queue<base::Closure> message_queue_;
- bool ignore_delayed_tasks_{false};
+ chromeos::FakeMessageLoop fake_loop_{&clock_};
};
TEST_F(XmppChannelTest, StartStream) {
@@ -222,7 +183,6 @@
}
TEST_F(XmppChannelTest, HandleStreamRestartedResponse) {
- SetIgnoreDelayedTasks(true);
StartWithState(XmppChannel::XmppState::kStreamRestartedPostAuthentication);
xmpp_client_->fake_stream_.AddReadPacketString({}, kRestartStreamResponse);
xmpp_client_->fake_stream_.ExpectWritePacketString({}, kBindMessage);
diff --git a/libweave/src/notification/xmpp_iq_stanza_handler.cc b/libweave/src/notification/xmpp_iq_stanza_handler.cc
index 4cee499..1a3ec36 100644
--- a/libweave/src/notification/xmpp_iq_stanza_handler.cc
+++ b/libweave/src/notification/xmpp_iq_stanza_handler.cc
@@ -7,6 +7,7 @@
#include <base/bind.h>
#include <base/strings/string_number_conversions.h>
#include <base/strings/stringprintf.h>
+#include <chromeos/message_loops/message_loop.h>
#include "libweave/src/notification/xml_node.h"
#include "libweave/src/notification/xmpp_channel.h"
@@ -46,10 +47,8 @@
} // anonymous namespace
-IqStanzaHandler::IqStanzaHandler(
- XmppChannelInterface* xmpp_channel,
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
- : xmpp_channel_{xmpp_channel}, task_runner_{task_runner} {
+IqStanzaHandler::IqStanzaHandler(XmppChannelInterface* xmpp_channel)
+ : xmpp_channel_{xmpp_channel} {
}
void IqStanzaHandler::SendRequest(const std::string& type,
@@ -76,7 +75,7 @@
requests_.emplace(++last_request_id_, response_callback);
// Schedule a time-out callback for this request.
if (timeout < base::TimeDelta::Max()) {
- task_runner_->PostDelayedTask(
+ chromeos::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::Bind(&IqStanzaHandler::OnTimeOut, weak_ptr_factory_.GetWeakPtr(),
last_request_id_, timeout_callback),
@@ -111,7 +110,7 @@
}
auto p = requests_.find(id);
if (p != requests_.end()) {
- task_runner_->PostTask(
+ chromeos::MessageLoop::current()->PostTask(
FROM_HERE, base::Bind(p->second, base::Passed(std::move(stanza))));
requests_.erase(p);
}
diff --git a/libweave/src/notification/xmpp_iq_stanza_handler.h b/libweave/src/notification/xmpp_iq_stanza_handler.h
index 78e9be2..a8d0245 100644
--- a/libweave/src/notification/xmpp_iq_stanza_handler.h
+++ b/libweave/src/notification/xmpp_iq_stanza_handler.h
@@ -25,9 +25,7 @@
using ResponseCallback = base::Callback<void(std::unique_ptr<XmlNode>)>;
using TimeoutCallback = base::Closure;
- IqStanzaHandler(
- XmppChannelInterface* xmpp_channel,
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
+ explicit IqStanzaHandler(XmppChannelInterface* xmpp_channel);
// Sends <iq> request to the server.
// |type| is the IQ stanza type, one of "get", "set", "query".
@@ -67,7 +65,6 @@
void OnTimeOut(RequestId id, const TimeoutCallback& timeout_callback);
XmppChannelInterface* xmpp_channel_;
- scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
std::map<RequestId, ResponseCallback> requests_;
RequestId last_request_id_{0};
diff --git a/libweave/src/notification/xmpp_iq_stanza_handler_unittest.cc b/libweave/src/notification/xmpp_iq_stanza_handler_unittest.cc
index 80559ff..29ac5af 100644
--- a/libweave/src/notification/xmpp_iq_stanza_handler_unittest.cc
+++ b/libweave/src/notification/xmpp_iq_stanza_handler_unittest.cc
@@ -8,6 +8,7 @@
#include <memory>
#include <chromeos/bind_lambda.h>
+#include <chromeos/message_loops/mock_message_loop.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -15,34 +16,11 @@
#include "libweave/src/notification/xmpp_channel.h"
#include "libweave/src/notification/xmpp_stream_parser.h"
-using testing::Invoke;
-using testing::Return;
using testing::_;
namespace weave {
namespace {
-// Mock-like task runner that allow the tests to inspect the calls to
-// TaskRunner::PostDelayedTask and verify the delays.
-class TestTaskRunner : public base::SingleThreadTaskRunner {
- public:
- TestTaskRunner() = default;
-
- MOCK_METHOD3(PostDelayedTask,
- bool(const tracked_objects::Location&,
- const base::Closure&,
- base::TimeDelta));
- MOCK_METHOD3(PostNonNestableDelayedTask,
- bool(const tracked_objects::Location&,
- const base::Closure&,
- base::TimeDelta));
-
- bool RunsTasksOnCurrentThread() const { return true; }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(TestTaskRunner);
-};
-
class MockXmppChannelInterface : public XmppChannelInterface {
public:
MockXmppChannelInterface() = default;
@@ -93,37 +71,24 @@
}
};
-// Functor to call the |task| immediately.
-struct TaskInvoker {
- bool operator()(const tracked_objects::Location& location,
- const base::Closure& task,
- base::TimeDelta delay) {
- task.Run();
- return true;
- }
-};
-
} // anonymous namespace
class IqStanzaHandlerTest : public testing::Test {
public:
void SetUp() override {
- task_runner_ = new TestTaskRunner;
+ mock_loop_.SetAsCurrent();
iq_stanza_handler_.reset(
- new IqStanzaHandler{&mock_xmpp_channel_, task_runner_});
+ new IqStanzaHandler{&mock_xmpp_channel_});
}
testing::StrictMock<MockXmppChannelInterface> mock_xmpp_channel_;
- scoped_refptr<TestTaskRunner> task_runner_;
+ base::SimpleTestClock clock_;
+ testing::NiceMock<chromeos::MockMessageLoop> mock_loop_{&clock_};
std::unique_ptr<IqStanzaHandler> iq_stanza_handler_;
MockResponseReceiver receiver_;
- TaskInvoker task_invoker_;
};
TEST_F(IqStanzaHandlerTest, SendRequest) {
- EXPECT_CALL(*task_runner_, PostDelayedTask(_, _, _))
- .WillRepeatedly(Return(true));
-
std::string expected_msg = "<iq id='1' type='set'><body/></iq>";
EXPECT_CALL(mock_xmpp_channel_, SendMessage(expected_msg)).Times(1);
iq_stanza_handler_->SendRequest("set", "", "", "<body/>", {}, {});
@@ -143,6 +108,8 @@
expected_msg = "<iq id='5' type='query' from='foo@bar' to='baz'><body/></iq>";
EXPECT_CALL(mock_xmpp_channel_, SendMessage(expected_msg)).Times(1);
iq_stanza_handler_->SendRequest("query", "foo@bar", "baz", "<body/>", {}, {});
+
+ // This test ignores all the posted callbacks.
}
TEST_F(IqStanzaHandlerTest, UnsupportedIqRequest) {
@@ -163,9 +130,8 @@
}
TEST_F(IqStanzaHandlerTest, SequentialResponses) {
- EXPECT_CALL(*task_runner_, PostDelayedTask(_, _, _))
- .Times(2)
- .WillRepeatedly(Return(true));
+ EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, _))
+ .Times(2);
EXPECT_CALL(mock_xmpp_channel_, SendMessage(_)).Times(2);
iq_stanza_handler_->SendRequest("set", "", "", "<body/>",
@@ -173,9 +139,8 @@
iq_stanza_handler_->SendRequest("get", "", "", "<body/>",
receiver_.callback(2), {});
- EXPECT_CALL(*task_runner_, PostDelayedTask(_, _, _))
- .Times(2)
- .WillRepeatedly(Invoke(task_invoker_));
+ EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, _))
+ .Times(2);
EXPECT_CALL(receiver_, OnResponse(1, "foo"));
auto request = XmlParser{}.Parse("<iq id='1' type='result'><foo/></iq>");
@@ -184,12 +149,13 @@
EXPECT_CALL(receiver_, OnResponse(2, "bar"));
request = XmlParser{}.Parse("<iq id='2' type='result'><bar/></iq>");
EXPECT_TRUE(iq_stanza_handler_->HandleIqStanza(std::move(request)));
+
+ mock_loop_.Run();
}
TEST_F(IqStanzaHandlerTest, OutOfOrderResponses) {
- EXPECT_CALL(*task_runner_, PostDelayedTask(_, _, _))
- .Times(2)
- .WillRepeatedly(Return(true));
+ EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, _))
+ .Times(2);
EXPECT_CALL(mock_xmpp_channel_, SendMessage(_)).Times(2);
iq_stanza_handler_->SendRequest("set", "", "", "<body/>",
@@ -197,9 +163,8 @@
iq_stanza_handler_->SendRequest("get", "", "", "<body/>",
receiver_.callback(2), {});
- EXPECT_CALL(*task_runner_, PostDelayedTask(_, _, _))
- .Times(2)
- .WillRepeatedly(Invoke(task_invoker_));
+ EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, _))
+ .Times(2);
EXPECT_CALL(receiver_, OnResponse(2, "bar"));
auto request = XmlParser{}.Parse("<iq id='2' type='result'><bar/></iq>");
@@ -208,11 +173,13 @@
EXPECT_CALL(receiver_, OnResponse(1, "foo"));
request = XmlParser{}.Parse("<iq id='1' type='result'><foo/></iq>");
EXPECT_TRUE(iq_stanza_handler_->HandleIqStanza(std::move(request)));
+
+ mock_loop_.Run();
}
TEST_F(IqStanzaHandlerTest, RequestTimeout) {
- EXPECT_CALL(*task_runner_, PostDelayedTask(_, _, _))
- .WillOnce(Invoke(task_invoker_));
+ EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, _))
+ .Times(1);
bool called = false;
auto on_timeout = [&called]() { called = true; };
@@ -221,6 +188,7 @@
EXPECT_FALSE(called);
iq_stanza_handler_->SendRequest("set", "", "", "<body/>", {},
base::Bind(on_timeout));
+ mock_loop_.Run();
EXPECT_TRUE(called);
}
diff --git a/libweave/src/weave_testrunner.cc b/libweave/src/weave_testrunner.cc
index f050805..df73d44 100644
--- a/libweave/src/weave_testrunner.cc
+++ b/libweave/src/weave_testrunner.cc
@@ -3,10 +3,11 @@
// found in the LICENSE file.
#include <base/at_exit.h>
+#include <chromeos/test_helpers.h>
#include <gtest/gtest.h>
int main(int argc, char** argv) {
base::AtExitManager exit_manager;
- ::testing::InitGoogleTest(&argc, argv);
+ SetUpTests(&argc, argv, true);
return RUN_ALL_TESTS();
}
