[chromedriver] setting browser default download directory

chrome/test/chromedriver/client/run_py_tests.py

Index: chrome/test/chromedriver/client/run_py_tests.py
diff --git a/chrome/test/chromedriver/test/run_py_tests.py b/chrome/test/chromedriver/client/run_py_tests.py
similarity index 94%
copy from chrome/test/chromedriver/test/run_py_tests.py
copy to chrome/test/chromedriver/client/run_py_tests.py
index 2b106a6783a372544520a9fcae0f182b673be9e2..dbed62ace8d4c9ea942f73b83593ef812c18e549 100755
--- a/chrome/test/chromedriver/test/run_py_tests.py
+++ b/chrome/test/chromedriver/client/run_py_tests.py
@@ -152,7 +152,7 @@ class ChromeDriverBaseTest(unittest.TestCase):
       except:
         pass
 
-  def CreateDriver(self, server_url=None, **kwargs):
+  def CreateDriver(self, server_url=None, download_dir=None, **kwargs):
     if server_url is None:
       server_url = _CHROMEDRIVER_SERVER_URL
 
@@ -165,7 +165,7 @@ class ChromeDriverBaseTest(unittest.TestCase):
         android_activity = constants.PACKAGE_INFO[_ANDROID_PACKAGE_KEY].activity
         android_process = '%s:main' % android_package
 
-    driver = chromedriver.ChromeDriver(server_url,
+    driver = chromedriver.ChromeDriver(server_url, download_dir,
                                        chrome_binary=_CHROME_BINARY,
                                        android_package=android_package,
                                        android_activity=android_activity,
@@ -203,11 +203,43 @@ class ChromeDriverTest(ChromeDriverBaseTest):
     return ChromeDriverTest._http_server.GetUrl() + file_path
 
   def setUp(self):
-    self._driver = self.CreateDriver()
+    if self._testMethodName == 'testFileDownLoad':
+       self.download_dir = tempfile.mkdtemp(dir='/tmp')

[samuong, 2014/10/15 22:08:18]

By default, tempfile.mkdtemp() will use dir='/tmp', so you don't need to specify
this explicitly. Just call "tempfile.mkdtemp()".

+    else:
+       self.download_dir =  None
+    self._driver = self.CreateDriver(download_dir=self.download_dir)
+
+  def tearDown(self):
+    try:
+       if self.download_dir:
+          os.removedirs(self.download_dir)
+          self.download_dir = None
+    except:
+        pass

[samuong, 2014/10/15 22:08:17]

If and when this fails, let's print out a proper error message rather than just
silently swallowing the exception.

+    ChromeDriverBaseTest.tearDown(self)
 
   def testStartStop(self):
     pass
 
+  def testFileDownLoad(self):
+    # check download file to make sure it does  not exist
+    download_name = self.download_dir + "/a_red_dot.png";

[samuong, 2014/10/15 22:08:18]

Windows uses "\" as the directory separator, so you can use os.path.join instead
of doing this manually:

download_name = os.path.join(self.download_dir, 'a_red_dot.png')

+    self.assertFalse(os.path.isfile(download_name),

[samuong, 2014/10/15 22:08:18]

If there's a directory called "a_red_dot.png" then this will cause the download
to fail. So let's use "os.path.exists" instead of "os.path.isfile".

+       "Test prefer download directory failed! - download file already exist")
+    #
+    self._driver.Load(self.GetHttpUrlForFile('/chromedriver/download.html'))
+    self._driver.FindElement('id', 'red-dot').Click()
+    # wait until download completed
+    timeout = time.time() + 60*1   # 1 minutes from now

[samuong, 2014/10/15 22:08:18]

How about we call this variable "deadline" instead of "timeout"? A timeout is a
relative time (e.g. 60 seconds) whereas a deadline is an absolute time (e.g. now
+ 60 seconds). This looks like a deadline not a timeout.

+    while True:
+       time.sleep(0.1)
+       if os.path.isfile(download_name) or time.time() > timeout:
+             break
+    #
+    self.assertTrue(os.path.isfile(download_name),
+            "Test user prefer download directory failed!")
+    os.remove(download_name)
+
   def testLoadUrl(self):
     self._driver.Load(self.GetHttpUrlForFile('/chromedriver/empty.html'))
 
@@ -750,6 +782,34 @@ class ChromeDriverAndroidTest(ChromeDriverBaseTest):
     self._drivers[0].Quit()
     self._drivers[0] = self.CreateDriver()
 
+class ChromeDownloadDirTest(ChromeDriverBaseTest):
+
+  # test existing  prefence profile - check setting if it is correct
+  def testDownloadDirectoryOverridesExistingPreferences(self):
+    userDataDir='/usr/local/google/home/andrewcheng/zz'

[samuong, 2014/10/15 22:08:17]

Please don't hardcode your home directory into test cases like this. This is
going to break when running on everyone else's computers, as well as on the
tester bots. Use tempfile.mkdtemp() to create a temporary directory to store
this in.

You'll also need to add the following line so that the subdirectory that
contains the Preferences file exists before the test tries to write to it:

os.mkdir(os.path.join(userDataDir, 'Default'))

+    defPrefFile = userDataDir+'/Default/Preferences'

[samuong, 2014/10/15 22:08:17]

For both defPrefFile and userDataDir, use names with underscores rather than
camelCase (see
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Naming).

Also please put spaces around binary operators such as '=' and '+' (see
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Whitespace).

You don't need to read and memorize the style guide from top to bottom, but
please at least look in the file that you're editing and stay as consistent with
the existing style as you can.

+    #

[samuong, 2014/10/15 22:08:18]

Also for consistency with the existing style in this file, use a blank line
instead of an empty comment.

+    data = {}
+    test_dir = {}
+    test_dir['directory_upgrade'] =  'true'
+    test_dir['default_directory'] = '/old/download/directory'
+    data['test'] =  'this should not be changed'
+    data['download'] = test_dir

[samuong, 2014/10/15 22:08:18]

How about we change these lines above so that data is declared as below?

data = {
  'test': 'this should not be changed',
  'download': {
    'directory_upgrade': 'true',
    'default_directory': '/old/download/directory'
  }
}

I think this will be easier to read since the indenting makes the nesting
clearer.

+    #
+    with open(defPrefFile, 'w') as outfile:
+      json.dump(data, outfile)
+
+    driver = self.CreateDriver(chrome_switches=
+            ['user-data-dir='+userDataDir],
+            download_dir="/usr/local/google/home/andrewcheng/zz")

[samuong, 2014/10/15 22:08:17]

You're setting the user data directory to the same directory as the download
directory. Please create two separate temporary directories at the start of the
test and use different directories for these two things.

+
+    with open(defPrefFile) as inFile: data = json.load(inFile)

[samuong, 2014/10/15 22:08:18]

Please break the above line up into two lines:

with open(defPrefFile) as inFile:
  data = json.load(inFile)

+    self.assertEqual(data['test'], 'this should not be changed',

[samuong, 2014/10/15 22:08:17]

Before doing this assert, please also check that the preferences file contains a
'test' key:

self.assertTrue('test' in data, 'existing preference has been deleted')

+                      "test data has changed")

[samuong, 2014/10/15 22:08:18]

"test data has changed" doesn't really tell anyone what has gone wrong here. How
about we change this to the following?

"existing preference has been changed to %s" % data['test']

+    download = data['download']
+    self.assertEqual(download['default_directory'],
+         '/usr/local/google/home/andrewcheng/zz',
+          "default_directory is not set")
 
 class ChromeSwitchesCapabilityTest(ChromeDriverBaseTest):
   """Tests that chromedriver properly processes chromeOptions.args capabilities.