Merge pull request #9 from cslzchen/fix/svcs-475-metadata-logging

[SVCS-475] Fix minor bugs and improve style

tests/core/fixtures.py

@@ -1,19 +1,15 @@
 import time
+from http import HTTPStatus
 from unittest import mock
 
 import pytest
 
-from tests.utils import MockCoroutine
 from waterbutler.core.path import WaterButlerPath
 from waterbutler.core.log_payload import LogPayload
-from tests.providers.osfstorage.fixtures import (
-    file_metadata_object,
-    file_path,
-    file_metadata,
-    file_lineage,
-    provider,
-    auth
-)
+
+from tests.utils import MockCoroutine
+from tests.providers.osfstorage.fixtures import (auth, file_path, file_lineage, provider,
+                                                 file_metadata_object, file_metadata)
 
 
 @pytest.fixture
@@ -195,16 +191,16 @@ def mock_time(monkeypatch):
     monkeypatch.setattr(time, 'time', mock_time)
 
 
-class MockResponse():
-    status = 200
-    read = MockCoroutine(return_value=b'{"status": "success"}')
+@pytest.fixture
+def mock_signed_request():
+    return MockCoroutine(return_value=MockResponse())
 
 
-class MockBadResponse():
-    status = 500
-    read = MockCoroutine(return_value=b'{"status": "failure"}')
+class MockResponse:
+    status = HTTPStatus.OK
+    read = MockCoroutine(return_value=b'{"status": "success"}')
 
 
-@pytest.fixture
-def mock_signed_request():
-    return MockCoroutine(return_value=MockResponse())
+class MockBadResponse:
+    status = HTTPStatus.INTERNAL_SERVER_ERROR
+    read = MockCoroutine(return_value=b'{"status": "failure"}')

tests/core/test_remote_logging.py

@@ -6,25 +6,12 @@
 from waterbutler.core.log_payload import LogPayload
 from waterbutler.core.remote_logging import log_to_callback
 
-from tests.providers.osfstorage.fixtures import (
-    file_metadata_object,
-    file_path,
-    file_metadata,
-    file_lineage,
-    provider,
-    auth,
-    credentials,
-    settings
-)
-from tests.core.fixtures import (
-    log_payload,
-    MockBadResponse,
-    mock_time,
-    callback_log_payload_move,
-    callback_log_payload_copy,
-    callback_log_payload_upload,
-    mock_signed_request
-)
+from tests.core.fixtures import (MockBadResponse, log_payload, mock_time,
+                                 mock_signed_request, callback_log_payload_upload,
+                                 callback_log_payload_move, callback_log_payload_copy)
+from tests.providers.osfstorage.fixtures import (auth, credentials, provider,
+                                                 settings, file_path, file_lineage,
+                                                 file_metadata, file_metadata_object)
 
 
 class TestScrubPayloadForKeen:
@@ -122,52 +109,63 @@ async def test_log_to_callback_no_logging(self):
         assert (await log_to_callback('metadata')) is None
 
     @pytest.mark.asyncio
-    async def test_log_to_callback_move(self,
-                                        log_payload,
-                                        callback_log_payload_move,
-                                        mock_signed_request,
-                                        mock_time):
-
+    async def test_log_to_callback_move(
+            self,
+            log_payload,
+            callback_log_payload_move,
+            mock_signed_request,
+            mock_time
+    ):
         with mock.patch('waterbutler.core.utils.send_signed_request', mock_signed_request):
-            await log_to_callback('move', log_payload, log_payload)
-            mock_signed_request.assert_called_with('PUT',
-                                                   log_payload.auth['callback_url'],
-                                                   callback_log_payload_move)
+            await log_to_callback('move', source=log_payload, destination=log_payload)
+            mock_signed_request.assert_called_with(
+                'PUT',
+                log_payload.auth['callback_url'],
+                callback_log_payload_move
+            )
 
     @pytest.mark.asyncio
-    async def test_log_to_callback_copy(self,
-                                        log_payload,
-                                        callback_log_payload_copy,
-                                        mock_signed_request,
-                                        mock_time):
-
+    async def test_log_to_callback_copy(
+            self,
+            log_payload,
+            callback_log_payload_copy,
+            mock_signed_request,
+            mock_time
+    ):
         with mock.patch('waterbutler.core.utils.send_signed_request', mock_signed_request):
-            await log_to_callback('copy', log_payload, log_payload)
-            mock_signed_request.assert_called_with('PUT',
-                                                   log_payload.auth['callback_url'],
-                                                   callback_log_payload_copy)
+            await log_to_callback('copy', source=log_payload, destination=log_payload)
+            mock_signed_request.assert_called_with(
+                'PUT',
+                log_payload.auth['callback_url'],
+                callback_log_payload_copy
+            )
 
     @pytest.mark.asyncio
-    async def test_log_to_callback_upload(self,
-                                          log_payload,
-                                          callback_log_payload_upload,
-                                          mock_signed_request,
-                                          mock_time):
-
+    async def test_log_to_callback_upload(
+            self,
+            log_payload,
+            callback_log_payload_upload,
+            mock_signed_request,
+            mock_time
+    ):
         with mock.patch('waterbutler.core.utils.send_signed_request', mock_signed_request):
-            await log_to_callback('upload', log_payload, log_payload)
-            mock_signed_request.assert_called_with('PUT',
-                                                   log_payload.auth['callback_url'],
-                                                   callback_log_payload_upload)
-
-    @pytest.mark.skipif(reason="This test takes too much time because it has 5 retries before "
-                               "throwing the desired exception, it should take around 50-60 "
-                               "seconds")
+            await log_to_callback('upload', source=log_payload, destination=log_payload)
+            mock_signed_request.assert_called_with(
+                'PUT',
+                log_payload.auth['callback_url'],
+                callback_log_payload_upload
+            )
+
+    # TODO: should we fix or skip this? This test never passes for me locally but always takes a long time.
+    @pytest.mark.skipif(
+        reason="This test takes too much time because it has 5 retries before "
+               "throwing the desired exception, it should take around 50-60 seconds"
+    )
     @pytest.mark.asyncio
-    async def test_log_to_callback_throws_exception(self, log_payload, mock_signed_request):
-
+    async def test_log_to_callback_throws_exception(self, mock_signed_request):
         with mock.patch('waterbutler.core.utils.send_signed_request', mock_signed_request):
             with pytest.raises(Exception) as exc:
-                await log_to_callback('upload', log_payload, log_payload)
-                assert exc.message == 'Callback for upload request failed with {},' \
-                                      ' got {"status": "failure"}'.format(MockBadResponse())
+                await log_to_callback('upload')
+                expected_message = 'Callback for upload request failed with {},' \
+                                   ' got {{"status": "failure"}}'.format(MockBadResponse())
+                assert exc.message == expected_message

tests/server/api/v1/fixtures.py

@@ -3,26 +3,18 @@
 from unittest import mock
 
 import pytest
-import tornado
+from tornado.httputil import HTTPServerRequest
+from tornado.http1connection import HTTP1ConnectionParameters
 
 from waterbutler.server.app import make_app
-from waterbutler.server.api.v1.provider import ProviderHandler
 from waterbutler.core.path import WaterButlerPath
 from waterbutler.core.log_payload import LogPayload
+from waterbutler.server.api.v1.provider import ProviderHandler
 
-from tests.providers.osfstorage.fixtures import (
-    file_metadata_object,
-    file_metadata,
-    file_path,
-    file_lineage,
-    provider,
-    auth
-)
+from tests.utils import MockProvider, MockFileMetadata
+from tests.providers.osfstorage.fixtures import (auth, provider, file_metadata_object,
+                                                 file_metadata, file_path, file_lineage)
 
-from tests.utils import (
-    MockProvider,
-    MockFileMetadata
-)
 
 @pytest.yield_fixture
 def event_loop():
@@ -37,74 +29,71 @@ def event_loop():
 
     res._close()
 
+
 @pytest.fixture
 def http_request():
-    http_request = tornado.httputil.HTTPServerRequest(
+    mocked_http_request = HTTPServerRequest(
         uri='/v1/resources/test/providers/test/path/mock',
-        method='GET')
-    http_request.headers['User-Agent'] = 'test'
-    http_request.connection = tornado.http1connection.HTTP1ConnectionParameters()
-    http_request.connection.set_close_callback = mock.Mock()
-    http_request.request_time = mock.Mock(return_value=10)
+        method='GET'
+    )
+    mocked_http_request.headers['User-Agent'] = 'test'
+    mocked_http_request.connection = HTTP1ConnectionParameters()
+    mocked_http_request.connection.set_close_callback = mock.Mock()
+    mocked_http_request.request_time = mock.Mock(return_value=10)
+
+    return mocked_http_request
 
-    return http_request
 
 @pytest.fixture
 def log_payload():
     return LogPayload('test', MockProvider(), path=WaterButlerPath('/test_path'))
 
+
 @pytest.fixture
 def mock_time(monkeypatch):
-    mock_time = mock.Mock()
-    mock_time.return_value = 10
-    monkeypatch.setattr(time, 'time', mock_time)
+    mocked_time = mock.Mock()
+    mocked_time.return_value = 10
+    monkeypatch.setattr(time, 'time', mocked_time)
+
 
 @pytest.fixture
 def handler(http_request):
-    handler = ProviderHandler(make_app(True), http_request)
-    handler.path = WaterButlerPath('/test_path')
+    mocked_handler = ProviderHandler(make_app(True), http_request)
+    mocked_handler.path = WaterButlerPath('/test_path')
 
-    handler.provider = MockProvider()
-    handler.resource = 'test_source_resource'
-    handler.metadata = MockFileMetadata()
+    mocked_handler.provider = MockProvider()
+    mocked_handler.resource = 'test_source_resource'
+    mocked_handler.metadata = MockFileMetadata()
 
-    handler.dest_provider = MockProvider()
-    handler.dest_resource = 'test_dest_resource'
-    handler.dest_meta = MockFileMetadata()
+    mocked_handler.dest_provider = MockProvider()
+    mocked_handler.dest_resource = 'test_dest_resource'
+    mocked_handler.dest_meta = MockFileMetadata()
 
-    return handler
+    return mocked_handler
 
 
 @pytest.fixture
 def source_payload(handler):
-    return LogPayload(handler.resource,
-                                handler.provider,
-                                path=handler.path)
+    return LogPayload(handler.resource, handler.provider, path=handler.path)
 
 
 @pytest.fixture
 def destination_payload(handler):
-    return LogPayload(handler.dest_resource,
-                      handler.provider,
-                      metadata=handler.dest_meta)
+    return LogPayload(handler.dest_resource, handler.provider, metadata=handler.dest_meta)
 
 
 @pytest.fixture
 def payload_path(handler):
-    return LogPayload(handler.resource,
-                                handler.provider,
-                                path=handler.path)
+    return LogPayload(handler.resource, handler.provider, path=handler.path)
 
 
 @pytest.fixture
 def payload_metadata(handler):
-    return LogPayload(handler.resource,
-                      handler.provider,
-                      metadata=handler.metadata)
+    return LogPayload(handler.resource, handler.provider, metadata=handler.metadata)
 
 
 @pytest.fixture
-def serialzied_request(handler):
+def serialized_request():
     return {
         'request': {
             'url': 'http://127.0.0.1/v1/resources/test/providers/test/path/mock',
@@ -118,5 +107,5 @@ def serialzied_request(handler):
         },
         'referrer': {
             'url': None
-        }
-    }
+        },
+    }