[Password manager Python tests] Remove some dead code

components/test/data/password_manager/automated_tests/tests.py

Index: components/test/data/password_manager/automated_tests/tests.py
diff --git a/components/test/data/password_manager/automated_tests/tests.py b/components/test/data/password_manager/automated_tests/tests.py
index a37114f960919595e34a40b55454516ef7e6e82b..9e47d5139e038e486f0cf142d983a90bce1a4e84 100644
--- a/components/test/data/password_manager/automated_tests/tests.py
+++ b/components/test/data/password_manager/automated_tests/tests.py
@@ -12,17 +12,6 @@ from environment import Environment
 from websitetest import WebsiteTest
 
 
-class TypeOfTestedWebsites:
-  """An enum to specify which groups of tests to run."""
-  # Runs all the tests.
-  ALL_TESTS = 0
-  # Runs a specified list of tests.
-  LIST_OF_TESTS = 1
-
-  def __init__(self):
-    pass
-
-
 class Alexa(WebsiteTest):
 
   def Login(self):
@@ -445,53 +434,60 @@ class Ziddu(WebsiteTest):
     self.Click(".login input")
 
 
-def Tests(environment, tests_to_run=None):
-
-  all_tests = {
-    "163": One63("163"), # http://crbug.com/368690
-    "adobe": Adobe("adobe"), # Password saving not offered.
-    "alexa": Alexa("alexa"),
-    "aliexpress": Aliexpress("aliexpress"), # Fails due to test framework issue.
-    "amazon": Amazon("amazon"), # Bug not reproducible without test.
-    "ask": Ask("ask"), # Password not saved.
-    "baidu": Baidu("baidu"), # Password not saved.
-    "cnn": Cnn("cnn"), # http://crbug.com/368690
-    "craigslist": Craigslist("craigslist"), # Too many failed logins per time.
-    "dailymotion": Dailymotion("dailymotion"), # Crashes.
-    "dropbox": Dropbox("dropbox"),
-    "ebay": Ebay("ebay"), # http://crbug.com/368690
-    "espn": Espn("espn"), # Iframe, password saved but not autofilled.
-    "facebook": Facebook("facebook"),
-    "flipkart": Flipkart("flipkart"), # Fails due to test framework issue.
-    "github": Github("github"),
-    "google": Google("google"),
-    "imgur": Imgur("imgur"),
-    "instagram": Instagram("instagram"), # Iframe, pw saved but not autofilled.
-    "linkedin": Linkedin("linkedin"),
-    "liveinternet": Liveinternet("liveinternet"),
-    "live": Live("live", username_not_auto=True),  # http://crbug.com/367768
-    "mailru": Mailru("mailru"),
-    "nytimes": Nytimes("nytimes"),
-    "odnoklassniki": Odnoklassniki("odnoklassniki"),
-    "pinterest": Pinterest("pinterest"),
-    "reddit": Reddit("reddit", username_not_auto=True),
-    "stackexchange": StackExchange("stackexchange"), # Iframe, not autofilled.
-    "tumblr": Tumblr("tumblr", username_not_auto=True),
-    "twitter": Twitter("twitter"),
-    "vkontakte": Vkontakte("vkontakte"),
-    "vube": Vube("vube"), # http://crbug.com/368690
-    "wikia": Wikia("wikia"),
-    "wikipedia": Wikipedia("wikipedia", username_not_auto=True),
-    "wordpress": Wordpress("wordpress"),
-    "yahoo": Yahoo("yahoo", username_not_auto=True),
-    "yandex": Yandex("yandex"),
-    "ziddu": Ziddu("ziddu"), # Password not saved.
-  }
-
-  tests_to_run = tests_to_run or all_tests.keys()
-  for test_name in tests_to_run:
-    if test_name in all_tests.keys():
-      environment.AddWebsiteTest(all_tests[test_name])
+all_tests = {
+  "163": One63("163"), # http://crbug.com/368690
+  "adobe": Adobe("adobe"), # Password saving not offered.
+  "alexa": Alexa("alexa"),
+  "aliexpress": Aliexpress("aliexpress"), # Fails due to test framework issue.
+  "amazon": Amazon("amazon"), # Bug not reproducible without test.
+  "ask": Ask("ask"), # Password not saved.
+  "baidu": Baidu("baidu"), # Password not saved.
+  "cnn": Cnn("cnn"), # http://crbug.com/368690
+  "craigslist": Craigslist("craigslist"), # Too many failed logins per time.
+  "dailymotion": Dailymotion("dailymotion"), # Crashes.
+  "dropbox": Dropbox("dropbox"),
+  "ebay": Ebay("ebay"), # http://crbug.com/368690
+  "espn": Espn("espn"), # Iframe, password saved but not autofilled.
+  "facebook": Facebook("facebook"),
+  "flipkart": Flipkart("flipkart"), # Fails due to test framework issue.
+  "github": Github("github"),
+  "google": Google("google"),
+  "imgur": Imgur("imgur"),
+  "instagram": Instagram("instagram"), # Iframe, pw saved but not autofilled.
+  "linkedin": Linkedin("linkedin"),
+  "liveinternet": Liveinternet("liveinternet"),
+  "live": Live("live", username_not_auto=True),  # http://crbug.com/367768
+  "mailru": Mailru("mailru"),
+  "nytimes": Nytimes("nytimes"),
+  "odnoklassniki": Odnoklassniki("odnoklassniki"),
+  "pinterest": Pinterest("pinterest"),
+  "reddit": Reddit("reddit", username_not_auto=True),
+  "stackexchange": StackExchange("stackexchange"), # Iframe, not autofilled.
+  "tumblr": Tumblr("tumblr", username_not_auto=True),
+  "twitter": Twitter("twitter"),
+  "vkontakte": Vkontakte("vkontakte"),
+  "vube": Vube("vube"), # http://crbug.com/368690
+  "wikia": Wikia("wikia"),
+  "wikipedia": Wikipedia("wikipedia", username_not_auto=True),
+  "wordpress": Wordpress("wordpress"),
+  "yahoo": Yahoo("yahoo", username_not_auto=True),
+  "yandex": Yandex("yandex"),
+  "ziddu": Ziddu("ziddu"), # Password not saved.
+}
+
+
+def Tests(environment, test_to_run):

[engedy, 2015/03/26 16:18:55]

nit: Do we even need this function? (Fine to leave it for now if removing it
interferes with refactoring.)

[vabr (Chromium), 2015/03/26 18:01:50]

Good call, removed.
Done.

+  """Initialize |environment| with the test to run.
+
+  TODO(vabr): If we end up using 1 environment for 1 tests, we need to
+  reconsider the structure and make some things more direct.
+
+  test_to_run: The name of the test to be run. This is a key for
+      |all_tests| dictionary above.
+  """
+
+  if test_to_run in all_tests.keys():

[engedy, 2015/03/26 16:18:55]

nit: We could just use the 'in' operator.

[vabr (Chromium), 2015/03/26 18:01:50]

You meant -- use it on the dictionary directly?
That sounds good, and is even asymptotically faster, assuming the dict is sorted
by the keys. Thanks for pointing that out (I forgot that in acts on the keys).
If you meant something else, please let me know.
Done.

[engedy, 2015/03/26 18:04:54]

Yes, that's what I meant.

+    environment.AddWebsiteTest(all_tests[test_to_run])
 
 
 def saveResults(environment_tests_results, environment_save_path):
@@ -517,8 +513,7 @@ def saveResults(environment_tests_results, environment_save_path):
 
 def RunTests(chrome_path, chromedriver_path, profile_path,

[engedy, 2015/03/26 16:18:55]

nit: Rename this to RunTest if it does not interfere with refactoring.

[vabr (Chromium), 2015/03/26 18:01:50]

Done.

              environment_passwords_path, enable_automatic_password_saving,
-             environment_numeric_level, log_to_console, environment_log_file,
-             environment_tested_websites, tests=None):
+             test_to_run):
 
   """Runs the the tests

[engedy, 2015/03/26 16:18:55]

typo/nit: Runs the specified test.

[vabr (Chromium), 2015/03/26 18:01:50]

I actually rephrased a bit more, because there was confusion between test in the
sense of a website for test, and test as a particular test scenario applied to
that website. There are multiple tests scenarios applied to one website. I now
keep referring to the scenarios as tests, and to the website as a website in the
function.

[engedy, 2015/03/26 18:04:54]

Acknowledged.

 
@@ -529,42 +524,26 @@ def RunTests(chrome_path, chromedriver_path, profile_path,
     environment_passwords_path: The usernames and passwords file.
     enable_automatic_password_saving: If True, the passwords are going to be
         saved without showing the prompt.
-    environment_numeric_level: The log verbosity.
-    log_to_console: If True, the debug logs will be shown on the console.
-    environment_log_file: The file where to store the log. If it's empty, the
-        log is not stored.
-    environment_tested_websites: One of the TypeOfTestedWebsites values,
-        indicating which group of tests to run.
-    tests: Specifies which tests to run. Ignored unless
-       |environment_tested_websites| is equal to LIST_OF_TESTS.
+    test_to_run: Name of the test to run.
 
   Returns:
     The results of tests as list of TestResults.
+
   Raises:
     Exception: An exception is raised if one of the tests fails.
   """
 
   environment = Environment(chrome_path, chromedriver_path, profile_path,
                             environment_passwords_path,
-                            enable_automatic_password_saving,
-                            environment_numeric_level,
-                            log_to_console,
-                            environment_log_file)
+                            enable_automatic_password_saving)
 
   # Test which care about the save-password prompt need the prompt
   # to be shown. Automatic password saving results in no prompt.
   run_prompt_tests = not enable_automatic_password_saving
 
-  Tests(environment, tests)
-
-  if environment_tested_websites == TypeOfTestedWebsites.ALL_TESTS:
-    environment.AllTests(run_prompt_tests)
-  elif environment_tested_websites == TypeOfTestedWebsites.LIST_OF_TESTS:
-    environment.Test(tests, run_prompt_tests)
-  else:
-    raise Exception("Error: |environment_tested_websites| has to be one of the"
-        "TypeOfTestedWebsites values")
+  Tests(environment, test_to_run)
 
+  environment.AllTests(run_prompt_tests)
 
   environment.Quit()
   return environment.tests_results
@@ -576,77 +555,46 @@ if __name__ == "__main__":
 
   parser.add_argument(
       "--chrome-path", action="store", dest="chrome_path",
-      help="Set the chrome path (required).", nargs=1, required=True)
+      help="Set the chrome path (required).", required=True)
   parser.add_argument(
       "--chromedriver-path", action="store", dest="chromedriver_path",
-      help="Set the chromedriver path (required).", nargs=1, required=True)
+      help="Set the chromedriver path (required).", required=True)
   parser.add_argument(
       "--profile-path", action="store", dest="profile_path",
       help="Set the profile path (required). You just need to choose a "
            "temporary empty folder. If the folder is not empty all its content "
            "is going to be removed.",
-      nargs=1, required=True)
+      required=True)
 
   parser.add_argument(
       "--passwords-path", action="store", dest="passwords_path",
-      help="Set the usernames/passwords path (required).", nargs=1,
-      required=True)
-  parser.add_argument("--log", action="store", nargs=1, dest="log_level",
-                      help="Set log level.")
-  parser.add_argument("--log-screen", action="store_true", dest="log_screen",
-                      help="Show log on the screen.")
-  parser.add_argument("--log-file", action="store", dest="log_file",
-                      help="Write the log in a file.", nargs=1)
-  parser.add_argument("--save-path", action="store", nargs=1, dest="save_path",
+      help="Set the usernames/passwords path (required).", required=True)
+  parser.add_argument("--save-path", action="store", dest="save_path",
                       help="Write the results in a file.")
-  parser.add_argument("tests", help="Tests to be run.",  nargs="*")
+  parser.add_argument("test", help="Test to be run.")
 
   args = parser.parse_args()
 
-  passwords_path = args.passwords_path[0]
-
-  tested_websites = TypeOfTestedWebsites.ALL_TESTS
-  if args.tests:
-    tested_websites = TypeOfTestedWebsites.LIST_OF_TESTS
-
-  numeric_level = None
-  if args.log_level:
-    numeric_level = getattr(logging, args.log_level[0].upper(), None)
-    if not isinstance(numeric_level, int):
-      raise ValueError("Invalid log level: %s" % args.log_level[0])
-
-  log_file = None
-  if args.log_file:
-    log_file = args.log_file[0]
-
   save_path = None
   if args.save_path:
-    save_path = args.save_path[0]
+    save_path = args.save_path
 
   # Run the test without enable-automatic-password-saving to check whether or
   # not the prompt is shown in the way we expected.
-  tests_results = RunTests(args.chrome_path[0],
-                           args.chromedriver_path[0],
-                           args.profile_path[0],
-                           passwords_path,
+  tests_results = RunTests(args.chrome_path,
+                           args.chromedriver_path,
+                           args.profile_path,
+                           args.passwords_path,
                            False,
-                           numeric_level,
-                           args.log_screen,
-                           log_file,
-                           tested_websites,
-                           args.tests)
+                           args.test)
 
   # Run the test with enable-automatic-password-saving to check whether or not
   # the passwords is stored in the the way we expected.
-  tests_results += RunTests(args.chrome_path[0],
-                            args.chromedriver_path[0],
-                            args.profile_path[0],
-                            passwords_path,
+  tests_results += RunTests(args.chrome_path,
+                            args.chromedriver_path,
+                            args.profile_path,
+                            args.passwords_path,
                             True,
-                            numeric_level,
-                            args.log_screen,
-                            log_file,
-                            tested_websites,
-                            args.tests)
+                            args.test)
 
   saveResults(tests_results, save_path)