Skip to content

Commit

Permalink
Replace MockPasswordManagerDriver with StubPasswordManagerDriver.
Browse files Browse the repository at this point in the history
Currently, there is a global MockPasswordManagerDriver, which mocks all
virtual methods of the PasswordManagerDriver interface. There is also
a locally (in a unit test) defined TestPasswordManagerDriver, which
inherits from the pure interface and provides stubs, a number of them
trivial.

That leads to code duplication and clutter.

This CL establishes the following structure:
It introduces StubPasswordManagerDriver, which is available to all
tests, and stubs all pure virtual methods of the driver interface.

Within the tests, mock or test drivers can inherit from the stub,
and only mock/modify methods interesting in that test.

See also http://crbug.com/352566#c2 for a description of this refactoring.

BUG=352566

Review URL: https://codereview.chromium.org/313113003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275104 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
vabr@chromium.org committed Jun 5, 2014
1 parent d3a7434 commit e437cb9
Show file tree
Hide file tree
Showing 12 changed files with 146 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_profile.h"
#include "components/autofill/core/common/password_form.h"
#include "components/password_manager/core/browser/mock_password_manager_driver.h"
#include "components/password_manager/core/browser/password_form_manager.h"
#include "components/password_manager/core/browser/password_manager_driver.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "components/password_manager/core/common/password_manager_ui.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "content/public/test/web_contents_tester.h"
Expand Down Expand Up @@ -81,7 +80,7 @@ TEST_F(ManagePasswordsUIControllerTest, PasswordAutofilled) {

TEST_F(ManagePasswordsUIControllerTest, PasswordSubmitted) {
password_manager::StubPasswordManagerClient client;
password_manager::MockPasswordManagerDriver driver;
password_manager::StubPasswordManagerDriver driver;
password_manager::PasswordFormManager* test_form_manager =
new password_manager::PasswordFormManager(
NULL, &client, &driver, test_form(), false);
Expand All @@ -106,7 +105,7 @@ TEST_F(ManagePasswordsUIControllerTest, PasswordSubmittedToNonWebbyURL) {
->NavigateAndCommit(GURL("chrome://sign-in"));

password_manager::StubPasswordManagerClient client;
password_manager::MockPasswordManagerDriver driver;
password_manager::StubPasswordManagerDriver driver;
password_manager::PasswordFormManager* test_form_manager =
new password_manager::PasswordFormManager(
NULL, &client, &driver, test_form(), false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/interactive_test_utils.h"
#include "components/autofill/core/common/password_form.h"
#include "components/password_manager/core/browser/mock_password_manager_driver.h"
#include "components/password_manager/core/browser/password_form_manager.h"
#include "components/password_manager/core/browser/password_manager_driver.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "testing/gtest/include/gtest/gtest.h"

void ManagePasswordsViewTest::SetUpOnMainThread() {
Expand Down Expand Up @@ -67,7 +66,7 @@ void ManagePasswordsViewTest::SetupManagingPasswords() {

void ManagePasswordsViewTest::SetupPendingPassword() {
password_manager::StubPasswordManagerClient client;
password_manager::MockPasswordManagerDriver driver;
password_manager::StubPasswordManagerDriver driver;
scoped_ptr<password_manager::PasswordFormManager> test_form_manager(
new password_manager::PasswordFormManager(
NULL, &client, &driver, *test_form(), false));
Expand Down
4 changes: 2 additions & 2 deletions components/password_manager.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,14 @@
'..',
],
'sources': [
'password_manager/core/browser/mock_password_manager_driver.cc',
'password_manager/core/browser/mock_password_manager_driver.h',
'password_manager/core/browser/mock_password_store.cc',
'password_manager/core/browser/mock_password_store.h',
'password_manager/core/browser/password_form_data.cc',
'password_manager/core/browser/password_form_data.h',
'password_manager/core/browser/stub_password_manager_client.cc',
'password_manager/core/browser/stub_password_manager_client.h',
'password_manager/core/browser/stub_password_manager_driver.cc',
'password_manager/core/browser/stub_password_manager_driver.h',
'password_manager/core/browser/test_password_store.cc',
'password_manager/core/browser/test_password_store.h',
],
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
#include "components/autofill/core/browser/test_autofill_driver.h"
#include "components/autofill/core/browser/test_autofill_manager_delegate.h"
#include "components/password_manager/core/browser/password_autofill_manager.h"
#include "components/password_manager/core/browser/password_manager_driver.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/rect_f.h"
Expand All @@ -33,25 +33,12 @@ namespace password_manager {

namespace {

// TODO(dubroy): Implement a TestPasswordManagerDriver that can be shared by
// all the tests that need it (crbug.com/352566).
class MockPasswordManagerDriver : public PasswordManagerDriver {
class MockPasswordManagerDriver : public StubPasswordManagerDriver {
public:
MOCK_METHOD1(FillPasswordForm, void(const autofill::PasswordFormFillData&));
MOCK_METHOD0(DidLastPageLoadEncounterSSLErrors, bool());
MOCK_METHOD0(IsOffTheRecord, bool());
MOCK_METHOD0(GetPasswordGenerationManager, PasswordGenerationManager*());
MOCK_METHOD0(GetPasswordManager, PasswordManager*());
MOCK_METHOD0(GetAutofillManager, autofill::AutofillManager*());
MOCK_METHOD1(AllowPasswordGenerationForForm, void(autofill::PasswordForm*));
MOCK_METHOD1(AccountCreationFormsFound,
void(const std::vector<autofill::FormData>&));
MOCK_METHOD2(FillSuggestion,
void(const base::string16&, const base::string16&));
MOCK_METHOD2(PreviewSuggestion,
void(const base::string16&, const base::string16&));
MOCK_METHOD0(ClearPreviewedForm, void());
MOCK_METHOD0(GetPasswordAutofillManager, PasswordAutofillManager*());
};

class TestPasswordManagerClient : public StubPasswordManagerClient {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "components/autofill/core/common/password_form.h"
#include "components/password_manager/core/browser/mock_password_manager_driver.h"
#include "components/password_manager/core/browser/mock_password_store.h"
#include "components/password_manager/core/browser/password_form_manager.h"
#include "components/password_manager/core/browser/password_manager.h"
#include "components/password_manager/core/browser/password_manager_driver.h"
#include "components/password_manager/core/browser/password_store.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "components/password_manager/core/browser/test_password_store.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -45,6 +45,12 @@ void RunAllPendingTasks() {
run_loop.Run();
}

class MockPasswordManagerDriver : public StubPasswordManagerDriver {
public:
MOCK_METHOD0(IsOffTheRecord, bool());
MOCK_METHOD1(AllowPasswordGenerationForForm, void(autofill::PasswordForm*));
};

class TestPasswordManagerClient : public StubPasswordManagerClient {
public:
explicit TestPasswordManagerClient(PasswordStore* password_store)
Expand Down Expand Up @@ -186,7 +192,7 @@ class PasswordFormManagerTest : public testing::Test {
TEST_F(PasswordFormManagerTest, TestNewLogin) {
scoped_ptr<TestPasswordManagerClient> client(
new TestPasswordManagerClient(NULL));
scoped_ptr<MockPasswordManagerDriver> driver;
scoped_ptr<StubPasswordManagerDriver> driver;
PasswordFormManager* manager = new PasswordFormManager(
NULL, client.get(), driver.get(), *observed_form(), false);

Expand Down Expand Up @@ -245,7 +251,7 @@ TEST_F(PasswordFormManagerTest, TestUpdatePassword) {
// saw this form and need to find matching logins.
scoped_ptr<TestPasswordManagerClient> client(
new TestPasswordManagerClient(NULL));
scoped_ptr<MockPasswordManagerDriver> driver;
scoped_ptr<StubPasswordManagerDriver> driver;
PasswordFormManager* manager = new PasswordFormManager(
NULL, client.get(), driver.get(), *observed_form(), false);

Expand Down Expand Up @@ -285,7 +291,7 @@ TEST_F(PasswordFormManagerTest, TestUpdatePassword) {
TEST_F(PasswordFormManagerTest, TestIgnoreResult) {
scoped_ptr<TestPasswordManagerClient> client(
new TestPasswordManagerClient(NULL));
scoped_ptr<MockPasswordManagerDriver> driver;
scoped_ptr<StubPasswordManagerDriver> driver;
PasswordFormManager* manager = new PasswordFormManager(
NULL, client.get(), driver.get(), *observed_form(), false);

Expand All @@ -307,7 +313,7 @@ TEST_F(PasswordFormManagerTest, TestIgnoreResult) {
TEST_F(PasswordFormManagerTest, TestEmptyAction) {
scoped_ptr<TestPasswordManagerClient> client(
new TestPasswordManagerClient(NULL));
scoped_ptr<MockPasswordManagerDriver> driver;
scoped_ptr<StubPasswordManagerDriver> driver;
scoped_ptr<PasswordFormManager> manager(new PasswordFormManager(
NULL, client.get(), driver.get(), *observed_form(), false));

Expand All @@ -329,7 +335,7 @@ TEST_F(PasswordFormManagerTest, TestEmptyAction) {
TEST_F(PasswordFormManagerTest, TestUpdateAction) {
scoped_ptr<TestPasswordManagerClient> client(
new TestPasswordManagerClient(NULL));
scoped_ptr<MockPasswordManagerDriver> driver;
scoped_ptr<StubPasswordManagerDriver> driver;
scoped_ptr<PasswordFormManager> manager(new PasswordFormManager(
NULL, client.get(), driver.get(), *observed_form(), false));

Expand All @@ -352,7 +358,7 @@ TEST_F(PasswordFormManagerTest, TestUpdateAction) {
TEST_F(PasswordFormManagerTest, TestDynamicAction) {
scoped_ptr<TestPasswordManagerClient> client(
new TestPasswordManagerClient(NULL));
scoped_ptr<MockPasswordManagerDriver> driver;
scoped_ptr<StubPasswordManagerDriver> driver;
scoped_ptr<PasswordFormManager> manager(new PasswordFormManager(
NULL, client.get(), driver.get(), *observed_form(), false));

Expand Down Expand Up @@ -604,7 +610,7 @@ TEST_F(PasswordFormManagerTest, TestForceInclusionOfGeneratedPasswords) {
TEST_F(PasswordFormManagerTest, TestSanitizePossibleUsernames) {
scoped_ptr<TestPasswordManagerClient> client(
new TestPasswordManagerClient(NULL));
scoped_ptr<MockPasswordManagerDriver> driver;
scoped_ptr<StubPasswordManagerDriver> driver;
scoped_ptr<PasswordFormManager> manager(new PasswordFormManager(
NULL, client.get(), driver.get(), *observed_form(), false));
PasswordForm credentials(*observed_form());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "components/password_manager/core/browser/password_generation_manager.h"
#include "components/password_manager/core/browser/password_manager.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
Expand All @@ -27,7 +28,7 @@ namespace password_manager {

namespace {

class TestPasswordManagerDriver : public PasswordManagerDriver {
class TestPasswordManagerDriver : public StubPasswordManagerDriver {
public:
TestPasswordManagerDriver(PasswordManagerClient* client)
: password_manager_(client),
Expand All @@ -37,34 +38,21 @@ class TestPasswordManagerDriver : public PasswordManagerDriver {
virtual ~TestPasswordManagerDriver() {}

// PasswordManagerDriver implementation.
virtual void FillPasswordForm(const autofill::PasswordFormFillData& form_data)
OVERRIDE {}
virtual bool DidLastPageLoadEncounterSSLErrors() OVERRIDE { return false; }
virtual bool IsOffTheRecord() OVERRIDE { return is_off_the_record_; }
virtual PasswordGenerationManager* GetPasswordGenerationManager() OVERRIDE {
return &password_generation_manager_;
}
virtual PasswordManager* GetPasswordManager() OVERRIDE {
return &password_manager_;
}
virtual autofill::AutofillManager* GetAutofillManager() OVERRIDE {
return NULL;
}
virtual PasswordAutofillManager* GetPasswordAutofillManager() OVERRIDE {
return &password_autofill_manager_;
}
virtual void AllowPasswordGenerationForForm(autofill::PasswordForm* form)
OVERRIDE {}
virtual void AccountCreationFormsFound(
const std::vector<autofill::FormData>& forms) OVERRIDE {
found_account_creation_forms_.insert(
found_account_creation_forms_.begin(), forms.begin(), forms.end());
}
virtual void FillSuggestion(const base::string16& username,
const base::string16& password) OVERRIDE {}
virtual void PreviewSuggestion(const base::string16& username,
const base::string16& password) OVERRIDE {}
virtual void ClearPreviewedForm() OVERRIDE {}

const std::vector<autofill::FormData>& GetFoundAccountCreationForms() {
return found_account_creation_forms_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

#include <vector>

#include "base/macros.h"
#include "base/strings/string16.h"

namespace autofill {
class AutofillManager;
struct FormData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
#include "base/prefs/testing_pref_service.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "components/password_manager/core/browser/mock_password_manager_driver.h"
#include "components/password_manager/core/browser/mock_password_store.h"
#include "components/password_manager/core/browser/password_autofill_manager.h"
#include "components/password_manager/core/browser/password_manager.h"
#include "components/password_manager/core/browser/password_manager_driver.h"
#include "components/password_manager/core/browser/password_store.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -48,6 +48,13 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
MOCK_METHOD0(GetDriver, PasswordManagerDriver*());
};

class MockPasswordManagerDriver : public StubPasswordManagerDriver {
public:
MOCK_METHOD1(FillPasswordForm, void(const autofill::PasswordFormFillData&));
MOCK_METHOD0(GetPasswordManager, PasswordManager*());
MOCK_METHOD0(GetPasswordAutofillManager, PasswordAutofillManager*());
};

ACTION_P(InvokeConsumer, forms) { arg0->OnGetPasswordStoreResults(forms); }

ACTION_P(SaveToScopedPtr, scoped) { scoped->reset(arg0); }
Expand Down Expand Up @@ -82,14 +89,8 @@ class PasswordManagerTest : public testing::Test {
password_autofill_manager_.reset(
new PasswordAutofillManager(&client_, NULL));

EXPECT_CALL(driver_, DidLastPageLoadEncounterSSLErrors())
.WillRepeatedly(Return(false));
EXPECT_CALL(driver_, IsOffTheRecord()).WillRepeatedly(Return(false));
EXPECT_CALL(driver_, GetPasswordGenerationManager())
.WillRepeatedly(Return(static_cast<PasswordGenerationManager*>(NULL)));
EXPECT_CALL(driver_, GetPasswordManager())
.WillRepeatedly(Return(manager_.get()));
EXPECT_CALL(driver_, AllowPasswordGenerationForForm(_)).Times(AnyNumber());
EXPECT_CALL(driver_, GetPasswordAutofillManager())
.WillRepeatedly(Return(password_autofill_manager_.get()));
}
Expand Down
Loading

0 comments on commit e437cb9

Please sign in to comment.