diff --git a/jenkins/__init__.py b/jenkins/__init__.py index 7ebdbea..1b56150 100755 --- a/jenkins/__init__.py +++ b/jenkins/__init__.py @@ -542,13 +542,13 @@ class Jenkins(object): # when accessing .text property return response - def _request(self, req): + def _request(self, req, stream=None): r = self._session.prepare_request(req) # requests.Session.send() does not honor env settings by design # see https://github.com/requests/requests/issues/2807 _settings = self._session.merge_environment_settings( - r.url, {}, None, self._session.verify, None) + r.url, {}, stream, self._session.verify, None) _settings['timeout'] = self.timeout return self._session.send(r, **_settings) @@ -559,7 +559,14 @@ class Jenkins(object): ''' return self.jenkins_request(req, add_crumb, resolve_auth).text - def jenkins_request(self, req, add_crumb=True, resolve_auth=True): + def jenkins_open_stream(self, req, add_crumb=True, resolve_auth=True): + '''Return the HTTP response body from a ``requests.Request``. + + :returns: ``stream`` + ''' + return self.jenkins_request(req, add_crumb, resolve_auth, True) + + def jenkins_request(self, req, add_crumb=True, resolve_auth=True, stream=None): '''Utility routine for opening an HTTP request to a Jenkins server. :param req: A ``requests.Request`` to submit. @@ -567,6 +574,7 @@ class Jenkins(object): before submitting. Defaults to ``True``. :param resolve_auth: If True, maybe add authentication. Defaults to ``True``. + :param stream: If True, return a stream :returns: A ``requests.Response`` object. ''' try: @@ -576,7 +584,7 @@ class Jenkins(object): self.maybe_add_crumb(req) return self._response_handler( - self._request(req)) + self._request(req, stream)) except req_exc.HTTPError as e: # Jenkins's funky authentication means its nigh impossible to @@ -719,23 +727,20 @@ class Jenkins(object): :param name: Job name, ``str`` :param number: Build number, ``str`` (also accepts ``int``) :param artifact: Artifact relative path, ``str`` - :returns: artifact to download, ``dict`` + :returns: artifact to download, ``bytes`` """ folder_url, short_name = self._get_job_folder(name) try: - response = self.jenkins_open(requests.Request( - 'GET', self._build_url(BUILD_ARTIFACT, locals()))) - - if response: - return json.loads(response) - else: - raise JenkinsException('job[%s] number[%s] does not exist' % (name, number)) + with self.jenkins_open_stream(requests.Request( + 'GET', self._build_url(BUILD_ARTIFACT, locals()))) as response: + if response.encoding is None: + return response.raw.read() + else: + return response.text.encode(response.encoding) + raise JenkinsException('job[%s] number[%s] does not exist' % (name, number)) except requests.exceptions.HTTPError: raise JenkinsException('job[%s] number[%s] does not exist' % (name, number)) - except ValueError: - raise JenkinsException( - 'Could not parse JSON info for job[%s] number[%s]' % (name, number)) except NotFoundException: # This can happen if the artifact is not found return None diff --git a/tests/test_build.py b/tests/test_build.py index acd3a58..c833c43 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -1,7 +1,7 @@ import json import collections -from mock import patch +from mock import patch, Mock import jenkins from tests.base import JenkinsTestBase @@ -723,32 +723,57 @@ class JenkinsBuildTestReportUrlTest(JenkinsTestBase): class JenkinsBuildArtifactUrlTest(JenkinsTestBase): - @patch.object(jenkins.Jenkins, 'jenkins_open') + def streamMock(self, encoding=None, text=None, binary=None): + streamMock = Mock() + streamMock.__exit__ = Mock() + streamMock.__enter__ = Mock() + if encoding is None and text is None and binary is None: + streamMock.__enter__.return_value = None + return streamMock + streamMock.__enter__.return_value = Mock() + streamMock.__enter__.return_value.encoding = encoding + streamMock.__enter__.return_value.text = text + streamMock.__enter__.return_value.raw = Mock() + streamMock.__enter__.return_value.raw.read = Mock() + streamMock.__enter__.return_value.raw.read.return_value = binary + return streamMock + + @patch.object(jenkins.Jenkins, 'jenkins_open_stream') def test_simple(self, jenkins_mock): - jenkins_mock.return_value = '{}' + jenkins_mock.return_value = self.streamMock('utf-8', 'ascii') ret = self.j.get_build_artifact(u'Test Job', number='52', artifact="filename") - self.assertEqual(ret, json.loads(jenkins_mock.return_value)) + self.assertEqual(ret, b'ascii') self.assertEqual( jenkins_mock.call_args[0][0].url, self.make_url('job/Test%20Job/52/artifact/filename')) self._check_requests(jenkins_mock.call_args_list) - @patch.object(jenkins.Jenkins, 'jenkins_open') + @patch.object(jenkins.Jenkins, 'jenkins_open_stream') + def test_simple_binary(self, jenkins_mock): + jenkins_mock.return_value = self.streamMock(binary=b'\0\1\2') + ret = self.j.get_build_artifact(u'Test Job', number='52', artifact="filename") + self.assertEqual(ret, b'\0\1\2') + self.assertEqual( + jenkins_mock.call_args[0][0].url, + self.make_url('job/Test%20Job/52/artifact/filename')) + self._check_requests(jenkins_mock.call_args_list) + + @patch.object(jenkins.Jenkins, 'jenkins_open_stream') def test_in_folder(self, jenkins_mock): - jenkins_mock.return_value = '{}' + jenkins_mock.return_value = self.streamMock('utf-8', 'ascii') ret = self.j.get_build_artifact(u'a Folder/Test Job', number='52', artifact="file name") - self.assertEqual(ret, json.loads(jenkins_mock.return_value)) + self.assertEqual(ret, b'ascii') self.assertEqual( jenkins_mock.call_args[0][0].url, self.make_url('job/a%20Folder/job/Test%20Job/52/artifact/file%20name')) self._check_requests(jenkins_mock.call_args_list) - @patch.object(jenkins.Jenkins, 'jenkins_open') + @patch.object(jenkins.Jenkins, 'jenkins_open_stream') def test_matrix(self, jenkins_mock): - jenkins_mock.return_value = '{}' + jenkins_mock.return_value = self.streamMock('utf-8', 'ascii') ret = self.j.get_build_artifact(u'a Folder/Test Job', number='52/index=matrix', artifact="file name") - self.assertEqual(ret, json.loads(jenkins_mock.return_value)) + self.assertEqual(ret, b'ascii') self.assertEqual( jenkins_mock.call_args[0][0].url, self.make_url('job/a%20Folder/job/Test%20Job/52/index=matrix/artifact/file%20name')) @@ -763,10 +788,9 @@ class JenkinsBuildArtifactUrlTest(JenkinsTestBase): ret = self.j.get_build_artifact(u'TestJob', number='52', artifact="filename") self.assertIsNone(ret) - @patch.object(jenkins.Jenkins, 'jenkins_open') + @patch.object(jenkins.Jenkins, 'jenkins_open_stream') def test_open_return_none(self, jenkins_mock): - jenkins_mock.return_value = None - + jenkins_mock.return_value = self.streamMock() with self.assertRaises(jenkins.JenkinsException) as context_manager: self.j.get_build_artifact(u'TestJob', number='52', artifact="filename") self.assertEqual( @@ -774,17 +798,6 @@ class JenkinsBuildArtifactUrlTest(JenkinsTestBase): 'job[TestJob] number[52] does not exist') self._check_requests(jenkins_mock.call_args_list) - @patch.object(jenkins.Jenkins, 'jenkins_open') - def test_return_invalid_json(self, jenkins_mock): - jenkins_mock.return_value = 'Invalid JSON' - - with self.assertRaises(jenkins.JenkinsException) as context_manager: - self.j.get_build_artifact(u'TestJob', number='52', artifact="filename") - self.assertEqual( - str(context_manager.exception), - 'Could not parse JSON info for job[TestJob] number[52]') - self._check_requests(jenkins_mock.call_args_list) - @patch('jenkins.requests.Session.send', autospec=True) def test_raise_HTTPError(self, session_send_mock): session_send_mock.side_effect = iter([