[chromedriver] setting browser default download directory

chrome/test/chromedriver/client/run_py_tests.py

 #!/usr/bin/env python

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

This file is still in the wrong location. Could you please move it back to
chrome/test/chromedriver/test? The command to do this is:

git mv -f chrome/test/chromedriver/client/run_py_tests.py
chrome/test/chromedriver/test

Next time you do a "git cl upload", please check under "Patch Set 7" that this
file is under chrome/test/chromedriver/test.

 # Copyright 2013 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.
 
 """End to end tests for ChromeDriver."""
 
 import base64
 import json
 import math
 import optparse
@@ skipping 133 matching lines @@
     super(ChromeDriverBaseTest, self).__init__(*args, **kwargs)
     self._drivers = []
 
   def tearDown(self):
     for driver in self._drivers:
       try:
         driver.Quit()
       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
 
     android_package = None
     android_activity = None
     android_process = None
     if _ANDROID_PACKAGE_KEY:
       android_package = constants.PACKAGE_INFO[_ANDROID_PACKAGE_KEY].package
       if _ANDROID_PACKAGE_KEY == 'chromedriver_webview_shell':
         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,
                                        android_process=android_process,
                                        **kwargs)
     self._drivers += [driver]
     return driver
 
 
 class ChromeDriverTest(ChromeDriverBaseTest):
@@ skipping 17 matching lines @@
   def GlobalTearDown():
     if _ANDROID_PACKAGE_KEY:
       forwarder.Forwarder.UnmapAllDevicePorts(ChromeDriverTest._device)
     ChromeDriverTest._http_server.Shutdown()
 
   @staticmethod
   def GetHttpUrlForFile(file_path):
     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'))
 
   def testGetCurrentWindowHandle(self):
     self._driver.GetCurrentWindowHandle()
 
   def _WaitForNewWindow(self, old_handles):
     """Wait for at least one new window to show up in 20 seconds.
 
     Args:
@@ skipping 522 matching lines @@
     except urllib2.URLError as e:
       print 'Unable to fetch current version info from omahaproxy (%s)' % e
 
   def testDeviceManagement(self):
     self._drivers = [self.CreateDriver() for x in
                      android_commands.GetAttachedDevices()]
     self.assertRaises(chromedriver.UnknownError, self.CreateDriver)
     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.
 
   Makes sure the switches are passed to Chrome.
   """
 
   def testSwitchWithoutArgument(self):
     """Tests that switch --dom-automation can be passed to Chrome.
 
@@ skipping 408 matching lines @@
 
   all_tests_suite = unittest.defaultTestLoader.loadTestsFromModule(
       sys.modules[__name__])
   tests = unittest_util.FilterTestSuite(all_tests_suite, options.filter)
   ChromeDriverTest.GlobalSetUp()
   MobileEmulationCapabilityTest.GlobalSetUp()
   result = unittest.TextTestRunner(stream=sys.stdout, verbosity=2).run(tests)
   ChromeDriverTest.GlobalTearDown()
   MobileEmulationCapabilityTest.GlobalTearDown()
   sys.exit(len(result.failures) + len(result.errors))