Improve tests' quality & coverage (#352)

* Add tests for `_scrub_url`.

I ran `pytest -x tests --cov=socketshark --cov-report=html` and noticed that
`_scrub_url` was not sufficiently tested. Now it is.

* Use hard-coded values instead of reusing constants.

Yes, this makes the tests more tedious to update if you merely change the copy
of an error message, but that's a feature, not a bug. You *should* update the
tests when you change the copy and thus acknowledge that the change you're
making is intentional and works well in all (presumably tested) scenarios.

In practice though, error copy barely ever changes and what you get instead
with hard-coded values is much better readability of tests.

* Add tests for `socketshark.metrics`.

That's another module that was sorely missing tests. The coverage for
`socketshark/metrics/__init__.py` jumped from 63% to 90% with these changes.

* Add `.coverage` to `.gitignore`.

We don't need this tracked and it's distracting to see it in `git status`
whenver you play with pytest-cov locally.

* Test `LogMetrics` more thoroughly.

This bumps the coverage on `socketshark/metrics/log.py` from 0% on master to
70% prior to this commit to now 95%.

The total coverage for all of `socketshark/` has reached 90% now.

* 2 extra tests for the `on_service_event` handler.

This bumps the coverage of socketshark/session.py` from 84% to 90%.

Author: wojcikstefan

.gitignore

@@ -3,6 +3,7 @@
 .DS_Store
 build/
 .cache/
+.coverage
 dist/
 *.egg-info/
 .tox/
@@ -11,4 +12,4 @@ venv
 .venv
 CLAUDE.local.md
 .claude/settings.local.json
-.claude/worktrees/
+.claude/worktrees/

tests/test_basic.py

@@ -12,12 +12,7 @@
 from structlog.testing import capture_logs
 from yarl import URL
 
-from socketshark import (
-    SocketShark,
-    config_defaults,
-    constants as c,
-    setup_logging,
-)
+from socketshark import SocketShark, config_defaults, setup_logging
 from socketshark.session import Session
 
 LOCAL_REDIS_HOST = os.environ.get('LOCAL_REDIS_HOST')
@@ -193,39 +188,39 @@ async def test_invalid_message(self):
         await session.on_client_event(None)
         assert client.log.pop() == {
             'status': 'error',
-            'error': c.ERR_INVALID_EVENT,
+            'error': 'Messages must be JSON and contain an event field.',
         }
 
         await session.on_client_event('hello')
         assert client.log.pop() == {
             'status': 'error',
-            'error': c.ERR_INVALID_EVENT,
+            'error': 'Messages must be JSON and contain an event field.',
         }
 
         await session.on_client_event({})
         assert client.log.pop() == {
             'status': 'error',
-            'error': c.ERR_INVALID_EVENT,
+            'error': 'Messages must be JSON and contain an event field.',
         }
 
         await session.on_client_event({'event': None})
         assert client.log.pop() == {
             'status': 'error',
-            'error': c.ERR_INVALID_EVENT,
+            'error': 'Messages must be JSON and contain an event field.',
         }
 
         await session.on_client_event({'event': ''})
         assert client.log.pop() == {
             'status': 'error',
             'event': '',
-            'error': c.ERR_EVENT_NOT_FOUND,
+            'error': 'Event not found.',
         }
 
         await session.on_client_event({'event': 'hello'})
         assert client.log.pop() == {
             'status': 'error',
             'event': 'hello',
-            'error': c.ERR_EVENT_NOT_FOUND,
+            'error': 'Event not found.',
         }
 
         assert not client.log
@@ -243,7 +238,7 @@ async def test_auth_invalid(self):
         assert client.log.pop() == {
             'status': 'error',
             'event': 'auth',
-            'error': c.ERR_AUTH_UNSUPPORTED,
+            'error': 'Authentication method unsupported.',
         }
 
         no_auth_config = TEST_CONFIG.copy()
@@ -255,7 +250,7 @@ async def test_auth_invalid(self):
         assert client.log.pop() == {
             'status': 'error',
             'event': 'auth',
-            'error': c.ERR_AUTH_UNSUPPORTED,
+            'error': 'Authentication method unsupported.',
         }
         assert not client.log
 
@@ -286,6 +281,77 @@ async def test_auth_invalid_emits_structlog_logs(self):
             },
         ]
 
+    @pytest.mark.asyncio
+    async def test_invalid_event_emits_structlog_logs(self):
+        shark = SocketShark(TEST_CONFIG)
+        session = MockClient(shark).session
+        with capture_logs() as structlog_logs:
+            await session.on_service_event({'invalid': 'event'})
+        assert structlog_logs == [
+            {
+                'log_level': 'warning',
+                'event': 'invalid service event',
+                'data': {'invalid': 'event'},
+                'pid': mock.ANY,
+                'session': mock.ANY,
+            },
+        ]
+
+    @pytest.mark.asyncio
+    async def test_invalid_published_at_is_handled_gracefully(self):
+        shark = SocketShark(TEST_CONFIG)
+        await shark.prepare()
+        client = MockClient(shark)
+        session = client.session
+
+        await session.on_client_event(
+            {
+                'event': 'subscribe',
+                'subscription': 'simple.topic',
+            }
+        )
+        assert client.log.pop() == {
+            'event': 'subscribe',
+            'subscription': 'simple.topic',
+            'status': 'ok',
+        }
+
+        with capture_logs() as structlog_logs:
+            await session.on_service_event(
+                {
+                    'subscription': 'simple.topic',
+                    'data': {'foo': 'bar'},
+                    'published_at': 'not-a-date',
+                }
+            )
+
+        assert structlog_logs == [
+            {
+                'log_level': 'warning',
+                'event': 'invalid published_at format',
+                'published_at': 'not-a-date',
+                'session': mock.ANY,
+                'pid': mock.ANY,
+            },
+            # The message is still expected to be sent. An invalid
+            # `published_at` should not break the entire end-to-end messaging
+            # flow .
+            {
+                'log_level': 'debug',
+                'event': 'client send',
+                'data': {
+                    'subscription': 'simple.topic',
+                    'event': 'message',
+                    'data': {'foo': 'bar'},
+                    'sent_at': mock.ANY,
+                },
+                'pid': mock.ANY,
+                'session': mock.ANY,
+            },
+        ]
+
+        await shark.shutdown()
+
     @pytest.mark.asyncio
     async def test_auth_ticket(self):
         """
@@ -299,14 +365,14 @@ async def test_auth_ticket(self):
         assert client.log.pop() == {
             'status': 'error',
             'event': 'auth',
-            'error': c.ERR_NEEDS_TICKET,
+            'error': 'Must specify ticket.',
         }
 
         await session.on_client_event({'event': 'auth', 'method': 'ticket'})
         assert client.log.pop() == {
             'status': 'error',
             'event': 'auth',
-            'error': c.ERR_NEEDS_TICKET,
+            'error': 'Must specify ticket.',
         }
 
         with aioresponses() as mock_responses:
@@ -321,7 +387,7 @@ async def test_auth_ticket(self):
             assert client.log.pop() == {
                 'status': 'error',
                 'event': 'auth',
-                'error': c.ERR_SERVICE_UNAVAILABLE,
+                'error': 'Service unavailable.',
             }
 
         with aioresponses() as mock_responses:
@@ -359,7 +425,7 @@ async def test_auth_ticket(self):
             assert client.log.pop() == {
                 'status': 'error',
                 'event': 'auth',
-                'error': c.ERR_AUTH_FAILED,
+                'error': 'Authentication failed.',
             }
 
             assert session.auth_info == {}
@@ -412,7 +478,7 @@ async def test_subscription_invalid(self):
         assert client.log.pop() == {
             'event': 'subscribe',
             'status': 'error',
-            'error': c.ERR_INVALID_SUBSCRIPTION_FORMAT,
+            'error': 'Invalid subscription format.',
         }
 
         await session.on_client_event(
@@ -422,7 +488,7 @@ async def test_subscription_invalid(self):
             'event': 'subscribe',
             'subscription': 'invalid',
             'status': 'error',
-            'error': c.ERR_INVALID_SUBSCRIPTION_FORMAT,
+            'error': 'Invalid subscription format.',
         }
 
         await session.on_client_event(
@@ -432,7 +498,7 @@ async def test_subscription_invalid(self):
             'event': 'subscribe',
             'subscription': 'invalid.topic',
             'status': 'error',
-            'error': c.ERR_INVALID_SERVICE,
+            'error': 'Invalid service.',
         }
 
         assert not client.log
@@ -456,7 +522,7 @@ async def test_subscription_needs_auth(self):
             'event': 'subscribe',
             'subscription': 'empty.topic',
             'status': 'error',
-            'error': c.ERR_AUTH_REQUIRED,
+            'error': 'Authentication required.',
         }
 
         assert not client.log
@@ -490,7 +556,7 @@ async def test_subscription_validation(self):
             'event': 'subscribe',
             'subscription': 'simple.topic',
             'status': 'error',
-            'error': c.ERR_ALREADY_SUBSCRIBED,
+            'error': 'Already subscribed.',
         }
 
         await session.on_client_event(
@@ -504,7 +570,7 @@ async def test_subscription_validation(self):
             'status': 'error',
             'subscription': 'simple.invalid',
             'event': 'message',
-            'error': c.ERR_SUBSCRIPTION_NOT_FOUND,
+            'error': 'Subscription does not exist.',
         }
 
         await session.on_client_event(
@@ -517,7 +583,7 @@ async def test_subscription_validation(self):
             'event': 'unsubscribe',
             'subscription': 'simple.invalid',
             'status': 'error',
-            'error': c.ERR_SUBSCRIPTION_NOT_FOUND,
+            'error': 'Subscription does not exist.',
         }
 
         await shark.shutdown()
@@ -679,7 +745,7 @@ async def test_subscription_auth(self):
             'event': 'subscribe',
             'subscription': 'simple_auth.topic',
             'status': 'error',
-            'error': c.ERR_AUTH_REQUIRED,
+            'error': 'Authentication required.',
         }
 
         await self._auth_session(session)
@@ -775,7 +841,7 @@ async def test_subscription_authorizer(self):
             'event': 'subscribe',
             'subscription': 'authorizer.topic',
             'status': 'error',
-            'error': c.ERR_AUTH_REQUIRED,
+            'error': 'Authentication required.',
         }
 
         await self._auth_session(session)
@@ -792,7 +858,7 @@ async def test_subscription_authorizer(self):
                 'event': 'subscribe',
                 'subscription': 'authorizer.topic',
                 'status': 'error',
-                'error': c.ERR_SERVICE_UNAVAILABLE,
+                'error': 'Service unavailable.',
             }
 
         with aioresponses() as mock_responses:
@@ -831,7 +897,7 @@ async def test_subscription_authorizer(self):
                 'event': 'subscribe',
                 'subscription': 'authorizer.topic',
                 'status': 'error',
-                'error': c.ERR_UNAUTHORIZED,
+                'error': 'Unauthorized.',
             }
 
             await session.on_client_event(
@@ -1023,7 +1089,7 @@ async def test_subscription_periodic_authorizer(self):
             assert client.log.pop(0) == {
                 'event': 'unsubscribe',
                 'subscription': 'periodic_authorizer.topic',
-                'error': c.ERR_UNAUTHORIZED,
+                'error': 'Unauthorized.',
             }
 
             assert client.log.pop(0) == {
@@ -1182,7 +1248,7 @@ async def test_subscription_complex(self):
                 'event': 'subscribe',
                 'subscription': 'complex.topic',
                 'status': 'error',
-                'error': c.ERR_UNHANDLED_EXCEPTION,
+                'error': 'Unhandled exception.',
             }
 
             mock_responses.post(conf['authorizer'], payload={'status': 'ok'})
@@ -1433,7 +1499,7 @@ async def test_subscription_complex(self):
                 'subscription': 'complex.topic',
                 'status': 'error',
                 'extra': 'hello',
-                'error': c.ERR_UNHANDLED_EXCEPTION,
+                'error': 'Unhandled exception.',
             }
 
             await session.on_client_event(