[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)
 
+
 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

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

Now that only one test case is run, we should return a single result, not a
list.
Ultimately, Environment should be changed to expect that it is created for a
single test case solely. In particular, environment.tests_results should also
not be a list and should not have the name in plural. But that is out of scope
of this CL. Adding that as a TODO(vabr) in Environment would be helpful, though.

[melandory, 2015/04/17 08:11:30]

Some after thoughts:
I don't like idea of making this change now, because one can use Environment to
run several test cases. And making this change only because we know that it's
not used this way doesn't sounds right. WDYT?
There is a similar TODO in Environment.AddWebsiteTest. 

[vabr (Chromium), 2015/04/17 08:23:27]

Leaving it for later sounds fine. The plan is to simplify all the classes, and
since the way we use them has changed it will very likely lead to such API
changes as proposed. But I agree that separating that into different CLs will be
easier.
Fair enough, sounds good.

+  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
   for test_case in test_cases_to_run:
     tests_results = RunTest(
         args.chrome_path, args.chromedriver_path, args.profile_path,