[Password manager Python tests] Remove the option to disable tests

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
 
 from environment import Environment
 from websitetest import WebsiteTest
 
 
 class TypeOfTestedWebsites:
   """An enum to specify which groups of tests to run."""
-  # Runs only the disabled tests.
-  # TODO(vabr): Remove this option.
-  DISABLED_TESTS = 0
-  # Runs only the enabled tests.
-  ENABLED_TESTS = 1
   # Runs all the tests.
-  ALL_TESTS = 2
+  ALL_TESTS = 0
   # Runs a specified list of tests.
-  LIST_OF_TESTS = 3
+  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")
@@ skipping 176 matching lines @@
 
 class Yandex(WebsiteTest):
 
   def Login(self):
     self.GoTo("https://mail.yandex.com")
     self.FillUsernameInto("#b-mail-domik-username11")
     self.FillPasswordInto("#b-mail-domik-password11")
     self.Click(".b-mail-button__button")
 
 
-# Disabled tests.
-
 # Fails due to test framework issue(?).
 class Aliexpress(WebsiteTest):
 
   def Login(self):
     self.GoTo("https://login.aliexpress.com/buyer.htm?return=http%3A%2F%2Fwww.aliexpress.com%2F")
     self.WaitUntilDisplayed("iframe#alibaba-login-box")
     frame = self.driver.find_element_by_css_selector("iframe#alibaba-login-box")
     self.driver.switch_to_frame(frame)
     self.FillUsernameInto("#fm-login-id")
     self.FillPasswordInto("#fm-login-password")
@@ skipping 213 matching lines @@
 
   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):
 
-  working_tests = {
+  all_tests = {
+    "163": One63("163"), # http://crbug.com/368690
+    "adobe": Adobe("adobe"), # Password saving not offered.
     "alexa": Alexa("alexa"),
-    "dropbox": Dropbox("dropbox"),
-    "facebook": Facebook("facebook"),
-    "github": Github("github"),
-    "google": Google("google"),
-    "imgur": Imgur("imgur"),
-    "liveinternet": Liveinternet("liveinternet"),
-    "linkedin": Linkedin("linkedin"),
-    "mailru": Mailru("mailru"),
-    "nytimes": Nytimes("nytimes"),
-    "odnoklassniki": Odnoklassniki("odnoklassniki"),
-    "pinterest": Pinterest("pinterest"),
-    "reddit": Reddit("reddit", username_not_auto=True),
-    "tumblr": Tumblr("tumblr", username_not_auto=True),
-    "twitter": Twitter("twitter"),
-    "vkontakte": Vkontakte("vkontakte"),
-    "wikia": Wikia("wikia"),
-    "wikipedia": Wikipedia("wikipedia", username_not_auto=True),
-    "wordpress": Wordpress("wordpress"),
-    "yahoo": Yahoo("yahoo", username_not_auto=True),
-    "yandex": Yandex("yandex")
-  }
-
-  disabled_tests = {
-    "adobe": Adobe("adobe"), # Password saving not offered.
     "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
-    "163": One63("163"), # http://crbug.com/368690
+    "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.
   }
 
   if tests_to_run:
     for test in tests_to_run:
-      if (test not in working_tests.keys() and
-          test not in disabled_tests.keys()):
+      if test not in all_tests.keys():
         print "Skip test: test {} is not in known tests".format(test)

[melandory, 2015/03/25 16:22:14]

Since recent switch to logger, probably this print should be changed too.

[vabr (Chromium), 2015/03/25 16:51:13]

Good point.
Because the value of this log is rather questionable (the automatically run
tests will run all tests, and a user running a test on demand can probably
figure out what's happening, or add a log locally), I decided to remove it
completely.

Once the refactoring is done, I'd like to introduce a unified way of logging in
these scripts, and for that I'll need to figure out a way to centralise the log
level for that and the target logger (either the root one, or define a single
designated one).

         continue
-      if test in working_tests.keys():
-        test_class = working_tests[test]
-      else:
-        test_class = disabled_tests[test]
+      test_class = all_tests[test]
       environment.AddWebsiteTest(test_class)
   else:
-    for test in working_tests.itervalues():
+    for test in all_tests.itervalues():
       environment.AddWebsiteTest(test)
-    for test in disabled_tests.itervalues():
-      environment.AddWebsiteTest(test, disabled=True)
 

[melandory, 2015/03/25 16:22:14]

Optional:
I would rewrite code in lines 490--500 in following way:

test_to_run = test_to_run or all_tests.keys()
for test_name in tests_to_run:
  if test_name not in all_tests.keys():
    print "Skip test: test {} is not in known tests".format(test_name)
  else:
    environment.AddWebsiteTest(all_tests[test_name])

It has some disadvantages, but in my opinion it's easier to read, so it's worth
it. 

[vabr (Chromium), 2015/03/25 16:51:13]

I like that, thanks for suggesting!
Done.

 
 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:
@@ skipping 46 matching lines @@
                             environment_log_file)
 
   # 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.DISABLED_TESTS:
-    environment.DisabledTests(run_prompt_tests)
   elif environment_tested_websites == TypeOfTestedWebsites.LIST_OF_TESTS:
     environment.Test(tests, run_prompt_tests)
-  elif environment_tested_websites == TypeOfTestedWebsites.ENABLED_TESTS:
-    environment.WorkingTests(run_prompt_tests)
   else:
     raise Exception("Error: |environment_tested_websites| has to be one of the"
         "TypeOfTestedWebsites values")
 
 
   environment.Quit()
   return environment.tests_results
 
 # Tests setup.
 if __name__ == "__main__":
@@ skipping 10 matching lines @@
       "--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)
 
   parser.add_argument(
       "--passwords-path", action="store", dest="passwords_path",
       help="Set the usernames/passwords path (required).", nargs=1,
       required=True)
-  parser.add_argument("--all", action="store_true", dest="all",
-                      help="Run all tests.")
-  parser.add_argument("--disabled", action="store_true", dest="disabled",
-                      help="Run only disabled tests.")
   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="Write the results in a file.")
   parser.add_argument("tests", help="Tests to be run.",  nargs="*")
 
   args = parser.parse_args()
 
   passwords_path = args.passwords_path[0]
 
-  if args.all:
-    tested_websites = TypeOfTestedWebsites.ALL_TESTS
-  elif args.disabled:
-    tested_websites = TypeOfTestedWebsites.DISABLED_TESTS
-  elif args.tests:
+  tested_websites = TypeOfTestedWebsites.ALL_TESTS
+  if args.tests:
     tested_websites = TypeOfTestedWebsites.LIST_OF_TESTS
-  else:
-    tested_websites = TypeOfTestedWebsites.ENABLED_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]
@@ skipping 22 matching lines @@
                             args.profile_path[0],
                             passwords_path,
                             True,
                             numeric_level,
                             args.log_screen,
                             log_file,
                             tested_websites,
                             args.tests)
 
   saveResults(tests_results, save_path)