[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 98%
copy from chrome/test/chromedriver/test/run_py_tests.py
copy to chrome/test/chromedriver/client/run_py_tests.py
index 2b106a6783a372544520a9fcae0f182b673be9e2..7368bc5fc4eb77b9718ca9cfe7201927bd34365b 100755
--- a/chrome/test/chromedriver/test/run_py_tests.py
+++ b/chrome/test/chromedriver/client/run_py_tests.py
@@ -140,7 +140,9 @@ _ANDROID_NEGATIVE_FILTER['chromedriver_webview_shell'] = (
 
 class ChromeDriverBaseTest(unittest.TestCase):
   """Base class for testing chromedriver functionalities."""
-
+  
+  download_dir = tempfile.mkdtemp(dir='/tmp')

[samuong, 2014/10/01 20:22:12]

This is going to be used by all tests, which is not necessary.

Could you create a subclass class of ChromeDriverBaseTest (maybe call it
ChromeDriverDownloadTest?) which contains just the testFileDownload() test, and
only set the download_dir for that class?

[andrewcheng, 2014/10/08 22:08:26]

def setUp(self):
    if self._testMethodName == 'testFileDownLoad':
       self.download_dir = tempfile.mkdtemp(dir='/tmp')
    else:
       self.download_dir =  None

Done.

+  
   def __init__(self, *args, **kwargs):
     super(ChromeDriverBaseTest, self).__init__(*args, **kwargs)
     self._drivers = []
@@ -165,7 +167,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, self.download_dir, 
                                        chrome_binary=_CHROME_BINARY,
                                        android_package=android_package,
                                        android_activity=android_activity,
@@ -177,7 +179,7 @@ class ChromeDriverBaseTest(unittest.TestCase):
 
 class ChromeDriverTest(ChromeDriverBaseTest):
   """End to end tests for ChromeDriver."""
-
+  
   @staticmethod
   def GlobalSetUp():
     ChromeDriverTest._http_server = webserver.WebServer(
@@ -208,6 +210,15 @@ class ChromeDriverTest(ChromeDriverBaseTest):
   def testStartStop(self):
     pass
 
+  def testFileDownLoad(self):
+    self._driver.Load(self.GetHttpUrlForFile('/chromedriver/download.html'))

[samuong, 2014/10/01 20:22:12]

Before clicking, please also add a check that ensures that the file is *not* in
the download directory.

[andrewcheng, 2014/10/08 22:08:26]

Done.

[andrewcheng, 2014/10/08 22:08:26]

   This is a random generated dir - very unlikely,  a download dir will  exist. 
However, we can put a check as following, in case it does exist

    self.assertFalse(os.path.isfile(download_name), 
             "Test user prefer download directory failed! - download file
already exist")

+    self._driver.FindElement('id', 'red-dot').Click()
+    time.sleep(0.5)

[samuong, 2014/10/01 20:22:13]

I'm a bit worried that this is going to be flaky, and also this test will always
take >0.5 seconds. Could you change this to a loop (with a timeout) that waits
until the file exists?

[andrewcheng, 2014/10/08 22:08:26]

    download_name = ChromeDriverBaseTest.download_dir + "/a_red_dot.png";
    timeout = time.time() + 60*1   # 1 minutes from now
    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 di rectory failed!")
Done.

+    download_name = ChromeDriverBaseTest.download_dir+"/a_red_dot.png";

[samuong, 2014/10/01 20:22:12]

minor style nit: please put spaces around the +

We try to follow the Google Python Style Guide
(http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Whitespace) with
a few exceptions (http://www.chromium.org/chromium-os/python-style-guidelines).

[andrewcheng, 2014/10/08 22:08:26]

Done.

+    self.assertTrue(os.path.isfile(download_name), "Test user prefer download directory failed!")

[samuong, 2014/10/01 20:22:12]

Another style nit: please keep lines within 80 characters. The style guide sets
a maximum width of 80 characters and there are presubmits which will block the
commit if code exceeds this.

[andrewcheng, 2014/10/08 22:08:26]

Done.

+    os.remove(download_name)
+    os.removedirs(ChromeDriverBaseTest.download_dir)    

[samuong, 2014/10/01 20:22:12]

If we add another test that uses the download directory, and it runs after this
test, it will fail because the directory does not exist. Please move this into
the tearDown() function (and call tempfile.mkdtemp() in the setUp() function).

[andrewcheng, 2014/10/08 22:08:26]

Done.

 

  def tearDown(self):
    try:
       if self.download_dir:
          os.removedirs(self.download_dir)
          self.download_dir = None

+        
   def testLoadUrl(self):
     self._driver.Load(self.GetHttpUrlForFile('/chromedriver/empty.html'))