Repository: EdinburghGenomics/EGCG-Core

Issue:
Deprecate get_expected_yield_for_sample
function `get_expected_yield_for_sample` in `clarity` returns the Expected yield Q30.
This function should be deprecated and a more explicitly named one should be used.
It is mainly used in Analysis-Driver (https://github.com/EdinburghGenomics/Analysis-Driver)
 

Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index 68e56d7..0292bb1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -7,7 +7,7 @@ install:
   - "pip install -r requirements.txt"
   - "pip install python-coveralls pytest-cov"
   - "python setup.py install"
-script: py.test tests/ --doctest-modules  -v --cov egcg_core --cov-report term-missing
+script: py.test tests/ --cov egcg_core --cov-report term-missing
 after_success:
   - coveralls
 # branches:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a6601ec..9adf174 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,10 @@ Changelog for EGCG-Core
 0.9 (unreleased)
 ----------------
 
-- Nothing changed yet.
+- Breaking change: removed `clarity.get_expected_yield_for_sample`
+- Catching notification failures
+- Added retries to `rest_communication`
+- Added log warnings to `util.find_file`
 
 
 0.8.2 (2018-05-28)
diff --git a/egcg_core/clarity.py b/egcg_core/clarity.py
index c3e9b86..0068e9d 100644
--- a/egcg_core/clarity.py
+++ b/egcg_core/clarity.py
@@ -206,19 +206,6 @@ def get_sample_genotype(sample_name, output_file_name):
             app_logger.warning('Cannot download genotype results for %s', sample_name)
 
 
-def get_expected_yield_for_sample(sample_name):
-    """
-    Query the LIMS and return the number of bases expected for a sample
-    :param sample_name: the sample name
-    :return: number of bases
-    """
-    sample = get_sample(sample_name)
-    if sample:
-        nb_gb = sample.udf.get('Yield for Quoted Coverage (Gb)')
-        if nb_gb:
-            return nb_gb * 1000000000
-
-
 def get_plate_id_and_well(sample_name):
     sample = get_sample(sample_name)
     if sample:
diff --git a/egcg_core/ncbi.py b/egcg_core/ncbi.py
index b6c26b0..840b8e5 100644
--- a/egcg_core/ncbi.py
+++ b/egcg_core/ncbi.py
@@ -36,6 +36,9 @@ def get_species_name(query_species):
         q, taxid, scientific_name, common_name = local_query
     else:
         taxid, scientific_name, common_name = _fetch_from_eutils(query_species)
+        if taxid is None:
+            return None
+
         _cache_species(query_species, taxid, scientific_name, common_name)
 
     return scientific_name
diff --git a/egcg_core/notifications/__init__.py b/egcg_core/notifications/__init__.py
index 17282da..10b1ae4 100644
--- a/egcg_core/notifications/__init__.py
+++ b/egcg_core/notifications/__init__.py
@@ -1,5 +1,8 @@
+import sys
+import traceback
 from egcg_core.app_logging import AppLogger
 from egcg_core.config import cfg
+from egcg_core.exceptions import EGCGError
 from .asana import AsanaNotification
 from .email import EmailNotification, EmailSender, send_email, send_html_email, send_plain_text_email
 from .log import LogNotification
@@ -16,21 +19,32 @@ class NotificationCentre(AppLogger):
         self.name = name
         self.subscribers = {}
 
-        for s in cfg.get('notifications', {}):
-            if s in self.ntf_aliases:
-                self.debug('Configuring notification for: ' + s)
-                config = cfg['notifications'][s]
-                self.subscribers[s] = self.ntf_aliases[s](name=self.name, **config)
+        for k, v in cfg.get('notifications', {}).items():
+            if k in self.ntf_aliases:
+                self.debug('Configuring notification for: ' + k)
+                self.subscribers[k] = self.ntf_aliases[k](name=self.name, **v)
             else:
-                self.warning("Bad notification config '%s' - this will be ignored", s)
+                self.warning("Bad notification config '%s' - this will be ignored", k)
 
     def notify(self, msg, subs):
+        exceptions = []
         for s in subs:
             if s in self.subscribers:
-                self.subscribers[s].notify(msg)
+                try:
+                    self.subscribers[s].notify(msg)
+                except Exception as e:
+                    etype, value, tb = sys.exc_info()
+                    stacktrace = ''.join(traceback.format_exception(etype, value, tb))
+                    self.critical(stacktrace)
+                    exceptions.append(e)
             else:
                 self.debug('Tried to notify by %s, but no configuration present', s)
 
+        if exceptions:
+            raise EGCGError(
+                'Encountered the following errors during notification: %s' % ', '.join(
+                    '%s: %s' % (e.__class__.__name__, str(e)) for e in exceptions)
+            )
+
     def notify_all(self, msg):
-        for s in self.subscribers.values():
-            s.notify(msg)
+        self.notify(msg, self.subscribers.keys())
diff --git a/egcg_core/rest_communication.py b/egcg_core/rest_communication.py
index 61dcdbf..0a9e0cc 100644
--- a/egcg_core/rest_communication.py
+++ b/egcg_core/rest_communication.py
@@ -1,7 +1,10 @@
 import json
 import mimetypes
-import requests
 from urllib.parse import urljoin
+import requests
+from multiprocessing import Lock
+from requests.packages.urllib3.util.retry import Retry
+from requests.adapters import HTTPAdapter
 from egcg_core.config import cfg
 from egcg_core.app_logging import AppLogger
 from egcg_core.exceptions import RestCommunicationError
@@ -11,9 +14,31 @@ from egcg_core.util import check_if_nested
 class Communicator(AppLogger):
     successful_statuses = (200, 201, 202, 204)
 
-    def __init__(self, auth=None, baseurl=None):
+    def __init__(self, auth=None, baseurl=None, retries=5):
         self._baseurl = baseurl
         self._auth = auth
+        self.retries = retries
+        self._session = None
+        self.lock = Lock()
+
+    def begin_session(self):
+        s = requests.Session()
+        adapter = HTTPAdapter(max_retries=Retry(self.retries, self.retries, self.retries, backoff_factor=0.2))
+        s.mount('http://', adapter)
+        s.mount('https://', adapter)
+
+        if isinstance(self.auth, tuple):
+            s.auth = self.auth
+        elif isinstance(self.auth, str):
+            s.headers.update({'Authorization': 'Token %s' % self.auth})
+
+        return s
+
+    @property
+    def session(self):
+        if self._session is None:
+            self._session = self.begin_session()
+        return self._session
 
     @staticmethod
     def serialise(queries):
@@ -103,11 +128,6 @@ class Communicator(AppLogger):
         return query
 
     def _req(self, method, url, quiet=False, **kwargs):
-        if isinstance(self.auth, tuple):
-            kwargs['auth'] = self.auth
-        elif isinstance(self.auth, str):
-            kwargs['headers'] = dict(kwargs.get('headers', {}), Authorization='Token %s' % self.auth)
-
         # can't upload json and files at the same time, so we need to move the json parameter to data
         # data can't upload complex structures that would require json encoding.
         # this means we can't upload data with nested lists/dicts at the same time as files
@@ -116,22 +136,25 @@ class Communicator(AppLogger):
                 raise RestCommunicationError('Cannot upload files and nested json in one query')
             kwargs['data'] = kwargs.pop('json')
 
-        r = requests.request(method, url, **kwargs)
+        self.lock.acquire()
+        r = self.session.request(method, url, **kwargs)
 
-        kwargs.pop('auth', None)
-        kwargs.pop('headers', None)
         kwargs.pop('files', None)
         # e.g: 'POST <url> ({"some": "args"}) -> {"some": "content"}. Status code 201. Reason: CREATED
         report = '%s %s (%s) -> %s. Status code %s. Reason: %s' % (
             r.request.method, r.request.path_url, kwargs, r.content.decode('utf-8'), r.status_code, r.reason
         )
+
         if r.status_code in self.successful_statuses:
             if not quiet:
                 self.debug(report)
+
+            self.lock.release()
+            return r
         else:
             self.error(report)
+            self.lock.release()
             raise RestCommunicationError('Encountered a %s status code: %s' % (r.status_code, r.reason))
-        return r
 
     def get_content(self, endpoint, paginate=True, quiet=False, **query_args):
         if paginate:
diff --git a/egcg_core/util.py b/egcg_core/util.py
index 6d8b08c..f8e9d1a 100644
--- a/egcg_core/util.py
+++ b/egcg_core/util.py
@@ -26,7 +26,10 @@ def find_files(*path_parts):
 
 def find_file(*path_parts):
     files = find_files(*path_parts)
-    if files:
+    nfiles = len(files)
+    if nfiles > 1:
+        app_logger.warning('Searched pattern %s for one file, but got %s', path_parts, nfiles)
+    elif nfiles == 1:
         return files[0]
 
 
@@ -102,20 +105,20 @@ def move_dir(src_dir, dest_dir):
     return 0
 
 
-def query_dict(data, query_string):
+def query_dict(data, query_string, ret_default=None):
     """
     Drill down into a dict using dot notation, e.g. query_dict({'this': {'that': 'other'}}, 'this.that'}).
     :param dict data:
     :param str query_string:
+    :param ret_default:
     """
     _data = data.copy()
 
     for q in query_string.split('.'):
         d = _data.get(q)
-        if d is None:
-            return None
-
-        else:
+        if d is not None:
             _data = d
+        else:
+            return ret_default
 
     return _data


Test patch:
diff --git a/tests/test_clarity.py b/tests/test_clarity.py
index 6b4fc43..c4446fb 100644
--- a/tests/test_clarity.py
+++ b/tests/test_clarity.py
@@ -178,11 +178,6 @@ class TestClarity(TestEGCG):
         assert open(genotype_vcf).read() == 'some test content'
         os.remove(genotype_vcf)
 
-    @patched_clarity('get_sample', Mock(udf={'Yield for Quoted Coverage (Gb)': 3}))
-    def test_get_expected_yield_for_sample(self, mocked_get_sample):
-        assert clarity.get_expected_yield_for_sample('a_sample_id') == 3000000000
-        mocked_get_sample.assert_called_with('a_sample_id')
-
     @patched_lims('get_processes', ['a_run'])
     def test_get_run(self, mocked_lims):
         assert clarity.get_run('a_run_id') == 'a_run'
diff --git a/tests/test_ncbi.py b/tests/test_ncbi.py
index a448117..287bc57 100644
--- a/tests/test_ncbi.py
+++ b/tests/test_ncbi.py
@@ -71,14 +71,20 @@ def test_cache():
     reset_cache()
 
 
-def test_get_species_name():
-    fetch = 'egcg_core.ncbi._fetch_from_eutils'
-    assert fetch_from_cache('a species') is None
-    with patch(fetch, return_value=(None, None, None)):
-        assert ncbi.get_species_name('a species') is None
-        assert fetch_from_cache('a species') is None
+@patch('egcg_core.ncbi._fetch_from_eutils')
+def test_get_species_name(mocked_fetch):
+    for table in ('species', 'aliases'):
+        ncbi.cursor.execute('SELECT * FROM ' + table)
+        assert ncbi.cursor.fetchall() == []
+
+    mocked_fetch.return_value = (None, None, None)
+    assert ncbi.get_species_name('a species') is None
+    for table in ('species', 'aliases'):
+        ncbi.cursor.execute('SELECT * FROM ' + table)
+        assert ncbi.cursor.fetchall() == []
+
     reset_cache()
-    with patch(fetch, return_value=('1337', 'Scientific name', 'a species')):
-        assert ncbi.get_species_name('a species') == 'Scientific name'
-        assert fetch_from_cache('a species') == ('a species', '1337', 'Scientific name', 'a species')
+    mocked_fetch.return_value = ('1337', 'Scientific name', 'a species')
+    assert ncbi.get_species_name('a species') == 'Scientific name'
+    assert fetch_from_cache('a species') == ('a species', '1337', 'Scientific name', 'a species')
     reset_cache()
diff --git a/tests/test_notifications.py b/tests/test_notifications.py
index a843958..99c7c76 100644
--- a/tests/test_notifications.py
+++ b/tests/test_notifications.py
@@ -52,6 +52,22 @@ class TestNotificationCentre(TestEGCG):
         for name, s in self.notification_centre.subscribers.items():
             s.notify.assert_called_with('a message')
 
+    @patch.object(n.NotificationCentre, 'critical')
+    def test_notify_failure(self, mocked_log):
+        self.notification_centre.subscribers = {
+            'asana': Mock(notify=Mock(side_effect=EGCGError('Something broke'))),
+            'email': Mock(notify=Mock(side_effect=ValueError('Something else broke')))
+        }
+        with self.assertRaises(EGCGError) as e:
+            self.notification_centre.notify('a message', ('asana', 'email'))
+
+        mocked_log.assert_called()
+        assert str(e.exception) == (
+            'Encountered the following errors during notification: '
+            'EGCGError: Something broke, '
+            'ValueError: Something else broke'
+        )
+
 
 class TestLogNotification(TestEGCG):
     def setUp(self):
diff --git a/tests/test_rest_communication.py b/tests/test_rest_communication.py
index 5fabb35..ddb3fdb 100644
--- a/tests/test_rest_communication.py
+++ b/tests/test_rest_communication.py
@@ -1,7 +1,7 @@
 import os
 import json
-import pytest
-from unittest.mock import patch
+from requests import Session
+from unittest.mock import Mock, patch, call
 from tests import FakeRestResponse, TestEGCG
 from egcg_core import rest_communication
 from egcg_core.util import check_if_nested
@@ -12,10 +12,6 @@ def rest_url(endpoint):
     return 'http://localhost:4999/api/0.1/' + endpoint + '/'
 
 
-def ppath(extension):
-    return 'egcg_core.rest_communication.Communicator.' + extension
-
-
 test_endpoint = 'an_endpoint'
 test_nested_request_content = {'data': ['some', {'test': 'content'}]}
 test_flat_request_content = {'key1': 'value1', 'key2': 'value2'}
@@ -33,7 +29,7 @@ def fake_request(method, url, **kwargs):
     return FakeRestResponse(test_nested_request_content)
 
 
-patched_response = patch('requests.request', side_effect=fake_request)
+patched_request = patch.object(Session, 'request', side_effect=fake_request)
 auth = ('a_user', 'a_password')
 
 
@@ -41,6 +37,17 @@ class TestRestCommunication(TestEGCG):
     def setUp(self):
         self.comm = rest_communication.Communicator(auth=auth, baseurl='http://localhost:4999/api/0.1')
 
+    def test_begin_session(self):
+        s = self.comm.begin_session()
+        assert s.adapters['http://'] is s.adapters['https://']
+        assert s.auth == auth
+        assert s.params == {}
+
+        hashed_token = '{"some": "hashed"}.tokenauthentication'
+        self.comm._auth = hashed_token
+        s = self.comm.begin_session()
+        assert s.headers['Authorization'] == 'Token ' + hashed_token
+
     def test_api_url(self):
         assert self.comm.api_url('an_endpoint') == rest_url('an_endpoint')
 
@@ -53,13 +60,13 @@ class TestRestCommunication(TestEGCG):
             'this': 'that', 'other': '{"another":"more"}', 'things': '1'
         }
 
-        with pytest.raises(RestCommunicationError) as e:
+        with self.assertRaises(RestCommunicationError) as e:
             self.comm._parse_query_string(dodgy_query_string)
-            assert str(e) == 'Bad query string: ' + dodgy_query_string
+            assert str(e.exception) == 'Bad query string: ' + dodgy_query_string
 
-        with pytest.raises(RestCommunicationError) as e2:
+        with self.assertRaises(RestCommunicationError) as e2:
             self.comm._parse_query_string(query_string, requires=['thangs'])
-            assert str(e2) == query_string + " did not contain all required fields: ['thangs']"
+            assert str(e2.exception) == query_string + " did not contain all required fields: ['thangs']"
 
     def test_detect_files_in_json(self):
         json_no_files = {'k1': 'v1', 'k2': 'v2'}
@@ -81,32 +88,70 @@ class TestRestCommunication(TestEGCG):
         ]
         assert obs_json == [{'k1': 'v1'}, {'k1': 'v1'}]
 
-    @patched_response
-    def test_req(self, mocked_response):
+    @patched_request
+    def test_req(self, mocked_request):
         json_content = ['some', {'test': 'json'}]
-        response = self.comm._req('METHOD', rest_url(test_endpoint), json=json_content)
-        assert response.status_code == 200
-        assert json.loads(response.content.decode('utf-8')) == response.json() == test_nested_request_content
-        mocked_response.assert_called_with('METHOD', rest_url(test_endpoint), auth=auth, json=json_content)
-
-    def test_get_documents_depaginate(self):
-        docs = (
+        for i in range(4):
+            response = self.comm._req('METHOD', rest_url(test_endpoint), json=json_content)
+            assert response.status_code == 200
+            assert json.loads(response.content.decode('utf-8')) == response.json() == test_nested_request_content
+            mocked_request.assert_called_with('METHOD', rest_url(test_endpoint), json=json_content)
+
+    @patch.object(Session, '__exit__')
+    @patch.object(Session, '__enter__')
+    @patched_request
+    def test_context_manager(self, mocked_request, mocked_enter, mocked_exit):
+        json_content = ['some', {'test': 'json'}]
+        with self.comm.session:
+            mocked_enter.assert_called_once()
+            mocked_exit.assert_not_called()
+            for i in range(4):  # multiple calls
+                response = self.comm._req('METHOD', rest_url(test_endpoint), json=json_content)
+                assert response.status_code == 200
+                assert response.json() == test_nested_request_content
+                mocked_request.assert_called_with('METHOD', rest_url(test_endpoint), json=json_content)
+
+        assert mocked_request.call_count == 4
+        mocked_exit.assert_called_once()
+
+    @patch.object(rest_communication.Communicator, 'error')
+    @patch.object(Session, 'request')
+    def test_communication_error(self, mocked_req, mocked_log):
+        response = FakeRestResponse({})
+        response.status_code = 500
+        mocked_req.return_value = response
+        self.comm.lock = Mock()
+        self.comm.lock.acquire.assert_not_called()
+        self.comm.lock.release.assert_not_called()
+
+        with self.assertRaises(RestCommunicationError) as e:
+            self.comm.get_document('an_endpoint')
+
+        assert mocked_log.call_args[0][0].endswith('Status code 500. Reason: a reason')
+        assert str(e.exception) == 'Encountered a 500 status code: a reason'
+        self.comm.lock.acquire.assert_called_once()
+        self.comm.lock.release.assert_called_once()  # exception raised, but lock still released
+
+    @patch.object(rest_communication.Communicator, '_req')
+    def test_get_documents_depaginate(self, mocked_req):
+        mocked_req.side_effect = (
             FakeRestResponse({'data': ['this', 'that'], '_links': {'next': {'href': 'an_endpoint?max_results=101&page=2'}}}),
             FakeRestResponse({'data': ['other', 'another'], '_links': {'next': {'href': 'an_endpoint?max_results=101&page=3'}}}),
             FakeRestResponse({'data': ['more', 'things'], '_links': {}})
         )
-        with patch(ppath('_req'), side_effect=docs) as mocked_req:
-            assert self.comm.get_documents('an_endpoint', all_pages=True, max_results=101) == [
-                'this', 'that', 'other', 'another', 'more', 'things'
-            ]
-            assert all([a[0][1].startswith(rest_url('an_endpoint')) for a in mocked_req.call_args_list])
-            assert [a[1] for a in mocked_req.call_args_list] == [
+
+        assert self.comm.get_documents('an_endpoint', all_pages=True, max_results=101) == [
+            'this', 'that', 'other', 'another', 'more', 'things'
+        ]
+        mocked_req.assert_has_calls(
+            (
                 # Communicator.get_content passes ints
-                {'params': {'page': 1, 'max_results': 101}, 'quiet': False},
+                call('GET', rest_url('an_endpoint'), params={'page': 1, 'max_results': 101}, quiet=False),
                 # url parsing passes strings, but requests removes the quotes anyway
-                {'params': {'page': '2', 'max_results': '101'}, 'quiet': False},
-                {'params': {'page': '3', 'max_results': '101'}, 'quiet': False}
-            ]
+                call('GET', rest_url('an_endpoint'), params={'page': '2', 'max_results': '101'}, quiet=False),
+                call('GET', rest_url('an_endpoint'), params={'page': '3', 'max_results': '101'}, quiet=False)
+            )
+        )
 
         docs = [
             FakeRestResponse(
@@ -119,38 +164,37 @@ class TestRestCommunication(TestEGCG):
         ]
         docs.append(FakeRestResponse({'data': ['last piece'], '_links': {}}))
 
-        with patch(ppath('_req'), side_effect=docs):
-            ret = self.comm.get_documents('an_endpoint', all_pages=True, max_results=101)
-            assert len(ret) == 1200
+        mocked_req.side_effect = docs
+        ret = self.comm.get_documents('an_endpoint', all_pages=True, max_results=101)
+        assert len(ret) == 1200
 
-    @patched_response
-    def test_get_content(self, mocked_response):
+    @patched_request
+    def test_get_content(self, mocked_request):
         data = self.comm.get_content(test_endpoint, max_results=100, where={'a_field': 'thing'})
         assert data == test_nested_request_content
-        assert mocked_response.call_args[0][1].startswith(rest_url(test_endpoint))
-        assert mocked_response.call_args[1] == {
-            'auth': ('a_user', 'a_password'),
-            'params': {'max_results': 100, 'where': '{"a_field": "thing"}', 'page': 1}
-        }
+        mocked_request.assert_called_with(
+            'GET',
+            rest_url(test_endpoint),
+            params={'max_results': 100, 'where': '{"a_field": "thing"}', 'page': 1}
+        )
 
     def test_get_documents(self):
-        with patched_response:
+        with patched_request:
             data = self.comm.get_documents(test_endpoint, max_results=100, where={'a_field': 'thing'})
             assert data == test_nested_request_content['data']
 
     def test_get_document(self):
         expected = test_nested_request_content['data'][0]
-        with patched_response:
+        with patched_request:
             observed = self.comm.get_document(test_endpoint, max_results=100, where={'a_field': 'thing'})
             assert observed == expected
 
-    @patched_response
-    def test_post_entry(self, mocked_response):
+    @patched_request
+    def test_post_entry(self, mocked_request):
         self.comm.post_entry(test_endpoint, payload=test_nested_request_content)
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'POST',
             rest_url(test_endpoint),
-            auth=auth,
             json=test_nested_request_content,
             files=None
         )
@@ -158,39 +202,35 @@ class TestRestCommunication(TestEGCG):
         test_request_content_plus_files = dict(test_flat_request_content)
         test_request_content_plus_files['f'] = ('file', file_path)
         self.comm.post_entry(test_endpoint, payload=test_request_content_plus_files)
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'POST',
             rest_url(test_endpoint),
-            auth=auth,
             data=test_flat_request_content,
             files={'f': (file_path, b'test content', 'text/plain')}
         )
 
         self.comm.post_entry(test_endpoint, payload=test_flat_request_content, use_data=True)
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'POST',
             rest_url(test_endpoint),
-            auth=auth,
             data=test_flat_request_content,
             files=None
         )
 
         self.comm.post_entry(test_endpoint, payload=test_request_content_plus_files, use_data=True)
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'POST',
             rest_url(test_endpoint),
-            auth=auth,
             data=test_flat_request_content,
             files={'f': (file_path, b'test content', 'text/plain')}
         )
 
-    @patched_response
-    def test_put_entry(self, mocked_response):
+    @patched_request
+    def test_put_entry(self, mocked_request):
         self.comm.put_entry(test_endpoint, 'an_element_id', payload=test_nested_request_content)
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'PUT',
             rest_url(test_endpoint) + 'an_element_id',
-            auth=auth,
             json=test_nested_request_content,
             files=None
         )
@@ -199,17 +239,16 @@ class TestRestCommunication(TestEGCG):
         test_request_content_plus_files = dict(test_flat_request_content)
         test_request_content_plus_files['f'] = ('file', file_path)
         self.comm.put_entry(test_endpoint, 'an_element_id', payload=test_request_content_plus_files)
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'PUT',
             rest_url(test_endpoint) + 'an_element_id',
-            auth=auth,
             data=test_flat_request_content,
             files={'f': (file_path, b'test content', 'text/plain')}
         )
 
-    @patch(ppath('get_document'), return_value=test_patch_document)
-    @patched_response
-    def test_patch_entry(self, mocked_response, mocked_get_doc):
+    @patch.object(rest_communication.Communicator, 'get_document', return_value=test_patch_document)
+    @patched_request
+    def test_patch_entry(self, mocked_request, mocked_get_doc):
         patching_payload = {'list_to_update': ['another']}
         self.comm.patch_entry(
             test_endpoint,
@@ -220,72 +259,58 @@ class TestRestCommunication(TestEGCG):
         )
 
         mocked_get_doc.assert_called_with(test_endpoint, where={'uid': 'a_unique_id'})
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'PATCH',
             rest_url(test_endpoint) + '1337',
             headers={'If-Match': 1234567},
-            auth=auth,
             json={'list_to_update': ['this', 'that', 'other', 'another']},
             files=None
         )
 
-    @patch(ppath('get_document'), return_value=test_patch_document)
-    @patched_response
-    def test_auth_token_and_if_match(self, mocked_response, mocked_get_doc):
-        self.comm._auth = 'an_auth_token'
+    @patch.object(rest_communication.Communicator, 'get_document', return_value=test_patch_document)
+    @patched_request
+    def test_if_match(self, mocked_request, mocked_get_doc):
         self.comm.patch_entry(test_endpoint, {'this': 'that'}, 'uid', 'a_unique_id')
         mocked_get_doc.assert_called_with(test_endpoint, where={'uid': 'a_unique_id'})
-        mocked_response.assert_called_with(
+        mocked_request.assert_called_with(
             'PATCH',
             rest_url(test_endpoint) + '1337',
-            headers={'If-Match': 1234567, 'Authorization': 'Token an_auth_token'},
+            headers={'If-Match': 1234567},
             json={'this': 'that'},
             files=None
         )
 
-    def test_post_or_patch(self):
+    @patch.object(rest_communication.Communicator, 'post_entry', return_value=True)
+    @patch.object(rest_communication.Communicator, '_patch_entry', return_value=True)
+    @patch.object(rest_communication.Communicator, 'get_document')
+    def test_post_or_patch(self, mocked_get, mocked_patch, mocked_post):
         test_post_or_patch_payload = {'uid': '1337', 'list_to_update': ['more'], 'another_field': 'that'}
         test_post_or_patch_payload_no_uid = {'list_to_update': ['more'], 'another_field': 'that'}
         test_post_or_patch_doc = {
             'uid': 'a_uid', '_id': '1337', '_etag': 1234567, 'list_to_update': ['things'], 'another_field': 'this'
         }
-        patched_post = patch(ppath('post_entry'), return_value=True)
-        patched_patch = patch(ppath('_patch_entry'), return_value=True)
-        patched_get = patch(ppath('get_document'), return_value=test_post_or_patch_doc)
-        patched_get_none = patch(ppath('get_document'), return_value=None)
-
-        with patched_get as mget, patched_patch as mpatch:
-            self.comm.post_or_patch(
-                'an_endpoint',
-                [test_post_or_patch_payload],
-                id_field='uid',
-                update_lists=['list_to_update']
-            )
-            mget.assert_called_with('an_endpoint', where={'uid': '1337'})
-            mpatch.assert_called_with(
-                'an_endpoint',
-                test_post_or_patch_doc,
-                test_post_or_patch_payload_no_uid,
-                ['list_to_update']
-            )
+        mocked_get.return_value = test_post_or_patch_doc
 
-        with patched_get_none as mget, patched_post as mpost:
-            self.comm.post_or_patch(
-                'an_endpoint', [test_post_or_patch_payload], id_field='uid', update_lists=['list_to_update']
-            )
-            mget.assert_called_with('an_endpoint', where={'uid': '1337'})
-            mpost.assert_called_with('an_endpoint', test_post_or_patch_payload)
+        self.comm.post_or_patch(
+            'an_endpoint',
+            [test_post_or_patch_payload],
+            id_field='uid',
+            update_lists=['list_to_update']
+        )
+        mocked_get.assert_called_with('an_endpoint', where={'uid': '1337'})
+        mocked_patch.assert_called_with(
+            'an_endpoint',
+            test_post_or_patch_doc,
+            test_post_or_patch_payload_no_uid,
+            ['list_to_update']
+        )
 
-    def test_token_auth(self):
-        hashed_token = '{"some": "hashed"}.tokenauthentication'
-        self.comm._auth = hashed_token
-        with patched_response as p:
-            self.comm._req('GET', self.comm.baseurl + 'an_endpoint')
-            p.assert_called_with(
-                'GET',
-                self.comm.baseurl + 'an_endpoint',
-                headers={'Authorization': 'Token ' + hashed_token}
-            )
+        mocked_get.return_value = None
+        self.comm.post_or_patch(
+            'an_endpoint', [test_post_or_patch_payload], id_field='uid', update_lists=['list_to_update']
+        )
+        mocked_get.assert_called_with('an_endpoint', where={'uid': '1337'})
+        mocked_post.assert_called_with('an_endpoint', test_post_or_patch_payload)
 
 
 def test_default():
diff --git a/tests/test_util.py b/tests/test_util.py
index fe703e8..dd2348c 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -2,8 +2,9 @@ import hashlib
 from os import makedirs, symlink
 from shutil import rmtree
 from os.path import join, basename
-from tests import TestEGCG
+from unittest.mock import patch
 from egcg_core import util
+from tests import TestEGCG
 
 fastq_dir = join(TestEGCG.assets_path, 'fastqs')
 
@@ -13,8 +14,14 @@ def test_find_files():
     assert util.find_files(TestEGCG.assets_path, 'ftest*.txt') == expected
 
 
-def test_find_file():
+@patch('logging.Logger.warning')
+def test_find_file(mocked_log):
     assert util.find_file(TestEGCG.assets_path, 'ftest.txt') == join(TestEGCG.assets_path, 'ftest.txt')
+    assert util.find_file(TestEGCG.assets_path, 'ftest_.txt') is None
+    assert util.find_file(TestEGCG.assets_path, 'ftest*.txt') is None
+    mocked_log.assert_called_with(
+        'Searched pattern %s for one file, but got %s', (TestEGCG.assets_path, 'ftest*.txt'), 2
+    )
 
 
 def test_str_join():
@@ -87,10 +94,10 @@ class TestMoveDir(TestEGCG):
         self._create_test_file(join(self.test_dir, 'exists', 'ftest.txt'), 'another file')
 
     def tearDown(self):
-        rmtree(util.find_file(self.test_dir, 'to'))
-        rmtree(util.find_file(self.test_dir, 'from'))
-        rmtree(util.find_file(self.test_dir, 'exists'))
-        rmtree(util.find_file(self.test_dir, 'external'))
+        for base in ('to', 'from', 'exists', 'external'):
+            f = util.find_file(self.test_dir, base)
+            if f:
+                rmtree(f)
 
     def test_move_dir(self):
         frm = join(self.test_dir, 'from')
@@ -108,7 +115,7 @@ class TestMoveDir(TestEGCG):
 
         assert util.find_file(to, 'external_renamed.txt')
 
-    def _move_dir_exists(self):
+    def test_move_dir_exists(self):
         frm = join(self.test_dir, 'from')
         to = join(self.test_dir, 'exists')
         md5_from1 = self._md5(join(frm, 'ftest.txt'))
@@ -132,3 +139,4 @@ def test_query_dict():
     assert util.query_dict(data, 'this') == {'that': 'other'}
     assert util.query_dict(data, 'this.that') == 'other'
     assert util.query_dict(data, 'nonexistent') is None
+    assert util.query_dict(data, 'nonexistent', ret_default='things') == 'things'