[Password manager Python tests] Remove some dead code

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

 # -*- coding: utf-8 -*-
 # Copyright 2014 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.
 
 """Automated tests for many websites"""
 
 import argparse
 import logging

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

I think this can be removed.

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

Done.

 
 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):
     self.GoTo("https://www.alexa.com/secure/login")
     self.FillUsernameInto("#email")
     self.FillPasswordInto("#pwd")
     self.Submit("#pwd")
 
 
 class Dropbox(WebsiteTest):
@@ skipping 402 matching lines @@
 # Password not saved.
 class Ziddu(WebsiteTest):
 
   def Login(self):
     self.GoTo("http://www.ziddu.com/login.php")
     self.FillUsernameInto("#email")
     self.FillPasswordInto("#password")
     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.
+}
 
-  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])
+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):
   """Save the test results in an xml file.
 
   Args:
     environment_tests_results: A list of the TestResults that are going to be
         saved.
     environment_save_path: The file where the results are going to be saved.
         If it's None, the results are not going to be stored.
   Raises:
     Exception: An exception is raised if the file is not found.
   """
   if environment_save_path:
     xml = "<result>"
     for test_result in environment_tests_results:
       xml += ("<test name='%s' successful='%s' type='%s'>%s</test>"
           % (test_result.name, str(test_result.successful),
           test_result.test_type, test_result.message))
     xml += "</result>"
     with open(environment_save_path, "w") as save_file:
       save_file.write(xml)
 
 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.

 
   Args:
     chrome_path: The chrome binary file.
     chromedriver_path: The chromedriver binary file.
     profile_path: The chrome testing profile folder.
     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)
+  Tests(environment, test_to_run)
 
-  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")
-
+  environment.AllTests(run_prompt_tests)
 
   environment.Quit()
   return environment.tests_results
 
 # Tests setup.
 if __name__ == "__main__":
   parser = argparse.ArgumentParser(
       description="Password Manager automated tests help.")
 
   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)