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

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 f21ce1dd6d480e4dff6e7c50f45bd602b85b69d2..b4d1d821c312f4f416b46937e29d89136ebee798 100644
--- a/components/test/data/password_manager/automated_tests/tests.py
+++ b/components/test/data/password_manager/automated_tests/tests.py
@@ -14,15 +14,10 @@ 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
@@ -219,8 +214,6 @@ class Yandex(WebsiteTest):
     self.Click(".b-mail-button__button")
 
 
-# Disabled tests.
-
 # Fails due to test framework issue(?).
 class Aliexpress(WebsiteTest):
 
@@ -454,66 +447,57 @@ class Ziddu(WebsiteTest):
 
 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"),
+    "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"),
-    "liveinternet": Liveinternet("liveinternet"),
+    "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")
-  }
-
-  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.
-    "ebay": Ebay("ebay"), # http://crbug.com/368690
-    "espn": Espn("espn"), # Iframe, password saved but not autofilled.
-    "flipkart": Flipkart("flipkart"), # Fails due to test framework issue.
-    "instagram": Instagram("instagram"), # Iframe, pw saved but not autofilled.
-    "live": Live("live", username_not_auto=True),  # http://crbug.com/367768
-    "163": One63("163"), # http://crbug.com/368690
-    "stackexchange": StackExchange("stackexchange"), # Iframe, not autofilled.
-    "vube": Vube("vube"), # http://crbug.com/368690
+    "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):
@@ -581,12 +565,8 @@ def RunTests(chrome_path, chromedriver_path, profile_path,
 
   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")
@@ -617,10 +597,6 @@ if __name__ == "__main__":
       "--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",
@@ -635,14 +611,9 @@ if __name__ == "__main__":
 
   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: