Password Manager testing automation

components/test/data/password_manager/action.py

+import time

[vabr (Chromium), 2014/05/08 13:53:45]

Please add a module docstring (also in other Python files).
http://legacy.python.org/dev/peps/pep-0008/#id24

[vabr (Chromium), 2014/05/08 13:53:45]

Also please add the encoding, like you did in tests.py, in all other Python
files, unless it is ASCII.
http://legacy.python.org/dev/peps/pep-0008/#id16

[rchtara, 2014/05/14 11:58:04]

Done.

+from selenium.webdriver.common.action_chains import ActionChains
+from selenium.webdriver.common.keys import Keys
+from selenium.common.exceptions import ElementNotVisibleException
+
+
+class Action:
+

[vabr (Chromium), 2014/05/08 13:53:45]

Please add a docstring documenting this class.
Also add it for all other classes you define.

+  def __init__(self, action_type, param, website):
+    self.action_type = action_type
+    self.param = param
+    self.website = website
+
+  def Do(self, driver):
+    """Do the default behavior of the action.
+    """

[vabr (Chromium), 2014/05/08 13:53:45]

'For one liner docstrings, please keep the closing """ on the same line.'
http://legacy.python.org/dev/peps/pep-0008/#id24

(For multi-line docstrings, having the closing """ on a separate line is OK.)

[rchtara, 2014/05/14 11:58:04]

Done.

+    print "actiontype: %s actionparam: %s" % (self.action_type, self.param)

[vabr (Chromium), 2014/05/08 13:53:45]

If you need this temporarily as a debugging message, please add a TODO(rchtara):
to remove it as soon as possible.
In general, we try to avoid spamming the output, so that debug messages we care
about at the moment are easily visible.

Ditto for all the uses of "print" below.

[rchtara, 2014/05/14 11:58:04]

I think it's important to put the log, because the tests crash quite often. And
the only way to figure out when and why the crash has happen is by reading log
msg. 

+    if self.action_type == "goto":
+      driver.get(self.param)
+    elif self.action_type == "wait":
+      time.sleep(self.param)
+    elif self.action_type == "click":
+      element = driver.find_element_by_css_selector(self.param)
+      element.click()
+    elif self.action_type == "hover":
+      element = driver.find_element_by_css_selector(self.param)
+      hover = ActionChains(driver).move_to_element(element)
+      hover.perform()
+    elif self.action_type == "submit":
+      element = driver.find_element_by_css_selector(self.param)
+      element.submit()
+    elif self.action_type == "enter":
+      body = driver.find_element_by_tag_name("body")
+      body.send_keys(Keys.ENTER)
+    elif self.action_type == "optinalfillusername":

[vabr (Chromium), 2014/05/08 13:53:45]

typo: optinal -> optional

[rchtara, 2014/05/14 11:58:04]

Done.

+      username_element = driver.find_element_by_css_selector(self.param)
+      print username_element
+      try:
+        username_element.clear()
+        username_element.send_keys(self.website.username)
+      except ElementNotVisibleException:
+        pass
+    elif self.action_type == "fillusername":
+      username_element = driver.find_element_by_css_selector(self.param)
+      username_element.clear()
+      username_element.send_keys(self.website.username)
+    elif self.action_type == "fillpassword":
+      """Chrome protects the password inputs and doesn't fill them until

[vabr (Chromium), 2014/05/08 13:53:45]

As already mentioned off-line: except for doc-strings for public modules,
functions, classes, and methods, do not use string literals as block comments.
Use standard comments, starting the line with a "# ".

[rchtara, 2014/05/14 11:58:04]

Done.

+      the user interacts with them. So when using chromedriver, we need
+      to be careful about that: we need to send a key to password
+      element, chrome is going to autofill it if he has already saved
+      password for it, then we need to clear everything and send the
+      password.
+      """
+      password_element = driver.find_element_by_css_selector(self.param)
+      password_element.send_keys("a")

[vabr (Chromium), 2014/05/08 13:53:45]

Why do we actually care if Chrome fills the password, when we overwrite it
instantly?

[rchtara, 2014/05/14 11:58:04]

Chrome doesn't fill password input until the user interacts with it.
When we send "a", chrome is going to fill the input with the saved password 
then adds "a" at the end.
We then clear the password. And then we can send whatever we like.
If we just send the password (even if we clear the input before that) , and
there is a saved password in chrome, the input will be filled by a concatenation
of both passwords.

+      password_element.clear()
+      password_element.send_keys(self.website.password)
+    elif self.action_type == "wait":
+      time.sleep(self.param)

[vabr (Chromium), 2014/05/08 13:53:45]

You already have one if-branch for "wait". Please remove this one.

[rchtara, 2014/05/14 11:58:04]

Done.

+
+  def DoAutofilled(self, driver):
+    """Make the action behavior when the  username and password fields are

[vabr (Chromium), 2014/05/08 13:53:45]

nit: Please use single-space consistently, no double-space between words.

[vabr (Chromium), 2014/05/08 13:53:45]

I cannot parse the sentence. Are you trying to say the following?
"For the actions fillusername and fillpassword, check that the state of the DOM
already corresponds to the result of intended action.
For other actions, just perform Do()."

[rchtara, 2014/05/14 11:58:04]

Done.

[rchtara, 2014/05/14 11:58:04]

Done.

+    expected to be autofilled: for fillusername amd fillpassword, check that
+    the autofilled values are equal to the ones we expect. For other actions,
+    do the default behavior.
+

[vabr (Chromium), 2014/05/08 13:53:45]

nit: Please remove the blank line.

[rchtara, 2014/05/14 11:58:04]

Done.

+    """
+    if (self.action_type == "fillusername" and
+        not self.website.username_not_auto):
+      print "actiontype: %s actionparam: %s" % (self.action_type, self.param)
+      username_element = driver.find_element_by_css_selector(self.param)
+      assert username_element.get_attribute("value") == self.website.username, (

[vabr (Chromium), 2014/05/08 13:53:45]

The Chromium style guide for Python
(http://dev.chromium.org/developers/coding-style#TOC-Python) says:
"Note that asserts are of limited use, and should not be used for validating
input – throw an exception instead."
Here you are validating input -- the displayed page, so you should use an
exception.

Ditto below.

[rchtara, 2014/05/14 11:58:04]

Done.

+          "Error: autofilled "
+          "username is different form the one we just saved for the "
+          "following website :  %s \n" % self.website.Url())
+    elif self.action_type == "fillpassword":
+      print "actiontype: %s actionparam: %s" % (self.action_type, self.param)
+      """Chrome protects the password inputs and doesn't fill them until

[vabr (Chromium), 2014/05/08 13:53:45]

Actually, some errors I saw in the past were when the password value was not
accessible even after the user submitted the form.

I don't think that "Chrome does not fill password inputs until the user
interacts with them" is correct -- mostly, Chrome should autofill the passwords,
and make them accessible on any user interaction with any part of the page.
There are exceptions -- PSL-matched passwords (should not be a concern of these
tests), and forms in iframes (that could happen). In that case, a user
interaction with the _username_ input (not password input) should be enough to
trigger autocomplete.

In other words, filling the username in the username input completely should
always guarantee the password being autofilled and accessible. If not, that's a
bug.

Could we make the test not to overwrite the autofilled passwords, to also catch
such bugs?

[rchtara, 2014/05/14 11:58:04]

I made the test and you right: the password is autofilled after any kind of
interaction with the page.
So I will change the comment, but not the code:
*sometimes we dont have to fill the user name, getting the password is going to
be our first interaction
*I tried to click on the password via chromedriver, but i go problem with some
with some website, because the password field is hidden.
So let's keep the code 

+      the user interacts with them. So when using chromedriver, if we
+      want to get the stored password, we need to be careful about that:
+      we need to send a key to password element, chrome is going to
+      autofill it if he has already saved password for it, then we need
+      to get the value of the input and we remove the character we add
+      it before.
+      """
+      password_element = driver.find_element_by_css_selector(self.param)
+      password_element.send_keys("a")
+      ps = password_element.get_attribute("value")[:-1]
+      password_element.clear()
+      password_element.send_keys(ps)
+      assert password_element.get_attribute("value") == self.website.password, (
+          "Error: autofilled password is different from the one we just saved "
+          "for the following website :  %s p1: %s p2:%s \n" %
+          (self.website.Url(), password_element.get_attribute("value"),
+           self.website.password))
+    else:
+      self.Do(driver)
+
+  def DoNotAutofilled(self, driver):
+    """Do the action behavior when the password field is not expected to be

[vabr (Chromium), 2014/05/08 13:53:45]

Please once you improve the docstring for the previous method, phrase this one
accordingly.

[rchtara, 2014/05/14 11:58:04]

Done.

[rchtara, 2014/05/14 11:58:04]

Done.

+    autofilled: for fillpassword check that the password field is empty before
+    filling it. For other actions, do the default behavior.
+    """
+    if self.action_type == "fillpassword":
+      """Chrome protects the password inputs and doesn't fill them until
+      the user interacts with them. So when using chromedriver, if we
+      want to get the stored password, we need to be careful about that:
+      we need to send a key to password element, chrome is going to
+      autofill it if he has already saved password for it, then we need
+      to get the value of the input and we remove the character we add
+      it before.
+      """
+      password_element = driver.find_element_by_css_selector(self.param)
+      password_element.send_keys("a")
+      ps = password_element.get_attribute("value")[1:]
+      password_element.clear()
+      password_element.send_keys(ps)
+
+      assert len(ps) == 0, ("Error: password is autofilled when it shouldn't "

[vabr (Chromium), 2014/05/08 13:53:45]

len(ps) == 0
should be just
ps
(Look for "len(" in http://legacy.python.org/dev/peps/pep-0008/#id41.)

Other than that, exception instead of an assert please, as noted above.

[rchtara, 2014/05/14 11:58:04]

Done.

+                            " be for the following website : %s \n"
+                            % self.website.Url())
+
+    self.Do(driver)