Enable import/export of passwords into/from Password Manager

components/password_manager/core/browser/import/password_importer_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 "components/password_manager/core/browser/import/password_importer.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/test_simple_task_runner.h"
+#include "base/thread_task_runner_handle.h"
+#include "components/autofill/core/common/password_form.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace password_manager {
+
+namespace {
+const char kTestOriginURL[] = "http://accounts.google.com/a/LoginAuth";
+const char kTestSignonRealm[] = "http://accounts.google.com/";
+const char kTestUsername[] = "test@gmail.com";
+const char kTestPassword[] = "test1";
+const char kTestFileName[] = "test_only.csv";
+}  // namespace
+
+class PasswordImporterTest : public testing::Test {
+ public:
+  PasswordImporterTest()
+      : callback_called_(false), result_(PasswordImporter::SUCCESS) {}
+
+  void SetUp() override { ASSERT_TRUE(temp_directory_.CreateUniqueTempDir()); }

[vabr (Chromium), 2016/03/16 17:48:28]

Please add this to the constructor instead. SetUp is discouraged, according to
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/Initia...

[xunlu, 2016/03/18 21:15:30]

Seems that ASSERT_TRUE cannot be used in constructor as it can return void:

../../components/password_manager/core/browser/import/password_importer_unittest.cc:34:5:
error: constructor 'PasswordImporterTest' must not return void expression
    ASSERT_TRUE(temp_directory_.CreateUniqueTempDir());
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../testing/gtest/include/gtest/gtest.h:1871:23: note: expanded from macro
'ASSERT_TRUE'
                      GTEST_FATAL_FAILURE_)
                      ^~~~~~~~~~~~~~~~~~~~~
../../testing/gtest/include/gtest/internal/gtest-internal.h:1192:5: note:
expanded from macro 'GTEST_TEST_BOOLEAN_'
    fail(::testing::internal::GetBoolAssertionFailureMessage(\
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../testing/gtest/include/gtest/internal/gtest-internal.h:1110:3: note:
expanded from macro 'GTEST_FATAL_FAILURE_'
  return GTEST_MESSAGE_(message, ::testing::TestPartResult::kFatalFailure)
  ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[vabr (Chromium), 2016/03/21 14:01:30]

I think it is fine to change ASSERT_TRUE to CHECK here. Avoiding the SetUp in
favour of the constructor is more important.
While we need to think twice about CHECKs in production code (use restricted by
the style guide to only security reasons), here in the test it only matters
whether the resulting error message and behaviour are comparable. Both
ASSERT_TRUE and CHECK will indicate which assertion failed, and both will crash
the test, so there is not much difference.

[xunlu, 2016/03/22 07:17:15]

Sounds good. Thanks!

+
+ protected:
+  void StartImportAndWaitForCompletion(const base::FilePath& input_file) {
+    PasswordImporter::Import(input_file, message_loop_.task_runner(),
+                             base::Bind(&PasswordImporterTest::OnImportFinished,
+                                        base::Unretained(this)));
+
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
+
+    ASSERT_TRUE(callback_called_);
+  }
+
+  void OnImportFinished(PasswordImporter::Result result,
+                        const std::vector<autofill::PasswordForm>& passwords) {
+    callback_called_ = true;
+    result_ = result;
+    imported_passwords_ = passwords;
+  }
+
+  const PasswordImporter::Result& result() { return result_; }
+  const std::vector<autofill::PasswordForm>& imported_passwords() {
+    return imported_passwords_;
+  }
+
+  // Directory for creating files by this test.
+  base::ScopedTempDir temp_directory_;
+
+ private:
+  base::MessageLoop message_loop_;
+
+  bool callback_called_;
+  PasswordImporter::Result result_;
+  std::vector<autofill::PasswordForm> imported_passwords_;
+
+  DISALLOW_COPY_AND_ASSIGN(PasswordImporterTest);
+};
+
+TEST_F(PasswordImporterTest, CSVImport) {
+  const char kTestCSVInput[] =
+      "Url,Username,Password\n"
+      "http://accounts.google.com/a/LoginAuth,test@gmail.com,test1\n";
+
+  base::FilePath input_path = temp_directory_.path().AppendASCII(kTestFileName);
+  ASSERT_TRUE(
+      base::WriteFile(input_path, kTestCSVInput, strlen(kTestCSVInput)));
+  ASSERT_NO_FATAL_FAILURE(StartImportAndWaitForCompletion(input_path));
+
+  EXPECT_EQ(PasswordImporter::SUCCESS, result());
+  ASSERT_EQ(1u, imported_passwords().size());
+  EXPECT_EQ(GURL(kTestOriginURL), imported_passwords()[0].origin);
+  EXPECT_EQ(kTestSignonRealm, imported_passwords()[0].signon_realm);
+  EXPECT_EQ(base::ASCIIToUTF16(kTestUsername),
+            imported_passwords()[0].username_value);
+  EXPECT_EQ(base::ASCIIToUTF16(kTestPassword),
+            imported_passwords()[0].password_value);
+}
+
+TEST_F(PasswordImporterTest, ImportIOErrorDueToUnreadableFile) {
+  base::FilePath non_existent_input_file(FILE_PATH_LITERAL("nonexistent.csv"));
+  ASSERT_NO_FATAL_FAILURE(
+      StartImportAndWaitForCompletion(non_existent_input_file));
+
+  EXPECT_EQ(PasswordImporter::IO_ERROR, result());
+  ASSERT_EQ(0u, imported_passwords().size());
+}
+
+}  // namespace password_manager