Remove D-Bus dependency from Service
Here are the main changes:
1. Update service to use ServiceAdaptorInterface instead of using
D-Bus adaptor directly.
2. Update D-Bus object registration for Service and Config to be
synchronous, to remove unnecessary complexity. This also
eliminates the need for Manager::CreateService handler to be
asynchronous.
3. Update Service APIs to use the internal Error instead of brillo::Error.
4. Use MockServiceAdaptor for testing, fake version is not needed
since the adaptor properties for Service are not being used in test.
Bug: 24194427
TEST=Start AP service on both Brillo and Chrome OS devices
TEST=Run unittests on both Brillo and Chrome OS
Change-Id: Ib94a4b91ef402415470e0131afdeeef5105817e4
diff --git a/service_unittest.cc b/service_unittest.cc
index 2f571ab..a23e06c 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -27,7 +27,7 @@
#if !defined(__ANDROID__)
#include <chromeos/dbus/service_constants.h>
#else
-#include "dbus/apmanager/dbus-constants.h"
+#include <dbus/apmanager/dbus-constants.h>
#endif // __ANDROID__
#include "apmanager/error.h"
@@ -40,6 +40,7 @@
#include "apmanager/mock_hostapd_monitor.h"
#include "apmanager/mock_manager.h"
#include "apmanager/mock_process_factory.h"
+#include "apmanager/mock_service_adaptor.h"
using brillo::ProcessMock;
using ::testing::_;
@@ -69,6 +70,8 @@
ServiceTest()
: manager_(&control_interface_),
hostapd_monitor_(new MockHostapdMonitor()) {
+ ON_CALL(control_interface_, CreateServiceAdaptorRaw())
+ .WillByDefault(ReturnNew<MockServiceAdaptor>());
ON_CALL(control_interface_, CreateConfigAdaptorRaw())
.WillByDefault(ReturnNew<FakeConfigAdaptor>());
// Defer creation of Service object to allow ControlInterface to
@@ -82,7 +85,7 @@
service_->hostapd_monitor_.reset(hostapd_monitor_);
}
- bool StartService(brillo::ErrorPtr* error) {
+ bool StartService(Error* error) {
return service_->StartInternal(error);
}
@@ -98,6 +101,14 @@
service_->config_.reset(config);
}
+ void VerifyError(const Error& error,
+ Error::Type expected_type,
+ const std::string& expected_message_start) {
+ EXPECT_EQ(expected_type, error.type());
+ EXPECT_TRUE(
+ base::StartsWithASCII(error.message(), expected_message_start, false));
+ }
+
protected:
MockControl control_interface_;
MockManager manager_;
@@ -108,26 +119,19 @@
std::unique_ptr<Service> service_;
};
-MATCHER_P(IsServiceErrorStartingWith, message, "") {
- return arg != nullptr &&
- arg->GetDomain() == brillo::errors::dbus::kDomain &&
- arg->GetCode() == kServiceError &&
- base::EndsWith(arg->GetMessage(), message, false);
-}
-
TEST_F(ServiceTest, StartWhenServiceAlreadyRunning) {
StartDummyProcess();
- brillo::ErrorPtr error;
+ Error error;
EXPECT_FALSE(StartService(&error));
- EXPECT_THAT(error, IsServiceErrorStartingWith("Service already running"));
+ VerifyError(error, Error::kInternalError, "Service already running");
}
TEST_F(ServiceTest, StartWhenConfigFileFailed) {
MockConfig* config = new MockConfig(&manager_);
SetConfig(config);
- brillo::ErrorPtr error;
+ Error error;
EXPECT_CALL(*config, GenerateConfigFile(_, _)).WillOnce(Return(false));
EXPECT_FALSE(StartService(&error));
}
@@ -142,7 +146,7 @@
ProcessMock* process = new ProcessMock();
std::string config_str(kHostapdConfig);
- brillo::ErrorPtr error;
+ Error error;
EXPECT_CALL(*config, GenerateConfigFile(_, _)).WillOnce(
DoAll(SetArgPointee<1>(config_str), Return(true)));
EXPECT_CALL(file_writer_, Write(kHostapdConfigFilePath, kHostapdConfig))
@@ -156,14 +160,14 @@
EXPECT_CALL(manager_, RequestDHCPPortAccess(_));
EXPECT_CALL(*hostapd_monitor_, Start());
EXPECT_TRUE(StartService(&error));
- EXPECT_EQ(nullptr, error);
+ EXPECT_TRUE(error.IsSuccess());
}
TEST_F(ServiceTest, StopWhenServiceNotRunning) {
- brillo::ErrorPtr error;
+ Error error;
EXPECT_FALSE(service_->Stop(&error));
- EXPECT_THAT(error, IsServiceErrorStartingWith(
- "Service is not currently running"));
+ VerifyError(
+ error, Error::kInternalError, "Service is not currently running");
}
TEST_F(ServiceTest, StopSuccess) {
@@ -171,7 +175,7 @@
MockConfig* config = new MockConfig(&manager_);
SetConfig(config);
- brillo::ErrorPtr error;
+ Error error;
EXPECT_CALL(*config, ReleaseDevice()).Times(1);
EXPECT_TRUE(service_->Stop(&error));
Mock::VerifyAndClearExpectations(config);