Merge pull request #389 from MarketSquare/378-file-descriptor-not-closed-for-file-parameter

fix: files descriptors not closed in files param #378

Author: lucagiove

src/RequestsLibrary/RequestsKeywords.py

@@ -47,10 +47,11 @@ def _common_request(
 
         self.last_response = resp
 
-        # file descriptors should be closed for files parameter as well
-        data = kwargs.get('data', None)
-        if is_file_descriptor(data):
-            data.close()
+        files = kwargs.get('files', {}) or {}
+        data = kwargs.get('data', []) or []
+        files_descriptor_to_close = filter(is_file_descriptor, list(files.values()) + [data])
+        for file_descriptor in files_descriptor_to_close:
+            file_descriptor.close()
 
         return resp
 
@@ -59,7 +60,7 @@ def _merge_url(session, uri):
         """
         Helper method that join session url and request url.
         It relies on urljoin that handles quite good join urls and multiple /
-        but has some counter intuitive behaviours if you join uri starting with /
+        but has some counterintuitive behaviours if you join uri starting with /
         It handles also override in case a full url (http://etc) is passed as uri.
         """
         base = ''

src/RequestsLibrary/version.py

@@ -1 +1 @@
-VERSION = '0.9.6'
+VERSION = '0.9.7'

utests/test_RequestsOnSessionKeywords.py

@@ -1,9 +1,8 @@
 import os
 
 from RequestsLibrary import RequestsLibrary
-from utests import mock
-
 from utests import SCRIPT_DIR
+from utests import mock
 
 
 def build_mocked_session_common_request(alias='alias', url='http://mocking.rules',
@@ -23,6 +22,16 @@ def test_common_request_file_descriptor_closing():
         assert f.closed is True
 
 
+def test_common_request_files_descriptor_closing_when_passed_as_files_param():
+    session, m_common_request = build_mocked_session_common_request()
+    with open(os.path.join(SCRIPT_DIR, '../atests/randombytes.bin'), 'rb') as f1:
+        with open(os.path.join(SCRIPT_DIR, '../atests/data.json'), 'rb') as f2:
+            m_common_request('get', session,
+                             'http://mocking.rules', files={'randombytes': f1, 'data': f2})
+            assert f1.closed is True
+            assert f2.closed is True
+
+
 def test_common_request_verify_override_true():
     session, m_common_request = build_mocked_session_common_request(verify=False)
     m_common_request('get', session, '/', verify=True)
@@ -68,22 +77,26 @@ def test_common_request_with_cookies_default_only():
     m_common_request('get', session, '/')
     session.get.assert_called_with('http://mocking.rules/', timeout=None, cookies={'a': 1, 'b': 2})
 
+
 def test_common_request_with_float_timeout():
     session, m_common_request = build_mocked_session_common_request(timeout=123.4)
     m_common_request('get', session, '/')
     session.get.assert_called_with('http://mocking.rules/', timeout=123.4, cookies={})
 
+
 def test_common_request_with_float_timeout_override():
     session, m_common_request = build_mocked_session_common_request(timeout=None)
     m_common_request('get', session, '/', timeout=123.4)
     session.get.assert_called_with('http://mocking.rules/', timeout=123.4, cookies={})
 
+
 def test_common_request_with_touple_timeout():
     session, m_common_request = build_mocked_session_common_request(timeout=(123.4, 432.1))
     m_common_request('get', session, '/')
     session.get.assert_called_with('http://mocking.rules/', timeout=(123.4, 432.1), cookies={})
 
+
 def test_common_request_with_touple_timeout_override():
     session, m_common_request = build_mocked_session_common_request(timeout=None)
     m_common_request('get', session, '/', timeout=(123.4, 432.1))
-    session.get.assert_called_with('http://mocking.rules/', timeout=(123.4, 432.1), cookies={})
+    session.get.assert_called_with('http://mocking.rules/', timeout=(123.4, 432.1), cookies={})