[Password manager tests automation] Refactor test_runner.

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 188b537e4fb7b6cd6a6d4037504eeb87287924db..9281b99dc8ab9fc4eb9f8a683f7bf0520141cc2d 100644
--- a/components/test/data/password_manager/automated_tests/tests.py
+++ b/components/test/data/password_manager/automated_tests/tests.py
@@ -11,6 +11,9 @@ from environment import Environment
 from websitetest import WebsiteTest
 
 
+TEST_CASES = ("PromptFailTest", "PromptSuccessTest", "SaveAndAutofillTest")
+
+
 class Alexa(WebsiteTest):
 
   def Login(self):
@@ -506,8 +509,10 @@ def SaveResults(environment_tests_results, environment_save_path,
     with open(environment_save_path, "w") as save_file:
       save_file.write(xml)
 
+

[melandory, 2015/04/16 08:48:00]

This is intentional: there are two empty lines between functions across this
file.

[vabr (Chromium), 2015/04/16 10:48:16]

Acknowledged.

 def RunTest(chrome_path, chromedriver_path, profile_path,
-            environment_passwords_path, website_test_name, test_case_name):
+            environment_passwords_path, website_test_name,
+            test_case_name):
   """Runs the test for the specified website.
 
   Args:
@@ -530,15 +535,16 @@ def RunTest(chrome_path, chromedriver_path, profile_path,
   environment = Environment(chrome_path, chromedriver_path, profile_path,
                             environment_passwords_path,
                             enable_automatic_password_saving)
+  try:
+    if website_test_name in all_tests:
+      environment.AddWebsiteTest(all_tests[website_test_name])
+    else:
+      raise Exception("Test name {} is unknown.".format(website_test_name))
 
-  if website_test_name in all_tests:
-    environment.AddWebsiteTest(all_tests[website_test_name])
-  else:
-    raise Exception("Test name {} is unknown.".format(website_test_name))
-
-  environment.RunTestsOnSites(test_case_name)
-  environment.Quit()
-  return environment.tests_results
+    environment.RunTestsOnSites(test_case_name)
+    return environment.tests_results
+  finally:
+    environment.Quit()
 
 def main():
   parser = argparse.ArgumentParser(
@@ -579,9 +585,7 @@ def main():
   if args.save_path:
     save_path = args.save_path
 
-  test_cases_to_run = args.test_cases_to_run or\
-      ("PromptFailTest", "PromptSuccessTest", "SaveAndAutofillTest")
-
+  test_cases_to_run = args.test_cases_to_run or TEST_CASES

[vabr (Chromium), 2015/04/16 10:48:16]

nit: Could you please add a TODO about validating the supplied test case names?
I know you had plans for using the decorators, that's likely worth pointing out
in the TODO.

[melandory, 2015/04/16 14:38:29]

TODO is in this CL:
https://codereview.chromium.org/1084553003/diff/40001/components/test/data/pa...

Current CL depends on it, so I'll land them one after another. Sorry for not
mentioning it.

[vabr (Chromium), 2015/04/16 14:55:43]

Acknowledged. I should have remembered that, thanks for reminding me.

   for test_case in test_cases_to_run:
     tests_results = RunTest(
         args.chrome_path, args.chromedriver_path, args.profile_path,