Password Manager testing automation

components/test/data/password_manager/websitetest.py

+"""WebsiteTest testing class."""
+
+import logging
+import time
+
+from selenium.common.exceptions import NoSuchElementException
+from selenium.common.exceptions import StaleElementReferenceException
+from selenium.webdriver.common.action_chains import ActionChains
+from selenium.webdriver.common.keys import Keys
+
+
+def _IsOneSubstringOfAnother(s1, s2):
+  """Checks if one of the string arguements is substring of the other.
+
+  Args:
+      s1: The first string.
+      s2: The second string.
+  Returns:
+
+    True if one of the string arguements is substring of the other.
+    False otherwise.
+  """
+  return s1 in s2 or s2 in s1
+
+
+class WebsiteTest:
+  """Handles a tested WebsiteTest."""
+
+  class Mode:
+    """Test mode."""
+    # Password and username are expected to be autofilled.
+    AUTOFILLED = 1
+    # Password and username are not expected to be autofilled.
+    NOT_AUTOFILLED = 2
+
+    def __init__(self):
+      pass
+
+  def __init__(
+      self, name, username=None, password=None,

[vabr (Chromium), 2014/05/20 14:47:25]

Please remove username and password, you don't use them.
You do use self.username and self.password, but you never assign to them through
__init__, so the arguments in __init__ are not used.

[rchtara, 2014/05/22 08:44:38]

There not used, but they are useful: if someone who wants to create new tests
and doesn't want to use an xml file for password 

+      username_not_auto=False):
+    """Creates a new WebsiteTest.
+
+    Args:
+      name: The website name.
+      username: The website username. If it's None, the username is going to be
+          replaced with the value in the usernames and passwords file.
+      password: The website password. If it's None, the password is going to be
+          replaced with the value in the usernames and passwords file.
+      username_not_auto: Username inputs in some websites (like wikipedia) are
+          sometimes filled with some messages and thus, the usernames are not
+          automatically autofilled. This flag handles that and disables us from
+          checking if the state of the DOM is the same as the username of
+          website.
+    """
+    # Name of the website
+    self.name = name
+    # Username of the website.
+    self.username = username
+    # Password of the website.
+    self.password = password
+    # Username is not automatically filled.
+    self.username_not_auto = username_not_auto
+    # Autofilling mode.
+    self.mode = self.Mode.NOT_AUTOFILLED
+    # The |remaining_time_to_wait| limits the total time spent in waiting, but

[vabr (Chromium), 2014/05/20 14:47:25]

nit: "spent in waiting, but ... running." -> "spent in potentially infinite
loops."

This would make clear that the purpose is not to set a hard limit on the test
duration, but rather avoid infinite loops. In that case it's not important to
comment on spent time which does not contribute to remaining_time_to_wait
decrements.

[rchtara, 2014/05/22 08:44:38]

Done.

+    # does not include the time spent running.
+    self.remaining_time_to_wait = 200
+    # The testing Environment.
+    self.environment = None
+    # The webdriver.
+    self.driver = None
+
+  # Mouse/Keyboard actions.
+
+  def Click(self, selector):
+    """Clicks on an element.
+
+    Args:
+      selector: The element CSS selector.
+    """
+    logging.info("action: Click %s" % selector)
+    element = self.driver.find_element_by_css_selector(selector)
+    element.click()
+
+  def ClickIfVisible(self, selector):
+    """Clicks on an element, if it's visible.

[vabr (Chromium), 2014/05/20 14:47:25]

I'm not sure I understand what's meant by "visible" -- if one element covers
another, if it has 100% transparency, is out of the viewing area, etc., would it
not be clicked?
The "NoSuchElementException" suggests only elements which are in fact not
present in the DOM at all will not be clicked. Please explain.

[rchtara, 2014/05/22 08:44:38]

Done.

+
+    Args:
+      selector: The element CSS selector.
+
+    Returns:
+      True if the element is visible.
+      False otherwise.
+    """
+    logging.info("action: ClickIfVisible %s" % selector)
+    try:
+      element = self.driver.find_element_by_css_selector(selector)
+      element.click()
+      return True
+    except NoSuchElementException:
+      return False
+    except StaleElementReferenceException:
+      return False
+
+  def GoTo(self, url):
+    """Navigates the main frame to the |url|.
+
+    Args:
+      url: The URL.
+    """
+    logging.info("action: GoTo %s" % self.name)
+    if self.environment.first_go_to:
+      self.environment.OpenTabAndGoToInternals(url)
+      self.environment.first_go_to = False
+    else:
+      self.driver.get(url)
+
+  def HoverOver(self, selector):
+    """Hovers over an element.
+
+    Args:
+      selector: The element CSS selector.
+    """
+    logging.info("action: Hover %s" % selector)
+    element = self.driver.find_element_by_css_selector(selector)
+    hover = ActionChains(self.driver).move_to_element(element)
+    hover.perform()
+
+  def SendEnterTo(self, selector):
+    """Sends an enter key to an element.
+
+    Args:
+      selector: The element CSS selector.
+    """
+    logging.info("action: SendEnterTo %s" % selector)
+    body = self.driver.find_element_by_tag_name("body")
+    body.send_keys(Keys.ENTER)
+
+  # Waiting/Displaying actions.
+
+  def IsDisplayed(self, selector):
+    """Checks if an element is displayed.
+
+    Args:
+      selector: The element CSS selector.
+
+    Returns:
+      True if the element is visible.

[vabr (Chromium), 2014/05/20 14:47:25]

Please be consistent with any changes you make about the term "visible" in
ClickIfVisible. Also here it's not clear what's the relation between "displayed"
and "visible". Please use one term if they mean the same thing.

[rchtara, 2014/05/22 08:44:38]

Done.

+      False otherwise.
+    """
+    logging.info("action: IsDisplayed %s" % selector)
+    try:
+      element = self.driver.find_element_by_css_selector(selector)
+      return element.is_displayed()
+    except NoSuchElementException:
+      return False
+    except StaleElementReferenceException:
+      return False
+
+  def Wait(self, duration):
+    """Wait for a duration in seconds.

[vabr (Chromium), 2014/05/20 14:47:25]

Please comment on the intended use-case: this needs to be used in potentially
infinite loops, to limit their running time.

[rchtara, 2014/05/22 08:44:38]

Done.

+
+    Args:
+      duration: The time to wait in seconds.
+    """
+    logging.info("action: Wait %s" % duration)
+    time.sleep(duration)
+    self.remaining_time_to_wait -= 1
+    if self.remaining_time_to_wait < 0:
+      raise Exception("Tests took more time than expected for the following "
+                      "website : %s \n" % self.name)
+
+  def WaitUntilDisplayed(self, selector, timeout=10):
+    """Waits until an element is displayed.
+
+    Args:
+      selector: The element CSS selector.
+      timeout: The maximum waiting time in seconds before failing.
+    """
+    if not self.IsDisplayed(selector):
+      self.Wait(1)
+      timeout = timeout - 1
+      if (timeout <= 0):
+        raise Exception("Error: Element %s not shown before timeout is "
+                        "finished for the following website: %s"
+                        % (selector, self.name))
+      else:
+        self.WaitUntilDisplayed(selector, timeout)
+
+  # Form actions.
+
+  def FillPasswordInto(self, selector):
+    """If the testing mode is the Autofilled mode, compares the website
+    password to the DOM state.
+    If the testing mode is the NotAutofilled mode, checks that the DOM state
+    is empty.
+    Then, fills the input with the Website password.
+
+    Args:
+      selector: The password input CSS selector.
+
+    Raises:
+      Exception: An exception is raised if the DOM value of the password is
+          different than the one we expected.
+    """
+    logging.info("action: FillPasswordInto %s" % selector)
+    password_element = self.driver.find_element_by_css_selector(selector)
+    if self.mode == self.Mode.AUTOFILLED:
+      autofilled_password = password_element.get_attribute("value")
+      if autofilled_password != self.password:
+        raise Exception("Error: autofilled password is different from the one "
+                        "we just saved for the following website : %s p1: %s "
+                        "p2:%s \n" % (self.name,
+                                      password_element.get_attribute("value"),
+                                      self.password))
+
+    elif self.mode == self.Mode.NOT_AUTOFILLED:
+      autofilled_password = password_element.get_attribute("value")
+      password_element.send_keys(autofilled_password)

[vabr (Chromium), 2014/05/20 14:47:25]

Unless we raise an exception in the next step, |autofilled_password| is empty.
Why do we send the empty string to the |password_element|?

[rchtara, 2014/05/22 08:44:38]

Done.

+      if autofilled_password:
+        raise Exception("Error: password is autofilled when it shouldn't  be "
+                        "for the following website : %s \n"
+                        % self.name)
+
+      password_element.send_keys(self.password)
+
+  def FillUsernameInto(self, selector):
+    """If the testing mode is the Autofilled mode, compares the website
+    username to the input value. Then, fills the input with the website
+    username.
+
+    Args:
+      selector: The username input CSS selector.
+
+    Raises:
+      Exception: An exception is raised if the DOM value of the username is
+          different that the one we expected.
+    """
+    logging.info("action: FillUsernameInto %s" % selector)
+    username_element = self.driver.find_element_by_css_selector(selector)
+
+    if (self.mode == self.Mode.AUTOFILLED and not self.username_not_auto):
+      if not (username_element.get_attribute("value") == self.username):
+        raise Exception("Error: autofilled username is different form the one "
+                        "we just saved for the following website : %s \n" %
+                        self.name)
+      # Chrome protects the password inputs and doesn't fill them until

[vabr (Chromium), 2014/05/20 14:47:25]

This assumes that FillUsernameInto is always called before FillPasswordInto.
Because you keep these methods separate, that's not guaranteed.

My suggestion here is: do this click() in FillPasswordInto instead. You can
either pass the username CSS selector to FillPasswordInto, or you can click just
about anywhere on the page.

P.S. I really liked the comment you wrote: very clear and helpful to understand
the reason for click().

[rchtara, 2014/05/22 08:44:38]

Thanks :)

+      # the user interacts with the page. If we fill the username, we don't
+      # have this problem anymore. Otherwise, we just click on the username.
+      username_element.click()
+    else:
+      username_element.clear()
+      username_element.send_keys(self.username)
+
+  def Submit(self, selector):
+    """Finds an element using CSS Selector and calls its submit() handler.
+
+    Args:
+      selector: The input CSS selector.
+    """
+    logging.info("action: Submit %s" % selector)
+    element = self.driver.find_element_by_css_selector(selector)
+    element.submit()
+
+  # Login/Logout Methods
+
+  def Login(self):
+    """Login Method. Has to be overloaded by the WebsiteTest test."""
+    raise NotImplementedError("Login is not implemented.")
+
+  def LoginWhenAutofilled(self):
+    """Logs in and checks that the password is autofilled."""
+    self.mode = self.Mode.AUTOFILLED
+    self.Login()
+
+  def LoginWhenNotAutofilled(self):
+    """Logs in and checks that the password is not autofilled."""
+    self.mode = self.Mode.NOT_AUTOFILLED
+    self.Login()
+
+  def Logout(self):
+    """Logout Method. Has to be overloaded by the Website test."""
+    raise NotImplementedError("Logout is not implemented.")
+
+  # Tests
+
+  def WrongLoginTest(self):
+    """Does the wrong login test: Tries to login with a wrong password and
+    checks that the prompt is not shown.
+
+    Raises:
+      Exception: An exception is raised if the test fail.

[vabr (Chromium), 2014/05/20 14:47:25]

nit: fails

[vabr (Chromium), 2014/05/20 14:47:25]

Actually, "test fails" is confusing in this case: failing means succeeding,
since the test tries a wrong password.
Please re-formulate here, and below, more concretely when the exception is
raised.

[rchtara, 2014/05/22 08:44:38]

Done.

[rchtara, 2014/05/22 08:44:38]

Done.

+    """
+    logging.info("\nWrong Login Test for %s \n" % self.name)
+    correct_password = self.password
+    self.password = self.password + "1"
+    self.LoginWhenNotAutofilled()
+    self.password = correct_password
+    self.Wait(2)
+    self.environment.SwitchToInternals()
+    self.environment.CheckPrompt(
+        "Message: Decision: SAVE the password",
+        False,
+        "Error: password manager thinks that a login with wrong password was "
+        "successful for the following website : %s \n" % self.name)
+    self.environment.SwitchFromInternals()
+
+  def SuccessfulLoginTest(self):
+    """Does the successful login when the password is not expected to be
+    autofilled test: Checks that the password is not autofilled, tries to login
+    with a right password and checks if the that the prompt is shown. Then logs
+    out.
+
+    Raises:
+      Exception: An exception is raised if the test fail.
+    """
+    logging.info("\nSuccessful Login Test for %s \n" % self.name)
+    self.LoginWhenNotAutofilled()
+    self.Wait(2)
+    self.environment.SwitchToInternals()
+    self.environment.CheckPrompt(
+        "Message: Decision: SAVE the password",
+        True,
+        "Error: password manager hasn't detected a successful login for the "
+        "following website : %s \n"
+        % self.name)
+    self.environment.SwitchFromInternals()
+    self.Logout()
+
+  def SuccessfulLoginWithAutofilledPasswordTest(self):
+    """Does the successful login when the password is expected to be autofilled
+    test: Checks that the password is autofilled, tries to login with a right
+    password and checks if the that the prompt is shown. Then logs out.
+
+    Raises:
+      Exception: An exception is raised if the test fail.
+    """
+    logging.info("\nSuccessful Login With Autofilled Password"
+                        " Test %s \n" % self.name)
+    self.LoginWhenAutofilled()
+    self.Wait(2)
+    self.environment.SwitchToInternals()
+    self.environment.CheckPrompt(
+        "Message: Decision: SAVE the password",
+        True,
+        "Error: password manager hasn't detected a successful login for the "
+        "following website : %s \n"
+        % self.name)
+    self.environment.SwitchFromInternals()
+    self.Logout()
+
+  def PromptTest(self):
+    """Does the wrong login test: Tries to login with a wrong password and

[vabr (Chromium), 2014/05/20 14:47:25]

Did you forget to change the doc string?

[rchtara, 2014/05/22 08:44:38]

Done.

+    checks that the prompt is not shown.
+
+    Raises:
+      Exception: An exception is raised if the test fail.
+    """
+    logging.info("\nWrong Login Test for %s \n" % self.name)

[vabr (Chromium), 2014/05/20 14:47:25]

Did you forget to change the log message?

[rchtara, 2014/05/22 08:44:38]

Done.

+    correct_password = self.password
+    self.password = self.password + "1"
+    self.LoginWhenNotAutofilled()
+    self.password = correct_password
+    self.Wait(2)
+    self.environment.SwitchToInternals()
+    self.environment.CheckPrompt(
+        "Message: Decision: ASK the user",
+        False,
+        "Error: password manager thinks that a login with wrong password was "
+        "successful for the following website : %s \n" % self.name)
+    self.environment.SwitchFromInternals()
+
+    self.LoginWhenNotAutofilled()
+    self.Wait(2)
+    self.environment.SwitchToInternals()
+    self.environment.CheckPrompt(
+        "Message: Decision: ASK the user",
+        True,
+        "Error: password manager thinks that a login with wrong password was "
+        "successful for the following website : %s \n" % self.name)
+    self.environment.SwitchFromInternals()