Parameterize FileManagerBrowserTransferTest.

chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc

 // Copyright (c) 2013 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.
 
 // Browser test for basic Chrome OS file manager functionality:
 //  - The file list is updated when a file is added externally to the Downloads
 //    folder.
 //  - Selecting a file and copy-pasting it with the keyboard copies the file.
 //  - Selecting a file and pressing delete deletes it.
 
@@ skipping 29 matching lines @@
 enum EntryType {
   FILE,
   DIRECTORY,
 };
 
 enum SharedOption {
   NONE,
   SHARED,
 };
 
+enum GestModeOption {

[hashimoto, 2013/05/31 09:16:21]

s/Gest/Guest/?

[hirono, 2013/06/03 01:45:53]

Done.

+  NOT_IN_GEST_MODE,
+  IN_GEST_MODE
+};
+
+std::ostream& operator<<(std::ostream& os, const GestModeOption& option) {

[hashimoto, 2013/05/31 09:16:21]

Please add a comment to describe why we need this.

[hirono, 2013/06/03 01:45:53]

Done.

+  return os << (option == NOT_IN_GEST_MODE ?
+                "Not in guest mode" : "In gest mode");
+}
+
 struct TestEntryInfo {
   EntryType type;
   const char* source_file_name;  // Source file name to be used as a prototype.
   const char* target_name;  // Target file or directory name.
   const char* mime_type;
   SharedOption shared_option;
   const char* last_modified_time_as_string;
 };
 
 TestEntryInfo kTestEntrySetCommon[] = {
@@ skipping 247 matching lines @@
   }
 
  private:
   base::ScopedTempDir test_cache_root_;
   google_apis::FakeDriveService* fake_drive_service_;
   drive::DriveIntegrationService* integration_service_;
 };
 
 // The base test class. Used by FileManagerBrowserLocalTest,
 // FileManagerBrowserDriveTest, and FileManagerBrowserTransferTest.
-// The boolean parameter, retrieved by GetParam(), is true if testing in the
-// guest mode. See SetUpCommandLine() below for details.
-class FileManagerBrowserTestBase : public ExtensionApiTest,
-                                   public ::testing::WithParamInterface<bool> {
+class FileManagerBrowserTestBase : public ExtensionApiTest {
  protected:
+  // gest_mode is true if testing in the guest mode. See SetUpCommandLine()

[hashimoto, 2013/05/31 09:16:21]

nit: I think this comment is useless or at least wrong. (the argument is not
boolean)

[hirono, 2013/06/03 01:45:53]

Done.

+  // below for details.
+  explicit FileManagerBrowserTestBase(GestModeOption gest_mode) :
+      gest_mode_(gest_mode_) {}
+
   // Adds an incognito and guest-mode flags for tests in the guest mode.
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE;
 
   // Loads our testing extension and sends it a string identifying the current
   // test.
   void StartTest(const std::string& test_name);
 
   // Creates test files and directories.
   void CreateTestEntries(TestVolume* volume, const TestEntryInfo* entries,
                          size_t num_entries);
 
   // Runs the file display test on the passed |volume|, shared by subclasses.
   void DoTestFileDisplay(TestVolume* volume);
+
+ private:
+  GestModeOption gest_mode_;
 };
 
 void FileManagerBrowserTestBase::SetUpCommandLine(CommandLine* command_line) {
-  bool in_guest_mode = GetParam();
-  if (in_guest_mode) {
+  if (gest_mode_ == IN_GEST_MODE) {
     command_line->AppendSwitch(chromeos::switches::kGuestSession);
     command_line->AppendSwitchNative(chromeos::switches::kLoginUser, "");
     command_line->AppendSwitch(switches::kIncognito);
   }
   ExtensionApiTest::SetUpCommandLine(command_line);
 }
 
 void FileManagerBrowserTestBase::StartTest(const std::string& test_name) {
   base::FilePath path = test_data_dir_.AppendASCII("file_manager_browsertest");
   const extensions::Extension* extension = LoadExtensionAsComponent(path);
   ASSERT_TRUE(extension);
 
-  bool in_guest_mode = GetParam();
+  bool in_guest_mode = gest_mode_ == IN_GEST_MODE;
   ExtensionTestMessageListener listener(
       in_guest_mode ? "which test guest" : "which test non-guest", true);
   ASSERT_TRUE(listener.WaitUntilSatisfied());
   listener.Reply(test_name);
 }
 
 void FileManagerBrowserTestBase::CreateTestEntries(
     TestVolume* volume, const TestEntryInfo* entries, size_t num_entries) {
   for (size_t i = 0; i < num_entries; ++i) {
     volume->CreateEntry(entries[i]);
@@ skipping 14 matching lines @@
     NONE,
     "4 Sep 1998 00:00:00"
   };
   volume->CreateEntry(entry);
   listener.Reply("file added");
 
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
 // A class to test local volumes.
-class FileManagerBrowserLocalTest : public FileManagerBrowserTestBase {
+class FileManagerBrowserLocalTest :
+      public FileManagerBrowserTestBase,
+      public ::testing::WithParamInterface<GestModeOption> {
  public:
-  FileManagerBrowserLocalTest() : volume_("Downloads") {}
+  FileManagerBrowserLocalTest() : FileManagerBrowserTestBase(GetParam()),
+                                  volume_("Downloads") {}
 
  protected:
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
     FileManagerBrowserTestBase::SetUpInProcessBrowserTestFixture();
     extensions::ComponentLoader::EnableBackgroundExtensionsForTesting();
   }
 
   virtual void SetUpOnMainThread() OVERRIDE {
     FileManagerBrowserTestBase::SetUpOnMainThread();
     ASSERT_TRUE(volume_.Mount(browser()->profile()));
     CreateTestEntries(&volume_, kTestEntrySetCommon,
                       arraysize(kTestEntrySetCommon));
   }
 
   LocalTestVolume volume_;
 };
 
-INSTANTIATE_TEST_CASE_P(InGuestMode,
+INSTANTIATE_TEST_CASE_P(BothGuestMode,
                         FileManagerBrowserLocalTest,
-                        ::testing::Values(true));
-
-INSTANTIATE_TEST_CASE_P(InNonGuestMode,
-                        FileManagerBrowserLocalTest,
-                        ::testing::Values(false));
+                        ::testing::Values(IN_GEST_MODE, NOT_IN_GEST_MODE));
 
 // A class to test Drive's volumes
-class FileManagerBrowserDriveTest : public FileManagerBrowserTestBase {
+class FileManagerBrowserDriveTest :
+      public FileManagerBrowserTestBase,
+      public ::testing::WithParamInterface<GestModeOption> {

[hashimoto, 2013/05/31 09:16:21]

Can't we just pass NOT_IN_GUEST_MODE to the ctor of FileManagerBrowserTestBase?
It seems FileManagerBrowserDriveTest never gets instantiated for guest mode.

The same goes for FileManagerBrowserJavaScriptTest.

[hirono, 2013/06/03 01:45:53]

Done.

+ public:
+  FileManagerBrowserDriveTest() : FileManagerBrowserTestBase(GetParam()) {}
+
  protected:
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
     FileManagerBrowserTestBase::SetUpInProcessBrowserTestFixture();
     extensions::ComponentLoader::EnableBackgroundExtensionsForTesting();
     ASSERT_TRUE(volume_.SetUp());
   }
 
   virtual void SetUpOnMainThread() OVERRIDE {
     FileManagerBrowserTestBase::SetUpOnMainThread();
     CreateTestEntries(&volume_, kTestEntrySetCommon,
                       arraysize(kTestEntrySetCommon));
     // For testing Drive, create more entries with Drive specific attributes.
     // TODO(haruki): Add a case for an entry cached by DriveCache.
     CreateTestEntries(&volume_, kTestEntrySetDriveOnly,
                       arraysize(kTestEntrySetDriveOnly));
     drive_test_util::WaitUntilDriveMountPointIsAdded(browser()->profile());
   }
 
   DriveTestVolume volume_;
 };
 
 // Don't test Drive in the guest mode as it's not supported.
 INSTANTIATE_TEST_CASE_P(InNonGuestMode,
                         FileManagerBrowserDriveTest,
-                        ::testing::Values(false));
+                        ::testing::Values(NOT_IN_GEST_MODE));
+
+typedef std::tr1::tuple<GestModeOption, const char*> TransferTestParam;
 
 // A class to test both local and Drive's volumes.
-class FileManagerBrowserTransferTest : public FileManagerBrowserTestBase {
+class FileManagerBrowserJavaScriptTest :

[hashimoto, 2013/05/31 09:16:21]

"JavaScriptTest" seems to be too generic.
Why did you change the test name?

[hirono, 2013/06/03 01:45:53]

To parameterize all the tests, I removed FileManagerBrowserJavaScriptTest class
and introduced FileManagerBrowserComplexTest and FileManagerBrowserSimpleTest.

+      public FileManagerBrowserTestBase,
+      public ::testing::WithParamInterface<TransferTestParam> {
  public:
-  FileManagerBrowserTransferTest() : local_volume_("Downloads") {}
+  FileManagerBrowserJavaScriptTest() :
+      // FileManagerBrowserJavaScriptTest depends on Drive, which is not
+      // supported in the guest mode.
+      FileManagerBrowserTestBase(std::tr1::get<0>(GetParam())),
+      local_volume_("Downloads") {}
 
  protected:
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
     FileManagerBrowserTestBase::SetUpInProcessBrowserTestFixture();
     extensions::ComponentLoader::EnableBackgroundExtensionsForTesting();
     ASSERT_TRUE(drive_volume_.SetUp());
   }
 
   virtual void SetUpOnMainThread() OVERRIDE {
     FileManagerBrowserTestBase::SetUpOnMainThread();
     ASSERT_TRUE(local_volume_.Mount(browser()->profile()));
     CreateTestEntries(&local_volume_, kTestEntrySetCommon,
                       arraysize(kTestEntrySetCommon));
     CreateTestEntries(&drive_volume_, kTestEntrySetCommon,
                       arraysize(kTestEntrySetCommon));
     CreateTestEntries(&drive_volume_, kTestEntrySetDriveOnly,
                       arraysize(kTestEntrySetDriveOnly));
     drive_test_util::WaitUntilDriveMountPointIsAdded(browser()->profile());
   }
 
   LocalTestVolume local_volume_;
   DriveTestVolume drive_volume_;
 };
 
-// FileManagerBrowserTransferTest depends on Drive and Drive is not supported in
-// the guest mode.
-INSTANTIATE_TEST_CASE_P(InNonGuestMode,
-                        FileManagerBrowserTransferTest,
-                        ::testing::Values(false));
+INSTANTIATE_TEST_CASE_P(

[hashimoto, 2013/05/31 09:16:21]

Can't we do the same thing for all tests except
FileManagerBrowserLocalTest.TestFileDisplay?

[hirono, 2013/06/03 01:45:53]

Done.

+    TransferTest,
+    FileManagerBrowserJavaScriptTest,
+    ::testing::Combine(
+         ::testing::Values(NOT_IN_GEST_MODE),
+         ::testing::Values(
+              "transferFromDriveToDownloads",
+              "transferFromDownloadsToDrive",
+              "transferFromSharedToDownloads",
+              "transferFromSharedToDrive",
+              "transferFromRecentToDownloads",
+              "transferFromRecentToDrive",
+              "transferFromOfflineToDownloads",
+              "transferFromOfflineToDrive")));
 
 IN_PROC_BROWSER_TEST_P(FileManagerBrowserLocalTest, TestFileDisplay) {
   DoTestFileDisplay(&volume_);
 }
 
 IN_PROC_BROWSER_TEST_P(FileManagerBrowserLocalTest, TestGalleryOpen) {
   ResultCatcher catcher;
   ASSERT_NO_FATAL_FAILURE(StartTest("galleryOpenDownloads"));
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
@@ skipping 64 matching lines @@
   ASSERT_NO_FATAL_FAILURE(StartTest("openSidebarSharedWithMe"));
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
 IN_PROC_BROWSER_TEST_P(FileManagerBrowserDriveTest, TestAutocomplete) {
   ResultCatcher catcher;
   ASSERT_NO_FATAL_FAILURE(StartTest("autocomplete"));
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromDriveToDownloads) {
+IN_PROC_BROWSER_TEST_P(FileManagerBrowserJavaScriptTest,
+                       ExecuteJavaScript) {
   ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromDriveToDownloads"));
+  ASSERT_NO_FATAL_FAILURE(StartTest(std::tr1::get<1>(GetParam())));
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
-
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromDownloadsToDrive) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromDownloadsToDrive"));
-  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
-}
-
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromSharedToDownloads) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromSharedToDownloads"));
-  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
-}
-
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromSharedToDrive) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromSharedToDrive"));
-  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
-}
-
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromRecentToDownloads) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromRecentToDownloads"));
-  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
-}
-
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromRecentToDrive) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromRecentToDrive"));
-  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
-}
-
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromOfflineToDownloads) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromOfflineToDownloads"));
-  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
-}
-
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromOfflineToDrive) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromOfflineToDrive"));
-  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
-}
-
 }  // namespace