Password Manager testing automation

components/test/data/password_manager/environment.py

+import time
+from selenium import webdriver
+from selenium.webdriver.chrome.options import Options
+import xml.etree.ElementTree as ET

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

Please don't use ET, use the original name, to increase readability.

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

Done.

+
+from website import Website
+
+
+class Environment:
+
+  def __init__(self, passwords_file = ""):

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

No spaces around "=" when defining default values.
http://legacy.python.org/dev/peps/pep-0008/#id20

Please correct also elsewhere in this CL.

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

Done.

+    options = Options()
+    """Use random empty folder for profile, otherwise chromedriver is going to
+    create it's own where saving passwords is disabled.
+    """
+    options.add_argument("enable-automatic-password-saving")

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

Please comment on why this is needed.
(I still know why :), but other people reading it might not.)

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

Done.

+    #chrome path

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

Please separate # and the first word with a space. (Also below.)

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

Done.

+    options.binary_location = (
+        "/usr/local/google/home/rchtara/chrome/src/out/Debug/chrome")

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

Please do not hard-code your local path into the uploaded script.
Ideally make it a command-line option, or allow specifying a configuration file
from which to read it.

This holds also for the profile and webdriver path below.

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

Done.

+    #new profile path
+    options.add_argument("user-data-dir=/tmp/ChromePasswordManagerTestvvvvvv")
+    #chromedriver path
+    self.driver = webdriver.Chrome(
+        "/usr/local/google/home/rchtara/chrome/src/out/Debug/chromedriver", 0,
+        options)
+    self.internals_window = self.driver.current_window_handle
+    self.websitewindow = None
+    self.websites = []
+
+    self.passwords_tree = None

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

Please comment on what is the |passwords_tree|.

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

Done.

+    if passwords_file != "":
+      self.passwords_tree = ET.parse(passwords_file).getroot()
+
+  def AddWebsite(self,

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

Please add a docstring documenting this method,
and all other public methods you define.

+                 name,
+                 username = None,
+                 password = None,
+                 username_not_auto=False):
+    website = (
+        Website(
+            name, username, password, self.driver, self, username_not_auto))
+    self.websites.append(website)
+    return website
+
+  def RemoveAllPasswords(self, websites):
+    print "RemoveAllPasswords"
+    self.driver.get("chrome://settings/passwords")
+    self.driver.switch_to_frame("settings")
+
+    for website in websites:
+      urls, _ = self.GetPasswordsAndButtons()
+      website.RemoveAllPasswords(urls)
+
+  def GetPasswordsAndButtons(self):
+    arr = self.driver.find_elements_by_css_selector(
+        "#saved-passwords-list .deletable-item")
+    urls = []
+    remove_buttons = []
+    for entry in arr:
+      urls.append(entry.find_element_by_class_name("url").text)
+      remove_buttons.append(
+          entry.find_element_by_class_name("row-delete-button"))
+    return (urls, remove_buttons)

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

Why do you bother with returning buttons, when you ignore them at the only place
when this method is called?

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

Done.

+
+  def NewWindow(self):
+    if self.websitewindow == None:
+      self.driver.get("https://google.com")
+
+      """It's not possible to open a new tab to chrome:// urls directly,

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

Sorry, this sentence is hard to parse, and I actually don't understand what is
going on here.

Please make each point one sentence, describe what is the main goal here, what
does not work, and what is a work-around.

Also, if you find that Chromedriver is not accepting chrome:// URLs, it would be
useful to know if that's a known limitation. When not, then ideally file a bug
(if Chromedriver developers accept bugreports), and add a tracking link here, so
that the work-around can be removed in the future.

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

There is two issues here:
*There is no straightforward way to open a new tab with chromedriver.
One of work-around is to go to a website, insert a link that is going to be
opened in a new tab, click on it. To avoid that the chrome popup blocker blocks
the new tab, we need to choose a trusted website which is google.com.
*Links for local resources (chrome://*) are blocked. So this why the 
new tab is going to be for google.com and then the browser is asked
via chromedriver to go to chrome://settings/passwords

+      this why we go to google.com website then we go to chrome://
+      sometime chromedriver doesn't detect that www.google.com is loaded
+      and the script just stops, this is why you should redo the
+      tests again.
+      """
+
+      a = self.driver.execute_script(
+          "var d=document; var "
+          "a=d.createElement('a');a.target='_blank';a.href=arguments[0];"
+          "a.innerHTML='.';d.body.appendChild(a);"
+          " return a;",
+          "https://google.com")
+      a.click()
+      time.sleep(1)
+
+      self.websitewindow = self.driver.window_handles[-1]
+
+      self.driver.switch_to_window(self.websitewindow)
+
+  def TestList(self, websites):
+    self.RemoveAllPasswords(websites)
+    self.NewWindow()
+    for website in websites:
+      website.WrongLoginTest()
+      website.SuccessfulLoginTest()
+      website.SuccessfulLoginWithAutofilledPasswordTest()
+
+    self.RemoveAllPasswords(websites)
+    for website in websites:
+      website.SuccessfulLoginAfterDeletionTest()
+
+  def AllTests(self):
+    self.TestList(self.websites)
+
+  def Test(self, tests):
+    websites = []
+    for website in self.websites:
+      for test in tests:

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

Use rather:
if website.name in tests:
  websites.append(website)

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

Done.

+        if website.name == test:
+          websites.append(website)
+    self.TestList(websites)
+
+  def Quit(self):
+    self.driver.quit()
+
+