[Password manager Python tests] Remove some dead code

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

 # 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.
 
 """The testing Environment class."""
 
-import logging
 import shutil
-import sys
 import time
-import traceback
 from xml.etree import ElementTree
-from xml.sax.saxutils import escape
-
-sys.path.insert(0, '../../../../third_party/webdriver/pylib/')
 
 from selenium import webdriver
-from selenium.common.exceptions import NoSuchElementException
-from selenium.common.exceptions import WebDriverException
 from selenium.webdriver.chrome.options import Options
 
 
 # Message strings to look for in chrome://password-manager-internals
 MESSAGE_ASK = "Message: Decision: ASK the user"
 MESSAGE_SAVE = "Message: Decision: SAVE the password"
 
 
 class TestResult:
   """Stores the information related to a test result. """
   def __init__(self, name, test_type, successful, message):
     """Creates a new TestResult.
 
     Args:
       name: The tested website name.
       test_type: The test type.
       successful: Whether or not the test was successful.
       message: The error message of the test.
     """
     self.name = name
     self.test_type = test_type
     self.successful = successful
     self.message = message
 
 
 class Environment:
   """Sets up the testing Environment. """
 
   def __init__(self, chrome_path, chromedriver_path, profile_path,
-               passwords_path, enable_automatic_password_saving,
-               numeric_level=None, log_to_console=False, log_file=""):
+               passwords_path, enable_automatic_password_saving):
     """Creates a new testing Environment.
 
     Args:
       chrome_path: The chrome binary file.
       chromedriver_path: The chromedriver binary file.
       profile_path: The chrome testing profile folder.
       passwords_path: The usernames and passwords file.
       enable_automatic_password_saving: If True, the passwords are going to be
           saved without showing the prompt.
-      numeric_level: The log verbosity.
-      log_to_console: If True, the debug logs will be shown on the console.
-      log_file: The file where to store the log. If it's empty, the log will
-          not be stored.
 
     Raises:
       Exception: An exception is raised if |profile_path| folder could not be
       removed.
     """
-    # Setting up the login.
-    if numeric_level is not None:
-      if log_file:
-        # Set up logging to file.
-        logging.basicConfig(level=numeric_level,
-                            filename=log_file,
-                            filemode='w')
-
-        if log_to_console:
-          console = logging.StreamHandler()
-          console.setLevel(numeric_level)
-          # Add the handler to the root logger.
-          logging.getLogger('').addHandler(console)
-
-      elif log_to_console:
-        logging.basicConfig(level=numeric_level)
 
     # Cleaning the chrome testing profile folder.
-    try:
-      shutil.rmtree(profile_path)
-    except Exception, e:
-      pass
-    # If |chrome_path| is not defined, this means that we are in the dashboard

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

Could you please mention in the CL description that this functionality is
removed too?

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

This was not so much removed, as simplified. In the past, to get the list of
websites, Environment had to be instantiated. I updated the CL description with
that. Note that getting the test names is not specific to the use in the
dashboard (which itself became deprecated in the meantime), so I don't mention
the dashboard in the CL description.
Done.

[engedy, 2015/03/26 18:04:54]

Acknowledged.

-    # website, and we just need to get the list of all websites. In this case,
-    # we don't need to initilize the webdriver.
-    if chrome_path:
-      options = Options()
-      self.enable_automatic_password_saving = enable_automatic_password_saving
-      if enable_automatic_password_saving:
-        options.add_argument("enable-automatic-password-saving")
-      # Chrome path.
-      options.binary_location = chrome_path
-      # Chrome testing profile path.
-      options.add_argument("user-data-dir=%s" % profile_path)
+    shutil.rmtree(profile_path, ignore_errors=True)

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

nit: Given that this masks both kinds of errors (i.e., directory did not exist
in the first place, existed but failed to remove), I would instead check
existence and then call this with ignore_errors=False.

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

Done.

+    options = Options()
+    self.enable_automatic_password_saving = enable_automatic_password_saving
+    if enable_automatic_password_saving:
+      options.add_argument("enable-automatic-password-saving")
+    # Chrome path.
+    options.binary_location = chrome_path
+    # Chrome testing profile path.
+    options.add_argument("user-data-dir=%s" % profile_path)
 
-      # The webdriver. It's possible to choose the port the service is going to
-      # run on. If it's left to 0, a free port will be found.
-      self.driver = webdriver.Chrome(chromedriver_path, 0, options)
-      # The password internals window.
-      self.internals_window = self.driver.current_window_handle
-      if passwords_path:
-        # An xml tree filled with logins and passwords.
-        self.passwords_tree = ElementTree.parse(passwords_path).getroot()
-      else:
-        raise Exception("Error: |passwords_path| needs to be provided if"
-            "|chrome_path| is provided, otherwise the tests could not be run")
+    # The webdriver. It's possible to choose the port the service is going to
+    # run on. If it's left to 0, a free port will be found.
+    self.driver = webdriver.Chrome(chromedriver_path, 0, options)
+    # The password internals window.
+    self.internals_window = self.driver.current_window_handle
+    if passwords_path:
+      # An xml tree filled with logins and passwords.
+      self.passwords_tree = ElementTree.parse(passwords_path).getroot()
+    else:
+      raise Exception("Error: |passwords_path| needs to be provided if"
+          "|chrome_path| is provided, otherwise the tests could not be run")
     # Password internals page.
     self.internals_page = "chrome://password-manager-internals/"
     # The Website window.
     self.website_window = None
     # The WebsiteTests list.
     self.websitetests = []
     # Map messages to the number of their appearance in the log.
-    self.message_count = dict()
-    self.message_count[MESSAGE_ASK] = 0
-    self.message_count[MESSAGE_SAVE] = 0
+    self.message_count = { MESSAGE_ASK: 0, MESSAGE_SAVE: 0 }
     # The tests needs two tabs to work. A new tab is opened with the first
     # GoTo. This is why we store here whether or not it's the first time to
     # execute GoTo.
     self.first_go_to = True
     # List of all tests results.
     self.tests_results = []
 
   def AddWebsiteTest(self, websitetest):
     """Adds a WebsiteTest to the testing Environment.
 
+    TODO(vabr): Currently, this is only called at most once for each
+    Environment instance. That is because to run all tests efficiently in
+    parallel, each test gets its own process spawned (outside of Python).
+    That makes sense, but then we should flatten the hierarchy of calls
+    and consider making the 1:1 relation of environment to tests more
+    explicit.
+
     Args:
       websitetest: The WebsiteTest instance to be added.
     """
     websitetest.environment = self
-    if hasattr(self, "driver"):
-      websitetest.driver = self.driver
-    if hasattr(self, "passwords_tree") and self.passwords_tree is not None:
-      if not websitetest.username:
-        username_tag = (
-            self.passwords_tree.find(
-                ".//*[@name='%s']/username" % websitetest.name))
-        if username_tag.text:
-          websitetest.username = username_tag.text
-      if not websitetest.password:
-        password_tag = (
-            self.passwords_tree.find(
-                ".//*[@name='%s']/password" % websitetest.name))
-        if password_tag.text:
-          websitetest.password = password_tag.text
+    websitetest.driver = self.driver

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

nit: Please add a todo to make this a property.

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

Done.

+    if not websitetest.username:
+      username_tag = (
+          self.passwords_tree.find(
+              ".//*[@name='%s']/username" % websitetest.name))
+      websitetest.username = username_tag.text
+    if not websitetest.password:
+      password_tag = (
+          self.passwords_tree.find(
+              ".//*[@name='%s']/password" % websitetest.name))
+      websitetest.password = password_tag.text
     self.websitetests.append(websitetest)
 
   def ClearCache(self, clear_passwords):
     """Clear the browser cookies. If |clear_passwords| is true, clear all the
     saved passwords too.
 
     Args:
       clear_passwords : Clear all the passwords if the bool value is true.
     """
-    logging.info("\nClearCache\n")
     self.driver.get("chrome://settings/clearBrowserData")
     self.driver.switch_to_frame("settings")
     script = (
         "if (!document.querySelector('#delete-cookies-checkbox').checked)"
         "  document.querySelector('#delete-cookies-checkbox').click();"
         )
     negation = ""
     if clear_passwords:
       negation = "!"
     script += (
         "if (%sdocument.querySelector('#delete-passwords-checkbox').checked)"
         "  document.querySelector('#delete-passwords-checkbox').click();"
         % negation)
     script += "document.querySelector('#clear-browser-data-commit').click();"
     self.driver.execute_script(script)
     time.sleep(2)
     # Every time we do something to the cache let's enable password saving.
     # TODO(melandory): We should check why it's off in a first place.
     # TODO(melandory): Investigate, maybe there is no need to enable it that
     # often.
     self.EnablePasswordsSaving()
 
   def EnablePasswordsSaving(self):
-    logging.info("\nEnablePasswordSaving\n")
     self.driver.get("chrome://settings")
     self.driver.switch_to_frame("settings")
     script = "document.getElementById('advanced-settings-expander').click();"
     self.driver.execute_script(script)
     time.sleep(2)
     script = (
         "if (!document.querySelector('#password-manager-enabled').checked)"
         "{ document.querySelector('#password-manager-enabled').click();}")
     self.driver.execute_script(script)
     time.sleep(2)
@@ skipping 89 matching lines @@
       Exception: An exception is raised in case the result does not match the
           expectation
     """
     if (self._DidMessageAppearUntilTimeout(log_message, timeout) !=
         message_should_show_up):
       raise Exception(error_message)
 
   def AllTests(self, prompt_test):
     """Runs the tests on all the WebsiteTests.
 
+    TODO(vabr): Currently, "all tests" always means one.
+
     Args:
       prompt_test: If True, tests caring about showing the save-password
           prompt are going to be run, otherwise tests which don't care about
           the prompt are going to be run.
 
     Raises:
       Exception: An exception is raised if the tests fail.
     """
     if prompt_test:
       self.PromptTestList(self.websitetests)
@@ skipping 73 matching lines @@
       except Exception as e:
         successful = False
         error = e.message
       self.tests_results.append(TestResult(websitetest.name, "prompt",
                                            successful, error))
 
   def Quit(self):
     """Closes the tests."""
     # Close the webdriver.
     self.driver.quit()