Error when closing browser did prevent cache clearing (robotframework#1383)

aaltat · web-flow · commit f663ed3d084a · 2019-04-30T21:17:59.000+01:00
Fixes robotframework#1379
diff --git a/src/SeleniumLibrary/keywords/webdrivertools.py b/src/SeleniumLibrary/keywords/webdrivertools.py
@@ -19,6 +19,7 @@
 
 from robot.utils import ConnectionCache
 from selenium import webdriver
+from selenium.common.exceptions import WebDriverException
 
 from SeleniumLibrary.utils import is_falsy, is_truthy, is_noney
 
@@ -220,17 +221,29 @@ def active_drivers(self):
     def close(self):
         if self.current:
             driver = self.current
-            driver.quit()
+            error = self._quit(driver, None)
             self.current = self._no_current
             self._closed.add(driver)
+            if error:
+                raise error
 
     def close_all(self):
+        error = None
         for driver in self._connections:
             if driver not in self._closed:
-                driver.quit()
+                error = self._quit(driver, error)
         self.empty_cache()
+        if error:
+            raise error
         return self.current
 
+    def _quit(self, driver, error):
+        try:
+            driver.quit()
+        except WebDriverException as exception:
+            error = exception
+        return error
+
     def get_index(self, alias_or_index):
         index = self._get_index(alias_or_index)
         try:
diff --git a/utest/test/keywords/test_webdrivercache.py b/utest/test/keywords/test_webdrivercache.py
@@ -1,12 +1,17 @@
 import unittest
 
-from mockito import mock, verify
+from mockito import mock, verify, when, unstub
+from robot.utils.connectioncache import NoConnection
+from selenium.common.exceptions import TimeoutException, RemoteDriverServerException
 
 from SeleniumLibrary.keywords import WebDriverCache
 
 
 class WebDriverCacheTests(unittest.TestCase):
 
+    def tearDown(self):
+        unstub()
+
     def test_no_current_message(self):
         cache = WebDriverCache()
         try:
@@ -157,3 +162,71 @@ def test_same_alias_new_browser(self):
         cache.close()
         index = cache.get_index('bar')
         self.assertEqual(index, None)
+
+    def test_close_all_cache_first_quite_fails(self):
+        cache = WebDriverCache()
+        driver = mock()
+        when(driver).quit().thenRaise(TimeoutException('timeout.'))
+        cache.register(driver, 'bar')
+        with self.assertRaises(TimeoutException):
+            cache.close_all()
+        self.verify_cache(cache)
+
+    def test_close_all_cache_middle_quite_fails(self):
+        cache = WebDriverCache()
+        driver0, driver1, driver2 = mock(), mock(), mock()
+        when(driver0).quit().thenReturn(None)
+        when(driver1).quit().thenRaise(TimeoutException('timeout.'))
+        when(driver2).quit().thenReturn(None)
+        cache.register(driver0, 'bar0')
+        cache.register(driver1, 'bar1')
+        cache.register(driver2, 'bar2')
+        with self.assertRaises(TimeoutException):
+            cache.close_all()
+        self.verify_cache(cache)
+
+    def test_close_all_cache_all_quite_fails(self):
+        cache = WebDriverCache()
+        driver0, driver1, driver2 = mock(), mock(), mock()
+        when(driver0).quit().thenRaise(RemoteDriverServerException('stuff.'))
+        when(driver1).quit().thenRaise(RemoteDriverServerException('stuff.'))
+        when(driver2).quit().thenRaise(TimeoutException('timeout.'))
+        cache.register(driver0, 'bar0')
+        cache.register(driver1, 'bar1')
+        cache.register(driver2, 'bar2')
+        with self.assertRaises(TimeoutException):
+            cache.close_all()
+        self.verify_cache(cache)
+
+    def test_close_all_no_error(self):
+        cache = WebDriverCache()
+        driver0, driver1, driver2 = mock(), mock(), mock()
+        when(driver0).quit().thenReturn(None)
+        when(driver1).quit().thenReturn(None)
+        when(driver2).quit().thenReturn(None)
+        cache.close_all()
+        self.verify_cache(cache)
+
+    def test_close_quite_fails(self):
+        cache = WebDriverCache()
+        driver = mock()
+        when(driver).quit().thenRaise(TimeoutException('timeout.'))
+        cache.register(driver, 'bar')
+        with self.assertRaises(TimeoutException):
+            cache.close()
+        self.assertTrue(isinstance(cache.current, NoConnection))
+        self.assertTrue(driver in cache._closed)
+
+    def test_close_no_error(self):
+        cache = WebDriverCache()
+        driver = mock()
+        when(driver).quit().thenReturn(None)
+        cache.register(driver, 'bar')
+        cache.close()
+        self.assertTrue(isinstance(cache.current, NoConnection))
+        self.assertTrue(driver in cache._closed)
+
+    def verify_cache(self, cache):
+        self.assertEqual(cache._connections, [])
+        self.assertEqual(cache._aliases, {})
+        self.assertTrue(isinstance(cache.current, NoConnection))
