Parameterize FileManagerBrowserTransferTest.

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

Index: chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
diff --git a/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc b/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
index 6ec9d31d303e3049097870202917ff1d5da562fe..975630a52c2d6d25dd558ca83f61e627bebe5073 100644
--- a/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
+++ b/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
@@ -47,6 +47,16 @@ enum SharedOption {
   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.
@@ -314,11 +324,13 @@ class DriveTestVolume : public TestVolume {
 
 // 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;
 
@@ -332,11 +344,13 @@ class FileManagerBrowserTestBase : public ExtensionApiTest,
 
   // 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);
@@ -349,7 +363,7 @@ void FileManagerBrowserTestBase::StartTest(const std::string& test_name) {
   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());
@@ -384,9 +398,12 @@ void FileManagerBrowserTestBase::DoTestFileDisplay(TestVolume* volume) {
 }
 
 // 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 {
@@ -404,16 +421,17 @@ class FileManagerBrowserLocalTest : public FileManagerBrowserTestBase {
   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();
@@ -438,12 +456,20 @@ class FileManagerBrowserDriveTest : public FileManagerBrowserTestBase {
 // 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 {
@@ -468,11 +494,20 @@ class FileManagerBrowserTransferTest : public FileManagerBrowserTestBase {
   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_);
@@ -557,60 +592,10 @@ IN_PROC_BROWSER_TEST_P(FileManagerBrowserDriveTest, TestAutocomplete) {
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
-IN_PROC_BROWSER_TEST_P(FileManagerBrowserTransferTest,
-                       TransferFromDriveToDownloads) {
-  ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromDriveToDownloads"));
-  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) {
+IN_PROC_BROWSER_TEST_P(FileManagerBrowserJavaScriptTest,
+                       ExecuteJavaScript) {
   ResultCatcher catcher;
-  ASSERT_NO_FATAL_FAILURE(StartTest("transferFromSharedToDownloads"));
+  ASSERT_NO_FATAL_FAILURE(StartTest(std::tr1::get<1>(GetParam())));
   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