Skip to content

Commit b91a059

Browse files
committed
Don't try to mock context manager; use a simple class instead
This one's related to #38. It turns out that while trying to mock a context manager kind of works, it will do the wrong thing in edge cases like when an exception is thrown from inside a `with` block, silently swallowing it and causing a return that's completely wrong. There may be some way to fix the mock to make it do the right thing, but instead of getting fancier with these mocks that are already awful, instead repair the problem by defining a plain class that implements context manager and just use that.
1 parent fc1a2b8 commit b91a059

File tree

2 files changed

+14
-11
lines changed

2 files changed

+14
-11
lines changed

.github/workflows/ci.yaml

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ jobs:
6363
run: psql --echo-errors --quiet -c '\timing off' -c "CREATE DATABASE ${TEST_DATABASE_NAME};" ${ADMIN_DATABASE_URL}
6464

6565
- name: river migrate-up
66-
run: river migrate-up --database-url "$TEST_DATABASE_URL"
66+
run: river migrate-up --database-url "$DATABASE_URL" --max-steps 5 # temporarily include max steps so tests can pass with unique fixes
6767

6868
- name: Test
6969
run: rye test
@@ -109,7 +109,7 @@ jobs:
109109
run: psql --echo-errors --quiet -c '\timing off' -c "CREATE DATABASE ${DATABASE_NAME};" ${ADMIN_DATABASE_URL}
110110

111111
- name: river migrate-up
112-
run: river migrate-up --database-url "$DATABASE_URL"
112+
run: river migrate-up --database-url "$DATABASE_URL" --max-steps 5 # temporarily include max steps so tests can pass with unique fixes
113113

114114
- name: Run examples
115115
run: rye run python3 -m examples.all

tests/client_test.py

+12-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from dataclasses import dataclass
22
from datetime import datetime, timezone
3-
from unittest.mock import MagicMock, Mock, patch
3+
from unittest.mock import MagicMock, patch
44

55
import pytest
66

@@ -16,17 +16,20 @@ def mock_driver() -> DriverProtocol:
1616

1717
@pytest.fixture
1818
def mock_exec(mock_driver) -> ExecutorProtocol:
19-
def mock_context_manager(val) -> Mock:
20-
context_manager_mock = MagicMock()
21-
context_manager_mock.__enter__.return_value = val
22-
context_manager_mock.__exit__.return_value = Mock()
23-
return context_manager_mock
19+
# Don't try to mock a context manager. It will cause endless pain around the
20+
# edges like swallowing raised exceptions.
21+
class TrivialContextManager:
22+
def __init__(self, with_val):
23+
self.with_val = with_val
2424

25-
# def mock_context_manager(val) -> Mock:
26-
# return Mock(__enter__=val, __exit__=Mock())
25+
def __enter__(self):
26+
return self.with_val
27+
28+
def __exit__(self, exc_type, exc_val, exc_tb):
29+
pass
2730

2831
mock_exec = MagicMock(spec=ExecutorProtocol)
29-
mock_driver.executor.return_value = mock_context_manager(mock_exec)
32+
mock_driver.executor.return_value = TrivialContextManager(mock_exec)
3033

3134
return mock_exec
3235

0 commit comments

Comments
 (0)