Merge branch 'coverage_2' into 'client_api'

Better coverage

See merge request !3

.coveragerc.p2

+[report]
+exclude_lines =
+	pragma: no cover
+	raise NotImplementedError
+	def __download_file_p3

.coveragerc.p3

+[report]
+exclude_lines =
+	pragma: no cover
+	raise NotImplementedError
+	def __download_file_p2

.gitlab-ci.yml

@@ -38,14 +38,14 @@ tests:2.7:
     - pwd
   script:
     - python -V
-    - python -m pytest --pep8 --cov=allgo tests/
+    - python -m pytest --pep8 --cov=allgo  --cov-config=.coveragerc.p2 tests/
 
 # ----------------------------
 tests:3.7:
   stage: test
   script:
     - python -V
-    - python -m pytest --pep8 --cov=allgo tests/
+    - python -m pytest --pep8 --cov=allgo  --cov-config=.coveragerc.p3 tests/
 
 # ----------------------------
 sphinx:

allgo/__init__.py

@@ -106,7 +106,7 @@ class StatusError(AllgoError):
         return self._response.json()['error']
 
 
-def _local_token():
+def _local_token():  # pragma: no cover
     """read and return the content of TOKEN_FILE or None."""
     if os.path.exists(TOKEN_FILE):
         with open(TOKEN_FILE) as f:
@@ -438,7 +438,7 @@ class Client:
 
         # method with os.fdopen(os.open(outfilepath, mode), 'w') doesn't exist
         if os.path.isfile(outfilepath) and not force:
-            msg = "file {} exists."
+            msg = "file {} exists.".format(outfilepath)
             raise OSError(errno.EEXIST, msg)
 
         try:
@@ -523,7 +523,6 @@ class Client:
                 return out_dict
 
         # should never goes here
-        return out_dict
 
     def get_log(self, job_id):
         """Generator of log file content.

tests/test_allgo.py

 #! /usr/bin/env python3
 
+"""Unit tests for Allgo module.
+
+    Notes:
+        cf https://docs.pytest.org/en/latest/contents.html
+        tmp_path is a pytest fixture.
+"""
 # standard lib
 import errno
 import os
@@ -31,7 +37,6 @@ class TestStatusError:
             { 'error': self.REF_MSG }
         return m
 
-    @pytest.mark.dbg
     def test_msg(self, response):
         err = StatusError(42, response)
         assert self.REF_MSG == err.msg
@@ -221,9 +226,84 @@ def test_create_job__param_error(mock_post, client):
     # ~ assert err.msg.startswith('Invalid parameters')
 
 
+@pytest.mark.success
+@patch('allgo.Client._request_api')
+def test_create_job__params_default(mock_request, client, tmp_path):
+    """Here I'll check the params transmission."""
+    app_name = 'fake'
+    client.create_job(app_name)
+
+    data = {  # default values, except for app_name
+        "job[webapp_id]"  : app_name,
+        "job[param]"      : '',
+        "job[version]"    : None,
+        "job[queue]"      : None,
+    }
+    file_dict = {}
+
+    # -- tests
+    mock_request.assert_called_once_with('post',
+                                         url_end='jobs',
+                                         data=data,
+                                         files=file_dict)
+
+
+@pytest.mark.success
+@patch('allgo.Client._request_api')
+def test_create_job__params(mock_request, client, tmp_path):
+    """Here I'll check the params transmission.
+        For that I'll mock the _request_api so that it returns
+        the params = data and files dict."""
+    # Mock
+    def side_effect(method, url_end, **kwargs):
+        return kwargs
+    mock_request.side_effect = side_effect
+
+    # create files to load
+    filepaths = []
+    for i, f in enumerate(['f1','f2','f3']):
+        filepath = os.path.join(str(tmp_path), f )
+        with open(filepath, "w") as fs:
+            fs.write(f)  # content doesn't matter
+        filepaths.append(filepath)
+
+    app_name = 'fake'
+    params   = 'un de kvar'
+    version  = '1.42'
+    queue    = 'quick'
+    transmitted_params = client.create_job(app_name,
+                                           version=version,
+                                           params=params,
+                                           files=filepaths,
+                                           queue=queue)
+    # expected
+    data = {  # default values, except for app_name
+        "job[webapp_id]"  : app_name,
+        "job[param]"      : params,
+        "job[version]"    : version,
+        "job[queue]"      : queue,
+    }
+    file_dict_keys = [ "file[{}]".format(i) for i, _ in enumerate(filepaths) ]
+
+    # -- tests
+    assert 'data' in transmitted_params
+    assert data == transmitted_params['data']
+    assert 'files' in transmitted_params
+    assert sorted(file_dict_keys) == sorted(transmitted_params['files'].keys())
+
+    if 2 == sys.version_info.major:
+        file_type = file
+    else:
+        from io import BufferedReader
+        file_type = BufferedReader
+    for handle in transmitted_params['files'].values():
+        assert isinstance(handle, file_type)
+        handle.close()
+
+
 @pytest.mark.success
 @patch('allgo.requests.post')
-def test_create_job(mock_post, client):
+def test_create_job__response(mock_post, client):
     # -- mock
     job_id  = 33
     job_url = "http://job_url"
@@ -382,6 +462,34 @@ def test_run_job__args(mock_sleep, mock_status, mock_create, client, capsys):
     assert x_output == capsys.readouterr().out
 
 
+# ------------------------------------------------
+@pytest.mark.error
+@patch('allgo.requests.get')
+def test__request_file(mock_get, client):
+    """assert a StatusError is raised if the request fail.
+        The error message is not checked."""
+    # -- mock
+    err_code = 666
+    mock_get.return_value = Mock(status_code=err_code)
+    # ~ mock_get.return_value.json.return_value = \
+    # ~     {'error': 'API request without http authorisation'}
+
+    # -- tests
+    with pytest.raises(StatusError) as err_info:
+        client._request_file('url_bidon')
+
+    err = err_info.value
+    assert err_code == err.status_code
+    # ~ assert '' == err.msg
+
+
+@pytest.mark.error
+def test_app_metrics(client):
+    """app_metrics method is not yet implemented."""
+    with pytest.raises(NotImplementedError):
+        client.app_metrics('fake', 'what')
+
+
 # ------------------------------------------------
 def __mock_file(mock_get):
     file_content = "fake line1.\nline2 is fake too.\n"
@@ -397,12 +505,13 @@ def __mock_file(mock_get):
 @pytest.mark.success
 @patch('allgo.requests.get')
 def test_download_file__default(mock_get, client):
-    """The method returns the filepath where the file has been written."""
+    """The method returns the filepath where the file has been written,
+        outdir is the localdir by default."""
     # -- mock
     file_content = __mock_file(mock_get)
 
     # -- tests
-    filename = tempfile.mktemp(dir='')
+    filename = 'wanted_file'
     file_url = "https://whatever.fr/" + filename
     filepath = client.download_file(file_url)
 
@@ -437,10 +546,50 @@ def test_download_file__outdir(mock_get, client, tmp_path):
     assert file_content == content
 
 
+@pytest.mark.error
+@patch('allgo.requests.get')
+def test_download_file__outdir_already_exists(mock_get, client, tmp_path):
+    """The method should not raise an error if the output dir already exist."""
+    # -- mock
+    __mock_file(mock_get)
+
+    # -- tests
+    file_url = "https://whatever.fr/x"
+    try:
+        client.download_file(file_url, outdir=tmp_path)
+    except Exception:
+        pytest.fail("In download_file, existing outdir should not raise an error.")
+
+
+@pytest.mark.error
+@patch('allgo.os.makedirs')
+@patch('allgo.requests.get')
+def test_download_file__outdir_error(mock_get, mock_mkdir, client, tmp_path):
+    """The method should raise an error if the output dir cannot be created."""
+    # -- mock
+    __mock_file(mock_get)
+
+    err_code = errno.EPERM
+    err_msg  = 'fake msg'
+    mock_mkdir.side_effect = OSError(err_code, err_msg)
+
+    # -- tests
+    file_url = "https://whatever.fr/x"
+    with pytest.raises(OSError) as err_info:
+        client.download_file(file_url, outdir=tmp_path)
+
+    err = err_info.value
+    assert err_code == err.errno
+    assert err_msg  == err.strerror
+
+
 @pytest.mark.success
 @patch('allgo.requests.get')
 def test_download_file__force(mock_get, client):
-    """The method returns the filepath where the file has been written."""
+    """If the output file already exists,
+        the force param, False by default result in an OSError,
+        if force is set to True, no error should be raised.
+    """
     # -- mock
     file_content = __mock_file(mock_get)
 
@@ -453,7 +602,7 @@ def test_download_file__force(mock_get, client):
     # default is to raise an exception if the file exists.
     # ~ with pytest.raises(FileExistsError): # python3
     with pytest.raises(OSError) as err_info:
-        filepath = client.download_file(file_url)  # default is force=False
+        filepath = client.download_file(file_url, force=False)  # default
 
     err = err_info.value
     assert errno.EEXIST == err.errno
@@ -567,3 +716,30 @@ def test_get_log__big(mock_get, mock_status, client):
 
 
 # ------------------------------------------------
+@pytest.mark.success
+@patch('allgo.requests.get')
+def test__request_api__manage_ValueErrors(mock_get, client):
+    """_request_api method should manage well error that can occurr in querying.
+        if ValueError the response should be returned.
+
+        ValueError happens when response content cannot be converted to json."""
+    # -- mock
+    response = mock_get.return_value
+    response.status_code = 200
+    response.json = Mock(side_effect=ValueError())
+
+    # -- tests
+    resp = client._request_api('get', 'fin_url')
+    assert response == resp
+
+
+@pytest.mark.error
+def test__request_api__manage_KeyErrors(client):
+    """_request_api method should manage well error that can occurr in querying.
+        if KeyError is raised, an AllgoError should be raised.
+
+        KeyError happens when an unknown actions is used as param."""
+
+    # -- tests
+    with pytest.raises(AllgoError):
+        client._request_api('fail', 'fin_url')

tests/test_integration.py

@@ -223,7 +223,7 @@ class TestAllgoOld(ApiClientIntegration):
     @classmethod
     def _get_job_id(cls):
         """return a job id that exist, with a log file named allgo.log"""
-        return 71771
+        return 72597
 
     def setup_class(cls):
         """define the client used in local instance.