Skip to content

Commit 8bdbd1a

Browse files
author
prakash-kalwaniya
committed
tests(config): fix on_commit test failures and add TransactionTestCase for handlers
- Wrap device creation and backend change in `captureOnCommitCallbacks(execute=True)` in test_config_backend_changed, test_device_create_with_group, and test_device_api_change_config_backend so that transaction.on_commit callbacks fire correctly inside TestCase (which wraps tests in a never-committed transaction) - Add test_handlers_transaction.py with TransactionTestCase covering all three model handlers (Device, DeviceGroup, Config) to verify change_devices_templates.delay is called after commit and NOT called on rollback, as requested by CodeRabbit - Fix flake8 E501 line-length violations in test_handlers_transaction.py
1 parent d90b543 commit 8bdbd1a

File tree

3 files changed

+86
-5
lines changed

3 files changed

+86
-5
lines changed

openwisp_controller/config/tests/test_api.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ def test_device_create_with_group(self):
172172
org = self._get_org()
173173
data["organization"] = org.pk
174174
data["group"] = dg.pk
175-
r = self.client.post(path, data, content_type="application/json")
175+
with self.captureOnCommitCallbacks(execute=True):
176+
r = self.client.post(path, data, content_type="application/json")
176177
self.assertEqual(r.status_code, 201)
177178
self.assertEqual(Device.objects.count(), 1)
178179
self.assertIn(template, Device.objects.first().config.templates.all())
@@ -474,7 +475,8 @@ def test_device_api_change_config_backend(self):
474475
t2 = self._create_template(name="t2", backend="netjsonconfig.OpenWisp")
475476
dg1 = self._create_device_group(name="dg-1")
476477
dg1.templates.add(t1, t2)
477-
d1 = self._create_device(name="test-device", group=dg1)
478+
with self.captureOnCommitCallbacks(execute=True):
479+
d1 = self._create_device(name="test-device", group=dg1)
478480
self.assertIn(t1, d1.config.templates.all())
479481
path = reverse("config_api:device_detail", args=[d1.pk])
480482
data = {
@@ -490,7 +492,8 @@ def test_device_api_change_config_backend(self):
490492
"config": {},
491493
},
492494
}
493-
r = self.client.put(path, data, content_type="application/json")
495+
with self.captureOnCommitCallbacks(execute=True):
496+
r = self.client.put(path, data, content_type="application/json")
494497
self.assertEqual(r.status_code, 200)
495498
self.assertEqual(r.data["group"], dg1.pk)
496499
self.assertEqual(r.data["config"]["backend"], "netjsonconfig.OpenWisp")

openwisp_controller/config/tests/test_config.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,8 @@ def test_config_backend_changed(self):
878878
group.templates.add(*[t1, t2])
879879
with self.subTest("config_backend_changed signal must not be sent on creation"):
880880
with catch_signal(config_backend_changed) as handler:
881-
d = self._create_device(group=group, organization=org)
881+
with self.captureOnCommitCallbacks(execute=True):
882+
d = self._create_device(group=group, organization=org)
882883
handler.assert_not_called()
883884
self.assertTrue(d.config.templates.filter(pk=t1.pk).exists())
884885
self.assertFalse(d.config.templates.filter(pk=t2.pk).exists())
@@ -896,7 +897,8 @@ def test_config_backend_changed(self):
896897
with catch_signal(config_backend_changed) as handler:
897898
c = d.config
898899
c.backend = backend
899-
c.save(update_fields=["backend"])
900+
with self.captureOnCommitCallbacks(execute=True):
901+
c.save(update_fields=["backend"])
900902
handler.assert_called_once_with(
901903
sender=Config,
902904
signal=config_backend_changed,
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
from unittest.mock import patch
2+
3+
from django.db import IntegrityError, transaction
4+
from django.test import TransactionTestCase
5+
from swapper import load_model
6+
7+
from openwisp_controller.config.handlers import devicegroup_templates_change_handler
8+
9+
Device = load_model("config", "Device")
10+
DeviceGroup = load_model("config", "DeviceGroup")
11+
Config = load_model("config", "Config")
12+
13+
14+
class TestHandlersTransaction(TransactionTestCase):
15+
@patch("openwisp_controller.config.tasks.change_devices_templates.delay")
16+
def test_devicegroup_templates_change_handler_on_commit(self, mock_delay):
17+
"""
18+
Test that change_devices_templates is called via delay and only after
19+
transaction.on_commit block commits successfully.
20+
"""
21+
22+
class MockMeta:
23+
def __init__(self, model_name):
24+
self.model_name = model_name
25+
26+
class MockState:
27+
def __init__(self, adding):
28+
self.adding = adding
29+
30+
class MockInstance:
31+
def __init__(self, id, model_name):
32+
self.id = id
33+
self._meta = MockMeta(model_name)
34+
self._state = MockState(adding=False)
35+
36+
# Test cases for each model caller
37+
model_names = [
38+
Device._meta.model_name,
39+
DeviceGroup._meta.model_name,
40+
Config._meta.model_name,
41+
]
42+
43+
for model_name in model_names:
44+
mock_instance = MockInstance(id="test-id", model_name=model_name)
45+
kwargs = {}
46+
if model_name == Config._meta.model_name:
47+
kwargs["backend"] = "test-backend"
48+
kwargs["old_backend"] = "old-backend"
49+
elif model_name == DeviceGroup._meta.model_name:
50+
kwargs["templates"] = []
51+
kwargs["old_templates"] = []
52+
elif model_name == Device._meta.model_name:
53+
kwargs["group_id"] = "test-group"
54+
kwargs["old_group_id"] = "old-group"
55+
56+
# Case 1: Transaction commits
57+
mock_delay.reset_mock()
58+
with transaction.atomic():
59+
devicegroup_templates_change_handler(mock_instance, **kwargs)
60+
# Should not be called inside the transaction
61+
mock_delay.assert_not_called()
62+
63+
mock_delay.assert_called_once() # Should be called after commit
64+
65+
# Case 2: Transaction rolls back
66+
mock_delay.reset_mock()
67+
try:
68+
with transaction.atomic():
69+
devicegroup_templates_change_handler(mock_instance, **kwargs)
70+
mock_delay.assert_not_called()
71+
raise IntegrityError("Force rollback")
72+
except IntegrityError:
73+
pass
74+
75+
# Should not be called because it rolled back
76+
mock_delay.assert_not_called()

0 commit comments

Comments
 (0)