Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(90)

Unified Diff: dbus/signal_sender_verification_unittest.cc

Issue 11358111: Make SignalSenderVerificationTest more robust (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: address comments Created 8 years, 1 month 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
Download patch
Index: dbus/signal_sender_verification_unittest.cc
diff --git a/dbus/signal_sender_verification_unittest.cc b/dbus/signal_sender_verification_unittest.cc
index 91db93fb421157083e004739baeab124979a0ec1..582cd54084c100ce7468a9a23cd8fddb36f69646 100644
--- a/dbus/signal_sender_verification_unittest.cc
+++ b/dbus/signal_sender_verification_unittest.cc
@@ -42,6 +42,7 @@ class SignalSenderVerificationTest : public testing::Test {
ASSERT_TRUE(test_service_->StartService());
ASSERT_TRUE(test_service_->WaitUntilServiceIsStarted());
ASSERT_TRUE(test_service_->HasDBusThread());
+ ASSERT_TRUE(test_service_->has_ownership());
// Same setup for the second TestService. This service should not have the
// ownership of the name at this point.
@@ -49,6 +50,7 @@ class SignalSenderVerificationTest : public testing::Test {
ASSERT_TRUE(test_service2_->StartService());
ASSERT_TRUE(test_service2_->WaitUntilServiceIsStarted());
ASSERT_TRUE(test_service2_->HasDBusThread());
+ ASSERT_FALSE(test_service2_->has_ownership());
// Create the client, using the D-Bus thread.
dbus::Bus::Options bus_options;
@@ -62,6 +64,10 @@ class SignalSenderVerificationTest : public testing::Test {
dbus::ObjectPath("/org/chromium/TestObject"));
ASSERT_TRUE(bus_->HasDBusThread());
+ object_proxy_->SetNameOwnerChangedCallback(
+ base::Bind(&SignalSenderVerificationTest::OnNameOwnerChanged,
+ base::Unretained(this)));
+
// Connect to the "Test" signal of "org.chromium.TestInterface" from
// the remote object.
object_proxy_->ConnectToSignal(
@@ -75,6 +81,24 @@ class SignalSenderVerificationTest : public testing::Test {
message_loop_.Run();
}
+ void OnOwnership(bool expected, bool success) {
+ ASSERT_EQ(expected, success);
+ }
+
+ void OnNameOwnerChanged(dbus::Signal* signal) {
+ dbus::MessageReader reader(signal);
+ std::string name, old_owner, new_owner;
+ ASSERT_TRUE(reader.PopString(&name));
+ ASSERT_TRUE(reader.PopString(&old_owner));
+ ASSERT_TRUE(reader.PopString(&new_owner));
+ latest_name_owner_ = new_owner;
+ // PostTask to quit the mMessageLoop as this is called from D-Bus thread.
satorux1 2012/11/13 06:32:03 mMessageLoop -> MessageLoop Thank you for adding
Haruki Sato 2012/11/14 06:35:15 This part is changed. Thank you for the review!
+ message_loop_.PostTask(
+ FROM_HERE,
+ base::Bind(&SignalSenderVerificationTest::QuitMessageLoop,
+ base::Unretained(this)));
+ }
+
virtual void TearDown() {
bus_->ShutdownOnDBusThreadAndBlock();
@@ -115,6 +139,10 @@ class SignalSenderVerificationTest : public testing::Test {
message_loop_.Run();
}
+ void QuitMessageLoop() {
+ message_loop_.Quit();
+ }
+
MessageLoop message_loop_;
scoped_ptr<base::Thread> dbus_thread_;
scoped_refptr<dbus::Bus> bus_;
@@ -123,6 +151,9 @@ class SignalSenderVerificationTest : public testing::Test {
scoped_ptr<dbus::TestService> test_service2_;
// Text message from "Test" signal.
std::string test_signal_string_;
+
+ // The known latest name owner of TestService. Updated in OnNameOwnerChanged.
+ std::string latest_name_owner_;
};
TEST_F(SignalSenderVerificationTest, TestSignalAccepted) {
@@ -157,7 +188,7 @@ TEST_F(SignalSenderVerificationTest, TestSignalRejected) {
EXPECT_EQ(samples1->TotalCount() + 1, samples2->TotalCount());
}
-TEST_F(SignalSenderVerificationTest, DISABLED_TestOwnerChanged) {
+TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
const char kMessage[] = "hello, world";
// Send the test signal from the exported object.
@@ -168,8 +199,20 @@ TEST_F(SignalSenderVerificationTest, DISABLED_TestOwnerChanged) {
ASSERT_EQ(kMessage, test_signal_string_);
// Release and aquire the name ownership.
+ latest_name_owner_ = "dummyowner";
satorux1 2012/11/13 06:32:03 this looks rather unclean. At this moment, test_se
Haruki Sato 2012/11/14 06:35:15 Done.
test_service_->ShutdownAndBlock();
- test_service2_->RequestOwnership();
+ // OnNameOwnerChanged will PostTask to quit the message loop.
+ message_loop_.Run();
+ // latest_name_owner_ should be empty as the owner is gone.
+ ASSERT_TRUE(latest_name_owner_.empty());
+
+ test_service2_->RequestOwnership(
+ base::Bind(&SignalSenderVerificationTest::OnOwnership,
+ base::Unretained(this), true));
+ // OnNameOwnerChanged will PostTask to quit the message loop.
satorux1 2012/11/13 06:32:03 OnNameOwnerChanged() Let's add () for function na
Haruki Sato 2012/11/14 06:35:15 Done.
+ message_loop_.Run();
satorux1 2012/11/13 06:32:03 What about calling this twice: message_loop_.Run(
Haruki Sato 2012/11/14 06:35:15 As discussed offline, I added more checks for call
+ // latest_name_owner_ becomes non empty as the new owner appears.
+ ASSERT_FALSE(latest_name_owner_.empty());
// Now the second service owns the name.
const char kNewMessage[] = "hello, new world";
« dbus/object_proxy.cc ('K') | « dbus/object_proxy.cc ('k') | dbus/test_service.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698