Skip to content

Commit 9700e8e

Browse files
committed
refactor(tests): scope _relation mocks to the tests that use them
Three sites in test_on_create_replication wrote the PropertyMock onto the class (type(relation)._relation = ...) without cleanup, leaking it to every later test; two more tests then silently RELIED on that leak — their instance writes only worked through the leaked mock's __set__, so both fail with AttributeError when run standalone. Six further instance writes were silent no-ops against whichever mock happened to be leaked, their assertions passing by coincidence of branch equivalence. Convert every site to scoped patch.object contexts: cleanup is automatic, order-dependence is gone, previously-red solo runs pass, and _REAL_RELATION_PROPERTY capture/restore machinery is deleted. The leaked .app PropertyMocks in test_promote_to_primary covered a path production never reads and are dropped entirely. Verified: full file green, and solo runs of test__configure_standby_ cluster / test_handle_forceful_promotion / the dying-relation probe now pass in isolation. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
1 parent 66c6665 commit 9700e8e

1 file changed

Lines changed: 81 additions & 47 deletions

File tree

tests/unit/test_async_replication.py

Lines changed: 81 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,6 @@
1919
_same_secret_id,
2020
)
2121

22-
# Several tests (e.g. ``test_on_create_replication``) reassign ``_relation`` on the class
23-
# via ``type(relation)._relation = PropertyMock(...)`` with no cleanup, leaking a mock over
24-
# the real property for later tests. Capture the real property once, before any test runs,
25-
# so a test that needs to exercise the real ``_relation`` can restore it for its own scope.
26-
_REAL_RELATION_PROPERTY = PostgreSQLAsyncReplication.__dict__["_relation"]
27-
2822

2923
def create_mock_unit(name="unit"):
3024
unit = MagicMock()
@@ -176,9 +170,13 @@ def test_on_create_replication():
176170

177171
mock_relation = MagicMock()
178172
mock_relation.name = REPLICATION_CONSUMER_RELATION
179-
type(relation)._relation = PropertyMock(return_value=mock_relation)
180-
181-
result = relation._on_create_replication(mock_event)
173+
with patch.object(
174+
PostgreSQLAsyncReplication,
175+
"_relation",
176+
new_callable=PropertyMock,
177+
return_value=mock_relation,
178+
):
179+
result = relation._on_create_replication(mock_event)
182180

183181
assert result is None
184182
mock_event.fail.assert_called_once_with(
@@ -196,9 +194,13 @@ def test_on_create_replication():
196194

197195
mock_relation = MagicMock()
198196
mock_relation.name = "Something"
199-
type(relation)._relation = PropertyMock(return_value=mock_relation)
200-
201-
result = relation._on_create_replication(mock_event)
197+
with patch.object(
198+
PostgreSQLAsyncReplication,
199+
"_relation",
200+
new_callable=PropertyMock,
201+
return_value=mock_relation,
202+
):
203+
result = relation._on_create_replication(mock_event)
202204

203205
assert result is None
204206

@@ -214,9 +216,13 @@ def test_on_create_replication():
214216

215217
mock_relation = MagicMock()
216218
mock_relation.name = "Something"
217-
type(relation)._relation = PropertyMock(return_value=mock_relation)
218-
219-
result = relation._on_create_replication(mock_event)
219+
with patch.object(
220+
PostgreSQLAsyncReplication,
221+
"_relation",
222+
new_callable=PropertyMock,
223+
return_value=mock_relation,
224+
):
225+
result = relation._on_create_replication(mock_event)
220226

221227
assert result is None
222228

@@ -232,7 +238,6 @@ def test_promote_to_primary():
232238
relation = PostgreSQLAsyncReplication(mock_charm)
233239
relation._get_primary_cluster = MagicMock(return_value=None)
234240

235-
type(relation).app = PropertyMock(return_value=mock_relation)
236241
result = relation.promote_to_primary(mock_event)
237242
assert result is None
238243

@@ -244,14 +249,12 @@ def test_promote_to_primary():
244249
mock_charm = MagicMock()
245250
mock_event = MagicMock()
246251
mock_relation = MagicMock()
247-
mock_app = MagicMock(spec=Application)
248252
mock_relation.status = MagicMock()
249253
mock_relation.status.message = READ_ONLY_MODE_BLOCKING_MESSAGE
250254

251255
relation = PostgreSQLAsyncReplication(mock_charm)
252256
relation._get_primary_cluster = MagicMock(return_value=None)
253257

254-
type(relation).app = PropertyMock(return_value=mock_app)
255258
relation._handle_replication_change = MagicMock(return_value=False)
256259

257260
result = relation.promote_to_primary(mock_event)
@@ -264,11 +267,17 @@ def test__configure_standby_cluster():
264267
mock_event = MagicMock()
265268

266269
relation = PostgreSQLAsyncReplication(mock_charm)
267-
relation._relation = MagicMock()
268-
relation._relation.name = REPLICATION_CONSUMER_RELATION
270+
mock_relation = MagicMock()
271+
mock_relation.name = REPLICATION_CONSUMER_RELATION
269272
relation._update_internal_secret = MagicMock(return_value=False)
270273

271-
result = relation._configure_standby_cluster(mock_event)
274+
with patch.object(
275+
PostgreSQLAsyncReplication,
276+
"_relation",
277+
new_callable=PropertyMock,
278+
return_value=mock_relation,
279+
):
280+
result = relation._configure_standby_cluster(mock_event)
272281

273282
assert result is False
274283

@@ -279,12 +288,20 @@ def test__configure_standby_cluster():
279288
mock_event = MagicMock()
280289

281290
relation = PostgreSQLAsyncReplication(mock_charm)
282-
relation._relation = MagicMock()
283-
relation._relation.name = "something_else"
291+
mock_relation = MagicMock()
292+
mock_relation.name = "something_else"
284293
relation._update_internal_secret = MagicMock(return_value=True)
285294
relation.get_system_identifier = MagicMock(return_value=(None, 2))
286295

287-
with pytest.raises(Exception) as exc_info:
296+
with (
297+
patch.object(
298+
PostgreSQLAsyncReplication,
299+
"_relation",
300+
new_callable=PropertyMock,
301+
return_value=mock_relation,
302+
),
303+
pytest.raises(Exception) as exc_info,
304+
):
288305
relation._configure_standby_cluster(mock_event)
289306

290307
assert str(exc_info.value) == "2"
@@ -294,17 +311,25 @@ def test__configure_standby_cluster():
294311
mock_event = MagicMock()
295312

296313
relation = PostgreSQLAsyncReplication(mock_charm)
297-
relation._relation = MagicMock()
298-
relation._relation.name = "some_relation"
299-
relation._relation.app = "remote-app"
300-
relation._relation.data = {relation._relation.app: {"system-id": "123"}}
314+
mock_relation = MagicMock()
315+
mock_relation.name = "some_relation"
316+
mock_relation.app = "remote-app"
317+
mock_relation.data = {"remote-app": {"system-id": "123"}}
301318

302319
relation._update_internal_secret = MagicMock(return_value=True)
303320
relation.get_system_identifier = MagicMock(return_value=("456", None))
304321
relation.charm = MagicMock()
305322
relation.charm.app_peer_data = {}
306323

307-
with patch("subprocess.check_call") as mock_check_call:
324+
with (
325+
patch.object(
326+
PostgreSQLAsyncReplication,
327+
"_relation",
328+
new_callable=PropertyMock,
329+
return_value=mock_relation,
330+
),
331+
patch("subprocess.check_call") as mock_check_call,
332+
):
308333
result = relation._configure_standby_cluster(mock_event)
309334

310335
assert result is True
@@ -518,16 +543,20 @@ def test_handle_forceful_promotion():
518543
mock_event.params.get.return_value = False
519544

520545
relation = PostgreSQLAsyncReplication(mock_charm)
521-
522-
relation._relation = MagicMock()
523-
relation._relation.app = MagicMock()
524-
relation._relation.app.name = "test-app"
546+
mock_relation = MagicMock()
547+
mock_relation.app.name = "test-app"
525548

526549
relation.get_all_primary_cluster_endpoints = MagicMock(return_value=[1, 2, 3])
527550

528551
mock_charm.patroni_manager.get_primary.side_effect = RetryError("timeout")
529552

530-
result = relation._handle_forceful_promotion(mock_event)
553+
with patch.object(
554+
PostgreSQLAsyncReplication,
555+
"_relation",
556+
new_callable=PropertyMock,
557+
return_value=mock_relation,
558+
):
559+
result = relation._handle_forceful_promotion(mock_event)
531560

532561
mock_event.fail.assert_called_once_with(
533562
"test-app isn't reachable. Pass `force=true` to promote anyway."
@@ -540,16 +569,20 @@ def test_handle_forceful_promotion():
540569
mock_event.params.get.return_value = False
541570

542571
relation = PostgreSQLAsyncReplication(mock_charm)
543-
544-
relation._relation = MagicMock()
545-
relation._relation.app = MagicMock()
546-
relation._relation.app.name = "test-app"
572+
mock_relation = MagicMock()
573+
mock_relation.app.name = "test-app"
547574

548575
relation.get_all_primary_cluster_endpoints = MagicMock(return_value=[1, 2, 3])
549576

550577
mock_charm._patroni.get_primary.side_effect = None
551578

552-
result = relation._handle_forceful_promotion(mock_event)
579+
with patch.object(
580+
PostgreSQLAsyncReplication,
581+
"_relation",
582+
new_callable=PropertyMock,
583+
return_value=mock_relation,
584+
):
585+
result = relation._handle_forceful_promotion(mock_event)
553586

554587
assert result is True
555588
# 4.
@@ -559,16 +592,20 @@ def test_handle_forceful_promotion():
559592
mock_event.params.get.return_value = False
560593

561594
relation = PostgreSQLAsyncReplication(mock_charm)
562-
563-
relation._relation = MagicMock()
564-
relation._relation.app = MagicMock()
565-
relation._relation.app.name = "test-app"
595+
mock_relation = MagicMock()
596+
mock_relation.app.name = "test-app"
566597

567598
relation.get_all_primary_cluster_endpoints = MagicMock(return_value=[])
568599

569600
mock_charm._patroni.get_primary.side_effect = None
570601

571-
result = relation._handle_forceful_promotion(mock_event)
602+
with patch.object(
603+
PostgreSQLAsyncReplication,
604+
"_relation",
605+
new_callable=PropertyMock,
606+
return_value=mock_relation,
607+
):
608+
result = relation._handle_forceful_promotion(mock_event)
572609

573610
assert result is True
574611

@@ -813,9 +850,6 @@ def test__relation_skips_unreadable_dying_relation(monkeypatch):
813850
# read, even though get_relation still returns it. _relation must probe and
814851
# treat such a relation as absent, so the promoted primary reconciles as a
815852
# standalone cluster instead of crashing every hook that writes relation data.
816-
# Restore the real property for this test's scope (an earlier test may have
817-
# leaked a class-level PropertyMock over it); monkeypatch reverts it afterwards.
818-
monkeypatch.setattr(PostgreSQLAsyncReplication, "_relation", _REAL_RELATION_PROPERTY)
819853
mock_charm = MagicMock()
820854

821855
dying = MagicMock()

0 commit comments

Comments
 (0)