From fc8f22432cfd5f0e994e104a8b7c2ee20ffa0bbf Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sun, 19 Feb 2023 15:13:31 +0100 Subject: [PATCH] Fix skipped tests in managemails --- gnuviechadmin/managemails/tests/test_forms.py | 327 ++++++++---------- gnuviechadmin/osusers/tests/testutils.py | 13 + 2 files changed, 149 insertions(+), 191 deletions(-) create mode 100644 gnuviechadmin/osusers/tests/testutils.py diff --git a/gnuviechadmin/managemails/tests/test_forms.py b/gnuviechadmin/managemails/tests/test_forms.py index d0beb59..32fe366 100644 --- a/gnuviechadmin/managemails/tests/test_forms.py +++ b/gnuviechadmin/managemails/tests/test_forms.py @@ -2,14 +2,14 @@ This module provides tests for :py:mod:`managemails.forms`. """ -from unittest import skip -from unittest.mock import ANY, MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, patch from django.forms import ValidationError from django.test import TestCase from django.urls import reverse -import osusers.models +from osusers.tests.testutils import create_test_user + from domains.models import MailDomain from managemails.forms import ( MAILBOX_OR_FORWARDS, @@ -21,20 +21,21 @@ from managemails.forms import ( multiple_email_validator, ) from managemails.models import MailAddress, Mailbox +from taskresults.tests.testutils import TestCaseWithCeleryTasks class CreateMailboxFormTest(TestCase): - def test_constructor_needs_hostingpackage(self): - instance = MagicMock() + def test_constructor_needs_hosting_package(self): + instance = Mailbox() with self.assertRaises(KeyError): CreateMailboxForm(instance) def test_constructor(self): - hostingpackage = Mock(id=42) + hosting_package = Mock(id=42) instance = MagicMock() - form = CreateMailboxForm(instance, hostingpackage=hostingpackage) + form = CreateMailboxForm(instance, hostingpackage=hosting_package) self.assertTrue(hasattr(form, "hosting_package")) - self.assertEqual(form.hosting_package, hostingpackage) + self.assertEqual(form.hosting_package, hosting_package) self.assertTrue(hasattr(form, "helper")) self.assertEqual( form.helper.form_action, reverse("create_mailbox", kwargs={"package": 42}) @@ -134,32 +135,47 @@ class MailAddressFieldMixinTest(TestCase): self.assertIn("forwards", form.fields) -class AddMailAddressFormTest(TestCase): - def test_constructor_needs_hostingpackage(self): - instance = MailAddress() - with self.assertRaises(KeyError): - AddMailAddressForm(instance=instance, maildomain=None) +def create_test_mailbox(os_user=None): + if os_user is None: + os_user = create_test_user() - def test_constructor_needs_maildomain(self): - instance = MagicMock() + mail_box = Mailbox.objects.create(osuser=os_user, username="mailbox23") + mail_box.set_password("test") + + return mail_box + + +def create_test_mail_domain(): + return MailDomain.objects.create(domain="example.org") + + +class AddMailAddressFormTest(TestCaseWithCeleryTasks): + def setUp(self) -> None: + super().setUp() + self.mail_domain = create_test_mail_domain() + self.instance = MailAddress() + self.os_user = create_test_user() + self.hosting_package = create_test_hosting_package(os_user=self.os_user) + + def test_constructor_needs_hosting_package(self): with self.assertRaises(KeyError): - AddMailAddressForm(instance=instance, hostingpackage=MagicMock()) + AddMailAddressForm(instance=self.instance, maildomain=MagicMock()) + + def test_constructor_needs_mail_domain(self): + with self.assertRaises(KeyError): + AddMailAddressForm(instance=self.instance, hostingpackage=MagicMock()) def test_constructor(self): - instance = MailAddress() - os_user = osusers.models.User(username="testuser") - hosting_package = MagicMock(id=42, osuser=os_user) - mail_domain = MailDomain(domain="example.org") form = AddMailAddressForm( - instance=instance, hostingpackage=hosting_package, maildomain=mail_domain + instance=self.instance, hostingpackage=self.hosting_package, maildomain=self.mail_domain ) self.assertIn("mailbox_or_forwards", form.fields) self.assertIn("mailbox", form.fields) self.assertIn("forwards", form.fields) self.assertTrue(hasattr(form, "hosting_package")) - self.assertEqual(form.hosting_package, hosting_package) + self.assertEqual(form.hosting_package, self.hosting_package) self.assertTrue(hasattr(form, "maildomain")) - self.assertEqual(form.maildomain, mail_domain) + self.assertEqual(form.maildomain, self.mail_domain) self.assertTrue(hasattr(form, "helper")) self.assertEqual( form.helper.form_action, @@ -169,15 +185,10 @@ class AddMailAddressFormTest(TestCase): self.assertEqual(form.helper.layout[1].name, "submit") def test_clean_localpart_valid(self): - mail_domain = MailDomain.objects.create(domain="example.org") - - instance = MailAddress() - os_user = osusers.models.User(username="testuser") - hosting_package = MagicMock(id=42, osuser=os_user) form = AddMailAddressForm( - instance=instance, - hostingpackage=hosting_package, - maildomain=mail_domain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards, @@ -188,17 +199,12 @@ class AddMailAddressFormTest(TestCase): self.assertEqual("test", form.clean_localpart()) def test_clean_localpart_duplicate(self): - mail_domain = MailDomain.objects.create(domain="example.org") + MailAddress.objects.create(localpart="test", domain=self.mail_domain) - MailAddress.objects.create(localpart="test", domain=mail_domain) - - instance = MailAddress() - osuser = osusers.models.User(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) form = AddMailAddressForm( - instance=instance, - hostingpackage=hostingpackage, - maildomain=mail_domain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards, @@ -209,14 +215,10 @@ class AddMailAddressFormTest(TestCase): self.assertIn("localpart", form.errors) def test_clean_no_mailbox_choice(self): - instance = MailAddress() - osuser = osusers.models.User(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) - maildomain = MailDomain(domain="example.org") form = AddMailAddressForm( - instance=instance, - hostingpackage=hostingpackage, - maildomain=maildomain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox, @@ -226,14 +228,10 @@ class AddMailAddressFormTest(TestCase): self.assertIn("mailbox", form.errors) def test_clean_no_forward_address_choice(self): - instance = MailAddress() - os_user = osusers.models.User(username="testuser") - hosting_package = MagicMock(id=42, osuser=os_user) - mail_domain = MailDomain(domain="example.org") form = AddMailAddressForm( - instance=instance, - hostingpackage=hosting_package, - maildomain=mail_domain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards, @@ -243,15 +241,10 @@ class AddMailAddressFormTest(TestCase): self.assertIn("forwards", form.errors) def test_save_with_forwards_no_commit(self): - mail_domain = MailDomain.objects.create(domain="example.org") - - instance = MailAddress() - os_user = osusers.models.User(username="testuser") - hosting_package = MagicMock(id=42, osuser=os_user) form = AddMailAddressForm( - instance=instance, - hostingpackage=hosting_package, - maildomain=mail_domain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards, @@ -260,18 +253,13 @@ class AddMailAddressFormTest(TestCase): ) self.assertTrue(form.is_valid()) form.save(commit=False) - self.assertEqual(mail_domain, instance.domain) + self.assertEqual(self.mail_domain, self.instance.domain) def test_save_with_forwards_commit(self): - maildomain = MailDomain.objects.create(domain="example.org") - - instance = MailAddress() - osuser = osusers.models.User(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) form = AddMailAddressForm( - instance=instance, - hostingpackage=hostingpackage, - maildomain=maildomain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards, @@ -280,85 +268,60 @@ class AddMailAddressFormTest(TestCase): ) self.assertTrue(form.is_valid()) form.save(commit=True) - self.assertEqual(maildomain, instance.domain) + self.assertEqual(self.mail_domain, self.instance.domain) forwards = list( - instance.mailaddressforward_set.values_list("target", flat=True).order_by( + self.instance.mailaddressforward_set.values_list("target", flat=True).order_by( "target" ) ) self.assertEqual(len(forwards), 2) self.assertEqual(forwards, ["test2@example.org", "test3@example.org"]) - @skip("does not work because it will create a real mailbox") def test_save_with_mailbox_no_commit(self): - instance = MailAddress() - - os_user = osusers.models.User(username="testuser") - hosting_package = MagicMock(id=42, osuser=os_user) - - mail_domain = MailDomain.objects.create(domain="example.org") - mail_box = Mailbox.objects.create(osuser=os_user, username="mailbox23") - mail_box.set_password("test") + mail_box = create_test_mailbox(os_user=self.os_user) form = AddMailAddressForm( - instance=instance, - hostingpackage=hosting_package, - maildomain=mail_domain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox, - "mailbox": "mailbox23", + "mailbox": mail_box, }, ) self.assertTrue(form.is_valid()) form.save(commit=False) - self.assertEqual(mail_domain, instance.domain) + self.assertEqual(self.mail_domain, self.instance.domain) - @skip("does not work because it will create a real mailbox") def test_save_with_mailbox_commit(self): - mail_domain = MailDomain.objects.create(domain="example.org") - - instance = MailAddress() - os_user = osusers.models.User(username="testuser") - - mail_box = Mailbox.objects.create(osuser=os_user, username="mailbox23") - mail_box.set_password("test") - - hosting_package = MagicMock(id=42, osuser=os_user) + mail_box = create_test_mailbox(os_user=self.os_user) form = AddMailAddressForm( - instance=instance, - hostingpackage=hosting_package, - maildomain=mail_domain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox, - "mailbox": "mailbox23", + "mailbox": mail_box, }, ) self.assertTrue(form.is_valid()) form.save(commit=True) - self.assertEqual(mail_domain, instance.domain) + self.assertEqual(self.mail_domain, self.instance.domain) - @skip("does not work because it will create a real mailbox") def test_save_with_other_choice(self): - mail_domain = MailDomain.objects.create(domain="example.org") - - instance = MailAddress() - os_user = osusers.models.User(username="testuser") - hosting_package = MagicMock(id=42, osuser=os_user) - - mail_box = Mailbox.objects.create(osuser=os_user, username="mailbox23") - mail_box.set_password("test") + mail_box = create_test_mailbox(os_user=self.os_user) form = AddMailAddressForm( - instance=instance, - hostingpackage=hosting_package, - maildomain=mail_domain, + instance=self.instance, + hostingpackage=self.hosting_package, + maildomain=self.mail_domain, data={ "localpart": "test", "mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox, - "mailbox": "mailbox23", + "mailbox": mail_box, }, ) self.assertTrue(form.is_valid()) @@ -366,82 +329,78 @@ class AddMailAddressFormTest(TestCase): form.save(commit=True) -class EditMailAddressFormTest(TestCase): - def test_constructor_needs_hostingpackage(self): +def create_test_hosting_package(os_user=None): + if os_user is None: + os_user = create_test_user() + + return MagicMock(id=42, osuser=os_user) + + +class EditMailAddressFormTest(TestCaseWithCeleryTasks): + def setUp(self) -> None: + super().setUp() + self.os_user = create_test_user() + self.hosting_package = create_test_hosting_package(os_user=self.os_user) + self.mail_domain = create_test_mail_domain() + self.instance = MailAddress.objects.create(localpart="test", domain=self.mail_domain) + + def test_constructor_needs_hosting_package(self): instance = MagicMock() with self.assertRaises(KeyError): EditMailAddressForm(instance=instance, maildomain=MagicMock()) - def test_constructor_needs_maildomain(self): + def test_constructor_needs_mail_domain(self): instance = MagicMock() with self.assertRaises(KeyError): EditMailAddressForm(instance=instance, hostingpackage=MagicMock()) def test_constructor(self): - instance = MailAddress(id=23) - osuser = osusers.models.User(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) - maildomain = MailDomain.objects.create(domain="example.org") form = EditMailAddressForm( - instance=instance, maildomain=maildomain, hostingpackage=hostingpackage + instance=self.instance, maildomain=self.mail_domain, hostingpackage=self.hosting_package ) self.assertIn("mailbox_or_forwards", form.fields) self.assertIn("mailbox", form.fields) self.assertIn("forwards", form.fields) self.assertTrue(hasattr(form, "hosting_package")) - self.assertEqual(form.hosting_package, hostingpackage) + self.assertEqual(form.hosting_package, self.hosting_package) self.assertTrue(hasattr(form, "maildomain")) - self.assertEqual(form.maildomain, maildomain) + self.assertEqual(form.maildomain, self.mail_domain) self.assertTrue(hasattr(form, "helper")) self.assertEqual( form.helper.form_action, reverse( "edit_mailaddress", - kwargs={"package": 42, "domain": "example.org", "pk": 23}, + kwargs={"package": 42, "domain": "example.org", "pk": self.instance.pk}, ), ) self.assertEqual(len(form.helper.layout), 2) self.assertEqual(form.helper.layout[1].name, "submit") - @skip("needs mailbox refactoring") def test_clean_no_mailbox_choice(self): - instance = MagicMock(id=23) - osuser = Mock(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) - maildomain = MagicMock(domain="example.org") form = EditMailAddressForm( - instance=instance, - maildomain=maildomain, - hostingpackage=hostingpackage, + instance=self.instance, + maildomain=self.mail_domain, + hostingpackage=self.hosting_package, data={"mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox}, ) self.assertFalse(form.is_valid()) self.assertIn("mailbox", form.errors) - @skip("needs mailbox refactoring") def test_clean_no_forward_address_choice(self): - instance = MagicMock(id=23) - osuser = Mock(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) - maildomain = MagicMock(domain="example.org") form = EditMailAddressForm( - instance=instance, - maildomain=maildomain, - hostingpackage=hostingpackage, + instance=self.instance, + maildomain=self.mail_domain, + hostingpackage=self.hosting_package, data={"mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards}, ) self.assertFalse(form.is_valid()) self.assertIn("forwards", form.errors) def test_save_with_forwards_no_commit(self): - maildomain = MailDomain.objects.create(domain="example.org") - instance = MailAddress(id=23, domain=maildomain) - osuser = osusers.models.User(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) form = EditMailAddressForm( - instance=instance, - maildomain=maildomain, - hostingpackage=hostingpackage, + instance=self.instance, + maildomain=self.mail_domain, + hostingpackage=self.hosting_package, data={ "mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards, "forwards": "test2@example.org,test3@example.org", @@ -451,15 +410,10 @@ class EditMailAddressFormTest(TestCase): form.save(commit=False) def test_save_with_forwards_commit(self): - osuser = osusers.models.User(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) - mail_domain = MailDomain.objects.create(domain="example.org") - instance = MailAddress(id=23, domain=mail_domain) - form = EditMailAddressForm( - instance=instance, - maildomain=mail_domain, - hostingpackage=hostingpackage, + instance=self.instance, + maildomain=self.mail_domain, + hostingpackage=self.hosting_package, data={ "mailbox_or_forwards": MAILBOX_OR_FORWARDS.forwards, "forwards": "test2@example.org,test3@example.org", @@ -468,64 +422,55 @@ class EditMailAddressFormTest(TestCase): self.assertTrue(form.is_valid()) form.save(commit=True) - @skip("needs mailbox refactoring") def test_save_with_mailbox_no_commit(self): - instance = MagicMock(id=23) - osuser = Mock(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) - maildomain = MagicMock(domain="example.org") + mail_box = create_test_mailbox(os_user=self.os_user) + form = EditMailAddressForm( - instance=instance, - maildomain=maildomain, - hostingpackage=hostingpackage, + instance=self.instance, + maildomain=self.mail_domain, + hostingpackage=self.hosting_package, data={ "mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox, - "mailbox": "mailbox23", + "mailbox": mail_box, }, ) self.assertTrue(form.is_valid()) - mailbox = MagicMock(osuser=osuser, username="testuserp01") - instance.set_mailbox.return_value = mailbox form.save(commit=False) - instance.set_mailbox.assert_called_with(ANY, False) - mailbox.save.assert_not_called() - instance.save.assert_not_called() - @skip("needs mailbox refactoring") + self.instance.refresh_from_db() + self.assertFalse(hasattr(self.instance, "mailaddressmailbox")) + def test_save_with_mailbox_commit(self): - instance = MailAddress(id=23) - osuser = osusers.models.User(username="testuser") - hostingpackage = MagicMock(id=42, osuser=osuser) - mail_domain = MailDomain.objects.create(domain="example.org") + mail_box = create_test_mailbox(os_user=self.os_user) + form = EditMailAddressForm( - instance=instance, - maildomain=mail_domain, - hostingpackage=hostingpackage, + instance=self.instance, + maildomain=self.mail_domain, + hostingpackage=self.hosting_package, data={ "mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox, - "mailbox": "mailbox23", + "mailbox": mail_box, }, ) self.assertTrue(form.is_valid()) form.save(commit=True) - @skip("needs mailbox refactoring") + self.instance.refresh_from_db() + self.assertEqual(self.instance.mailaddressmailbox.mailbox, mail_box) + def test_save_with_other_choice(self): - mail_domain = MailDomain.objects.create(domain="example.org") - instance = MailAddress(id=23, domain=mail_domain) - os_user = osusers.models.User(username="testuser") - hosting_package = MagicMock(id=42, osuser=os_user) + mail_box = create_test_mailbox(os_user=self.os_user) + form = EditMailAddressForm( - instance=instance, - maildomain=mail_domain, - hostingpackage=hosting_package, + instance=self.instance, + maildomain=self.mail_domain, + hostingpackage=self.hosting_package, data={ "mailbox_or_forwards": MAILBOX_OR_FORWARDS.mailbox, - "mailbox": "mailbox23", + "mailbox": mail_box, }, ) self.assertTrue(form.is_valid()) + form.cleaned_data["mailbox_or_forwards"] = -1 form.save(commit=True) - instance.set_mailbox.assert_not_called() - instance.save.assert_called_with() diff --git a/gnuviechadmin/osusers/tests/testutils.py b/gnuviechadmin/osusers/tests/testutils.py new file mode 100644 index 0000000..dcee73e --- /dev/null +++ b/gnuviechadmin/osusers/tests/testutils.py @@ -0,0 +1,13 @@ +from django.contrib import auth + +from osusers.models import User + + +def create_test_customer(): + customer_model = auth.get_user_model() + return customer_model.objects.create_user("test_customer", "testuser@example.org", "test_password") + + +def create_test_user(): + customer = create_test_customer() + return User.objects.create_user(customer)