Refactor native_backend_kwallet_x

chrome/browser/password_manager/kwallet_dbus_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/password_manager/kwallet_dbus.h"
+
+#include <memory>
+#include <string>
+
+#include "base/logging.h"
+#include "base/nix/xdg_util.h"
+#include "dbus/message.h"
+#include "dbus/mock_bus.h"
+#include "dbus/mock_object_proxy.h"
+#include "dbus/object_path.h"
+#include "dbus/object_proxy.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+using testing::_;
+using testing::Return;
+using testing::Invoke;
+using testing::DoAll;
+using testing::ElementsAreArray;
+
+const char kKWalletInterface[] = "org.kde.KWallet";
+const char kKLauncherInterface[] = "org.kde.KLauncher";
+
+dbus::Response* RespondBool(bool value) {
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendBool(value);
+  return response.release();
+}
+
+dbus::Response* RespondString(const std::string& value) {
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendString(value);
+  return response.release();
+}
+
+dbus::Response* RespondBytes(const uint8_t* bytes, int count) {
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendArrayOfBytes(bytes, count);
+  return response.release();
+}
+
+dbus::Response* RespondArrayOfStrings(std::vector<std::string> strings) {

[vasilii, 2016/06/16 17:54:43]

const std::vector<std::string>&

[cfroussios, 2016/06/17 12:17:26]

Done.

+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendArrayOfStrings(strings);
+  return response.release();
+}
+
+dbus::Response* RespondInt32(int value) {
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendInt32(value);
+  return response.release();
+}
+
+dbus::Response* RespondEmpty() {
+  return dbus::Response::CreateEmpty().release();
+}
+
+class KWalletDBusTest
+    : public testing::TestWithParam<base::nix::DesktopEnvironment> {
+ public:
+  KWalletDBusTest() : desktop_env_(GetParam()), kwallet_dbus_(desktop_env_) {
+    if (desktop_env_ == base::nix::DESKTOP_ENVIRONMENT_KDE5) {
+      dbus_service_name_ = "org.kde.kwalletd5";
+      dbus_path_ = "/modules/kwalletd5";
+      kwalletd_name_ = "kwalletd5";
+    } else {
+      dbus_service_name_ = "org.kde.kwalletd";
+      dbus_path_ = "/modules/kwalletd";
+      kwalletd_name_ = "kwalletd";
+    }
+  }
+
+  void SetUp() override {

[vasilii, 2016/06/16 17:54:44]

Why not doing everything in the constructor?

[cfroussios, 2016/06/17 12:17:27]

Done.

+    dbus::Bus::Options options;
+    options.bus_type = dbus::Bus::SESSION;
+    mock_session_bus_ = new dbus::MockBus(options);
+
+    mock_klauncher_proxy_ =
+        new dbus::MockObjectProxy(mock_session_bus_.get(), "org.kde.klauncher",
+                                  dbus::ObjectPath("/KLauncher"));
+    mock_kwallet_proxy_ =
+        new dbus::MockObjectProxy(mock_session_bus_.get(), dbus_service_name_,
+                                  dbus::ObjectPath(dbus_path_));
+
+    // The kwallet proxy is aquired once, when preparing |kwallet_dbus_|
+    EXPECT_CALL(
+        *mock_session_bus_.get(),
+        GetObjectProxy(dbus_service_name_, dbus::ObjectPath(dbus_path_)))
+        .WillOnce(Return(mock_kwallet_proxy_.get()));
+
+    // The klauncher proxy is aquired every time StartKWalletd() is tested

[vasilii, 2016/06/16 17:54:43]

It's tested only once. Therefore, the EXPECT_CALL should go there. Instead of
WillRepeatedly it becomes WillOnce.

[cfroussios, 2016/06/17 12:17:26]

Done.

+    EXPECT_CALL(

[vasilii, 2016/06/16 17:54:43]

EXPECT_CALLs should be just above the method which triggers them. That one isn't
called in SetUp().
Ideally you call testing::Mock::VerifyAndClearExpectations() in the end of
SetUp(). Otherwise, if I create an empty test it will fail, that's strange.

[cfroussios, 2016/06/17 12:17:26]

Done.

+        *mock_session_bus_.get(),
+        GetObjectProxy("org.kde.klauncher", dbus::ObjectPath("/KLauncher")))
+        .WillRepeatedly(Return(mock_klauncher_proxy_.get()));
+
+    kwallet_dbus_.SetSessionBus(mock_session_bus_);
+
+    // No unexpected calls.
+    EXPECT_CALL(*mock_kwallet_proxy_.get(), MockCallMethodAndBlock(_, _))

[vasilii, 2016/06/16 17:54:44]

This is achieved by instantiating StrictMock<dbus::MockObjectProxy> instead of
dbus::MockObjectProxy.

[cfroussios, 2016/06/17 12:17:27]

I cannot instantiate StrictMock<dbus::MockObjectProxy>, because MockObjectProxy
doesn't have a public destructor.

[vasilii, 2016/06/17 14:43:41]

It's protected. What is the problem?

scoped_refptr<dbus::MockObjectProxy> mock_klauncher_proxy = new
StrictMock<MockObjectProxy>(...);

[cfroussios, 2016/06/20 13:00:43]

Done.

+        .Times(0);
+  }
+
+  virtual ~KWalletDBusTest() = default;

[vasilii, 2016/06/16 17:54:43]

That's probably unnecessary.

[cfroussios, 2016/06/17 12:17:27]

Done.

+
+ protected:
+  base::nix::DesktopEnvironment desktop_env_;

[vasilii, 2016/06/16 17:54:43]

const?

[cfroussios, 2016/06/17 12:17:27]

Done.

+  scoped_refptr<dbus::MockBus> mock_session_bus_;
+  scoped_refptr<dbus::MockObjectProxy> mock_klauncher_proxy_;
+  scoped_refptr<dbus::MockObjectProxy> mock_kwallet_proxy_;
+  KWalletDBus kwallet_dbus_;
+
+  std::string dbus_service_name_;
+  std::string dbus_path_;
+  std::string kwalletd_name_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(KWalletDBusTest);
+};
+
+INSTANTIATE_TEST_CASE_P(,
+                        KWalletDBusTest,
+                        ::testing::Values(base::nix::DESKTOP_ENVIRONMENT_KDE4,
+                                          base::nix::DESKTOP_ENVIRONMENT_KDE5));
+
+// Matches a method call to the specified dbus target.
+MATCHER_P2(Calls, interface, member, "") {
+  return arg->GetMember() == member && arg->GetInterface() == interface;
+}
+
+// Pops items from the dbus message and compares them to the expected values.
+ACTION_P3(PopIntStringString, int_1, str_2, str_3) {

[vasilii, 2016/06/16 17:54:43]

This is not an action. This is a matcher because you test some properties of the
argument.

MATCHER_P(MessageHasInt, int_val, , "") {
  reader.PopInt32(&i);
  return int_val == i;
}

MATCHER_P(MessageHasString, str_val, , "") {
  reader.PopString(&str);
  return str == str_val;
}

In the test:

EXPECT_CALL(*mock_kwallet_proxy_.get(),
    MockCallMethodAndBlock(AllOf(
        Calls(kKWalletInterface, "hasEntry"),
        MessageHasInt(7),
        MessageHasString("asdf"),
        MessageHasString("vbnm")), _))
   .WillOnce(Return(RespondBool(true)));

[cfroussios, 2016/06/17 12:17:26]

I like the style of composing the expectation one argument at a time. I couldn't
make it work without making things even more complicated, because they have to
share a reader, which has to be created during/after the call.

Also, in theory, a matcher should not have side effects, such as advancing the
read pointer. In general, if a matcher fails, you try matching against the next
pattern until you find something. I do not know if gmock would actually do that,
but that's how I understand the concept of matchers. Actions, on the other hand,
can modify state.

[vasilii, 2016/06/17 14:43:40]

Good point regarding the reader. This should work:

EXPECT_CALL(*mock_kwallet_proxy_.get(),
    MockCallMethodAndBlock(AllOf(
        Calls(kKWalletInterface, "hasEntry"),
        ArgumentsAre(7, "asdf", "vbnm")), _))
   .WillOnce(Return(RespondBool(true)));

[cfroussios, 2016/06/20 13:00:43]

Done.

+  dbus::MessageReader reader(arg0);
+
+  int i;
+  EXPECT_TRUE(reader.PopInt32(&i));
+  EXPECT_EQ(int_1, i);
+
+  std::string str;
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_2, str);
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_3, str);
+}
+
+// Pops items from the dbus message and compares them to the expected values.
+ACTION_P4(PopIntStringStringString, int_1, str_2, str_3, str_4) {
+  dbus::MessageReader reader(arg0);
+
+  int i;
+  EXPECT_TRUE(reader.PopInt32(&i));
+  EXPECT_EQ(int_1, i);
+  std::string str;
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_2, str);
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_3, str);
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_4, str);
+}
+
+// Pops items from the dbus message and compares them to the expected values.
+ACTION_P5(PopIntStringStringVectorString, int_1, str_2, str_3, vec_4, str_5) {
+  dbus::MessageReader reader(arg0);
+
+  int i;
+  EXPECT_TRUE(reader.PopInt32(&i));
+  EXPECT_EQ(int_1, i);
+
+  std::string str;
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_2, str);
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_3, str);
+
+  const uint8_t* bytes = nullptr;
+  size_t length = 0;
+  EXPECT_TRUE(reader.PopArrayOfBytes(&bytes, &length));
+  EXPECT_EQ(vec_4.size(), length);
+  EXPECT_THAT(vec_4, ElementsAreArray(bytes, length));
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_5, str);
+}
+
+// Pops items from the dbus message and compares them to the expected values.
+ACTION_P3(PopStringInt64String, str_1, int_2, str_3) {
+  dbus::MessageReader reader(arg0);
+
+  std::string str;
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_1, str);
+
+  int64_t i;
+  EXPECT_TRUE(reader.PopInt64(&i));
+  EXPECT_EQ(int_2, i);
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_3, str);
+}
+
+// Pops items from the dbus message and compares them to the expected values.
+ACTION_P5(PopStringStringsStringsStringBool,
+          str_1,
+          vec_2,
+          vec_3,
+          str_4,
+          bool_5) {
+  dbus::MessageReader reader(arg0);
+
+  std::string str;
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_1, str);
+
+  std::vector<std::string> strings;
+  EXPECT_TRUE(reader.PopArrayOfStrings(&strings));
+  EXPECT_EQ(vec_2, strings);
+
+  EXPECT_TRUE(reader.PopArrayOfStrings(&strings));
+  EXPECT_EQ(vec_3, strings);
+
+  EXPECT_TRUE(reader.PopString(&str));
+  EXPECT_EQ(str_4, str);
+
+  bool b;
+  EXPECT_TRUE(reader.PopBool(&b));
+  EXPECT_EQ(bool_5, b);
+}
+
+TEST_P(KWalletDBusTest, StartWalletd) {
+  // The receiver of the message takes ownership of the response object.
+  dbus::Response* response_success = dbus::Response::CreateEmpty().release();
+  dbus::MessageWriter writer(response_success);
+  writer.AppendInt32(0);  // return code
+  writer.AppendString("dbus_name");
+  writer.AppendString(std::string());  // error message
+  writer.AppendInt32(100);             // pid
+
+  std::vector<std::string> empty;
+
+  EXPECT_CALL(
+      *mock_klauncher_proxy_.get(),
+      MockCallMethodAndBlock(
+          Calls(kKLauncherInterface, "start_service_by_desktop_name"), _))
+      .WillOnce(DoAll(PopStringStringsStringsStringBool(
+                          kwalletd_name_, empty, empty, std::string(), false),
+                      Return(response_success)))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));

[vasilii, 2016/06/16 17:54:44]

Here and below. Those WillOnce are extremely confusing. It should be

EXPECT_CALL().WillOnce(Return(somethins));
EXPECT_TRUE(kwallet_dbus_.StartKWalletd());

EXPECT_CALL().WillOnce(Return(something_else));
EXPECT_FALSE(kwallet_dbus_.StartKWalletd());

Ideally, they are even in the different tests.

[cfroussios, 2016/06/17 12:17:27]

According to
https://github.com/google/googletest/blob/master/googlemock/docs/ForDummies.md
"you mustn't interleave EXPECT_CALL()s and calls to the mock functions"

The documentation for gmock is full of examples about preparing for multiple
calls, and then performing those calls. It shouldn't be confusing. 

I've moved checking the error codes to separate tests, if that helps.

[vasilii, 2016/06/17 14:43:40]

That's why I mentioned different tests (or VerifyAndClearExpectation). Most of
the committers will agree that 

EXPECT_TRUE(kwallet_dbus_.StartKWalletd());
EXPECT_FALSE(kwallet_dbus_.StartKWalletd());
EXPECT_FALSE(kwallet_dbus_.StartKWalletd());

is weird.

[cfroussios, 2016/06/20 13:00:43]

Done.

+
+  EXPECT_TRUE(kwallet_dbus_.StartKWalletd());
+  EXPECT_FALSE(kwallet_dbus_.StartKWalletd());
+  EXPECT_FALSE(kwallet_dbus_.StartKWalletd());
+}
+
+TEST_P(KWalletDBusTest, IsEnabled) {
+  EXPECT_CALL(*mock_kwallet_proxy_.get(),
+              MockCallMethodAndBlock(Calls(kKWalletInterface, "isEnabled"), _))
+      .WillOnce(Return(RespondBool(false)))
+      .WillOnce(Return(RespondBool(true)))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  bool is_enabled = true;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS, kwallet_dbus_.IsEnabled(&is_enabled));
+  EXPECT_FALSE(is_enabled);
+
+  is_enabled = false;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS, kwallet_dbus_.IsEnabled(&is_enabled));
+  EXPECT_TRUE(is_enabled);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.IsEnabled(&is_enabled));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.IsEnabled(&is_enabled));
+}
+
+TEST_P(KWalletDBusTest, NetworkWallet) {
+  EXPECT_CALL(
+      *mock_kwallet_proxy_.get(),
+      MockCallMethodAndBlock(Calls(kKWalletInterface, "networkWallet"), _))
+      .WillOnce(Return(RespondString("mock_wallet")))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  std::string wallet;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS, kwallet_dbus_.NetworkWallet(&wallet));
+  EXPECT_EQ("mock_wallet", wallet);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.NetworkWallet(&wallet));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.NetworkWallet(&wallet));
+}
+
+TEST_P(KWalletDBusTest, HasEntry) {
+  EXPECT_CALL(*mock_kwallet_proxy_.get(),
+              MockCallMethodAndBlock(Calls(kKWalletInterface, "hasEntry"), _))
+      .WillOnce(DoAll(PopIntStringStringString(123, "folder", "realm", "app"),
+                      Return(RespondBool(true))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  bool has_entry = false;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.HasEntry(123, "folder", "realm", "app", &has_entry));
+  EXPECT_TRUE(has_entry);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.HasEntry(123, "folder", "realm", "app", &has_entry));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.HasEntry(123, "folder", "realm", "app", &has_entry));
+}
+
+TEST_P(KWalletDBusTest, ReadEntry) {
+  std::vector<uint8_t> bytes_expected = {1, 2, 1, 2};
+  EXPECT_CALL(*mock_kwallet_proxy_.get(),
+              MockCallMethodAndBlock(Calls(kKWalletInterface, "readEntry"), _))
+      .WillOnce(DoAll(
+          PopIntStringStringString(123, "folder", "realm", "app"),
+          Return(RespondBytes(bytes_expected.data(), bytes_expected.size()))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  std::vector<uint8_t> bytes;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.ReadEntry(123, "folder", "realm", "app", &bytes));
+  EXPECT_EQ(bytes_expected, bytes);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.ReadEntry(123, "folder", "realm", "app", &bytes));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.ReadEntry(123, "folder", "realm", "app", &bytes));
+}
+
+TEST_P(KWalletDBusTest, EntryList) {
+  std::vector<std::string> strings_expected = {"one", "two"};
+  EXPECT_CALL(*mock_kwallet_proxy_.get(),
+              MockCallMethodAndBlock(Calls(kKWalletInterface, "entryList"), _))
+      .WillOnce(DoAll(PopIntStringString(123, "folder", "app"),
+                      Return(RespondArrayOfStrings(strings_expected))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  std::vector<std::string> strings;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.EntryList(123, "folder", "app", &strings));
+  EXPECT_EQ(strings_expected, strings);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.EntryList(123, "folder", "app", &strings));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.EntryList(123, "folder", "app", &strings));
+}
+
+TEST_P(KWalletDBusTest, RemoveEntry) {
+  EXPECT_CALL(
+      *mock_kwallet_proxy_.get(),
+      MockCallMethodAndBlock(Calls(kKWalletInterface, "removeEntry"), _))
+      .WillOnce(DoAll(PopIntStringStringString(123, "folder", "realm", "app"),
+                      Return(RespondInt32(0))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  int ret;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.RemoveEntry(123, "folder", "realm", "app", &ret));
+  EXPECT_EQ(0, ret);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.RemoveEntry(123, "folder", "realm", "app", &ret));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.RemoveEntry(123, "folder", "realm", "app", &ret));
+}
+
+TEST_P(KWalletDBusTest, WriteEntry) {
+  std::vector<uint8_t> bytes = {1, 2, 3, 1};
+  EXPECT_CALL(*mock_kwallet_proxy_.get(),
+              MockCallMethodAndBlock(Calls(kKWalletInterface, "writeEntry"), _))
+      .WillOnce(DoAll(
+          PopIntStringStringVectorString(123, "folder", "realm", bytes, "app"),
+          Return(RespondInt32(0))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  int ret;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.WriteEntry(123, "folder", "realm", "app",
+                                     bytes.data(), bytes.size(), &ret));
+  EXPECT_EQ(0, ret);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.WriteEntry(123, "folder", "realm", "app",
+                                     bytes.data(), bytes.size(), &ret));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.WriteEntry(123, "folder", "realm", "app",
+                                     bytes.data(), bytes.size(), &ret));
+}
+
+TEST_P(KWalletDBusTest, Open) {
+  EXPECT_CALL(*mock_kwallet_proxy_.get(),
+              MockCallMethodAndBlock(Calls(kKWalletInterface, "open"), _))
+      .WillOnce(DoAll(PopStringInt64String("wallet", 0, "app"),
+                      Return(RespondInt32(1234))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  int ret;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.Open("wallet", "app", &ret));
+  EXPECT_EQ(1234, ret);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.Open("wallet", "app", &ret));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.Open("wallet", "app", &ret));
+}
+
+TEST_P(KWalletDBusTest, HasFolder) {
+  EXPECT_CALL(*mock_kwallet_proxy_.get(),
+              MockCallMethodAndBlock(Calls(kKWalletInterface, "hasFolder"), _))
+      .WillOnce(DoAll(PopIntStringString(123, "wallet", "app"),
+                      Return(RespondBool(true))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  bool has_folder = false;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.HasFolder(123, "wallet", "app", &has_folder));
+  EXPECT_EQ(true, has_folder);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.HasFolder(123, "wallet", "app", &has_folder));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.HasFolder(123, "wallet", "app", &has_folder));
+}
+
+TEST_P(KWalletDBusTest, CreateFolder) {
+  EXPECT_CALL(
+      *mock_kwallet_proxy_.get(),
+      MockCallMethodAndBlock(Calls(kKWalletInterface, "createFolder"), _))
+      .WillOnce(DoAll(PopIntStringString(123, "folder", "app"),
+                      Return(RespondBool(true))))
+      .WillOnce(Return(RespondEmpty()))
+      .WillOnce(Return(nullptr));
+
+  bool created_folder = false;
+  EXPECT_EQ(KWalletDBus::Error::SUCCESS,
+            kwallet_dbus_.CreateFolder(123, "folder", "app", &created_folder));
+  EXPECT_EQ(true, created_folder);
+
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_READ,
+            kwallet_dbus_.CreateFolder(123, "folder", "app", &created_folder));
+  EXPECT_EQ(KWalletDBus::Error::CANNOT_CONTACT,
+            kwallet_dbus_.CreateFolder(123, "folder", "app", &created_folder));
+}
+
+}  // namespace