#255 Migration of TestCases to Pytest and Addition of new TestCase
Merged 7 months ago by jflory7. Opened 7 months ago by alishapapun.
fedora-commops/ alishapapun/fedora-happiness-packets add/pytest-migrations  into  master

@@ -11,15 +11,19 @@ 

  from django.urls import reverse

  from django.db.models import Q

  from django.utils import timezone

- from email_normalize import normalize

  import bleach

  

  from .models import Message, strip_email

  

  logger = logging.getLogger(__name__)

  

+ def check_recipient_is_sender(sender_emails, recipient_emails) -> bool:

+     if not (set(sender_emails).isdisjoint(recipient_emails)):

+         return True

+     else:

+         return False

  

- def validate_email(email):

+ def validate_email_is_rate_limited(email):

      timeframe = timezone.now() - timedelta(hours=24)

      stripped_email = strip_email(email)

      recent_message_count = Message.objects.filter(Q(sender_email_stripped=stripped_email) | Q(recipient_email_stripped=stripped_email), created__gte=timeframe).count()

@@ -46,7 +50,7 @@ 

  

          self.fields['recipient_name'].label = 'Name'

          self.fields['recipient_email'].label = 'Email'

-         self.fields['recipient_email'].validators = [validate_email]

+         self.fields['recipient_email'].validators = [validate_email_is_rate_limited]

          self.fields['message'].help_text = 'Writer\'s block? Check out our <a href="%s">message inspiration</a>.' % reverse('messaging:inspiration')

          self.fields['sender_named'].label = 'I agree to share my name and email address with the recipient.'

          self.fields['sender_approved_public'].label = "I agree to publish this message and display it publicly in the Happiness Archive."

@@ -62,20 +66,6 @@ 

              HTML("<br>"),

              Submit('submit', 'Send some happiness', css_class='btn-lg centered'),

          )

- 

-     def is_recipient_email_equals_sender_email(self):

-         recipient_email = self.cleaned_data.get('recipient_email')

-         sender_email = self.user.email

-         sender_username = self.user.username

-         normalized_sender_email = normalize(sender_email)

-         normalized_recipient_email = normalize(recipient_email)

-         #Fedora assigned email to the Sender

-         sender_fedora_email = sender_username + '@fedoraproject.org'

-         if normalized_recipient_email in (

-             sender_email, sender_fedora_email, normalized_sender_email):

-             return True

-         else:

-             return False

      

      def clean_message(self): 

          """ Cleans given HTML with bleach.clean() """

@@ -106,19 +96,28 @@ 

              attributes=allowed_attributes, 

              styles=allowed_styles

          ) 

-     

+ 

      def clean(self):

          super(MessageSendForm, self).clean()

-         isREEqualsSE = self.is_recipient_email_equals_sender_email()

-         # if self.cleaned_data.get('hp'):

-         #     raise forms.ValidationError('')

-         if self.cleaned_data.get('sender_approved_public_named') and not self.cleaned_data.get('sender_approved_public'):

-             self.add_error('sender_approved_public_named', "If you want us to publish the message including your names, "

-                                                            "you must also check 'I agree to publish this message and"

-                                                            "display it publicly in the Happiness Archive'")

-         if isREEqualsSE:

-             raise forms.ValidationError("You cannot send a Fedora Happiness Packet to yourself!")

-         validate_email(self.user.email)

+         sender_emails = []

+         sender_emails.append(self.user.email)

+         sender_emails.append(strip_email(self.user.email))

+         sender_emails.append(self.user.username + '@fedoraproject.org')

+         recipient_emails = []

+         recipient_email = self.cleaned_data.get('recipient_email')

+         recipient_emails.append(recipient_email)

+         if recipient_email:                     #Before clean(), validation is performed, which removes recipient_email field if validation is failed

+             recipient_emails.append(strip_email(recipient_emails[0]))

+ 

+             if check_recipient_is_sender(sender_emails, recipient_emails):

+                 raise forms.ValidationError("You cannot send a Fedora Happiness Packet to yourself!")

+             elif self.cleaned_data.get('sender_approved_public_named') and not self.cleaned_data.get('sender_approved_public'):

+                 self.add_error('sender_approved_public_named', "If you want us to publish the message including your names, "

+                                                                 "you must also check 'I agree to publish this message and"

+                                                                 "display it publicly in the Happiness Archive'")

+         validate_email_is_rate_limited(self.user.email)

+ 

+     

  

  class MessageRecipientForm(forms.ModelForm):

      class Meta:

@@ -0,0 +1,19 @@ 

+ from ..forms import check_recipient_is_sender

+ from django.test import TestCase

+ 

+ 

+ class TestIsReceipientEqualsSenderEmail(TestCase):

+ 

+     def test_should_return_true_for_same_sender_and_recipient(self):

+         sender_email = []

+         recipient_email = []

+         sender_email.append('user@gmail.com')

+         recipient_email.append('user@gmail.com')

+         self.assertTrue(check_recipient_is_sender(sender_email, recipient_email))

+ 

+     def test_should_return_false_for_different_sender_and_recipient(self):

+         sender_email = []

+         recipient_email = []

+         sender_email.append('user+sender@gmail.com')

+         recipient_email.append('user2+fedora@gmail.com')

+         self.assertFalse(check_recipient_is_sender(sender_email, recipient_email)) 

\ No newline at end of file

@@ -1,9 +1,7 @@ 

  # -*- coding: utf-8 -*-

  from __future__ import unicode_literals

- 

  import factory

- from django.test import TestCase

- 

+ import pytest

  

  class MessageModelFactory(factory.DjangoModelFactory):

      class Meta:

@@ -24,9 +22,11 @@ 

      email = 'emailemail@null'

      confirmation_ip = '127.0.0.1'

  

- 

- class MessageModelTest(TestCase):

+ @pytest.mark.django_db

This is a handy decorator. :smile:

+ class TestMessageModel:

+     

      def test_unique_identifier(self):

          obj1 = MessageModelFactory()

          obj2 = MessageModelFactory()

-         self.assertNotEqual(obj1.identifier, obj2.identifier)

+         assert obj1.identifier != obj2.identifier

+ 

@@ -14,14 +14,11 @@ 

  from ..models import Message, BLACKLIST_HMAC_SALT, BlacklistedEmail

  

  

- class SearchViewTest(TestCase):

+ class TestSearchView(TestCase):

      url = reverse('messaging:search')

  

      def test_anonymous_message_indexed(self):

-         MessageModelFactory(sender_approved_public=True, sender_approved_public_named=False,

-                             recipient_approved_public=True, recipient_approved_public_named=True,

-                             admin_approved_public=True)

-         msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=True,

+         msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=False,

                                    recipient_approved_public=True, recipient_approved_public_named=False,

                                    admin_approved_public=True)

          update_index.Command().handle(using=['default'])

@@ -33,6 +30,18 @@ 

          self.assertNotContains(response, msg.sender_email)

          self.assertNotContains(response, msg.recipient_email)

  

+     def test_pagination_is_ten(self):

+         number_of_msgs = 11

+         for msg in range(number_of_msgs):

+             msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=True,

+                                   recipient_approved_public=True, recipient_approved_public_named=True,

+                                   admin_approved_public=True)

+         response = self.client.get(self.url,{'q':msg.message})

+         self.assertEqual(response.status_code, 200)

+         self.assertTrue('is_paginated' in response.context)

+         self.assertTrue(response.context['is_paginated'] == True)

+         self.assertTrue(len(response.context['object_list']) == 10)

+ 

      def test_named_message_indexed(self):

          msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=True,

                                    recipient_approved_public=True, recipient_approved_public_named=True,

@@ -61,7 +70,7 @@ 

          self.assertEqual(response.status_code, 200)

          self.assertEqual(len(response.context['object_list']),0)

  

- class MessageCounterTest(TestCase):

+ class TestMessageCounter(TestCase):

      url = reverse('messaging:start')

  

      def test_message_sent_included(self):

@@ -80,7 +89,7 @@ 

          self.assertEqual(response.context['packets_sent'],0)

  

  

- class StartViewTest(TestCase):

+ class TestStartView(TestCase):

      url = reverse('messaging:start')

  

      def test_renders(self):

@@ -99,7 +108,7 @@ 

          self.assertNotContains(response, msg.recipient_email)

  

  

- class FaqViewTest(TestCase):

+ class TestFaqView(TestCase):

      url = reverse('messaging:faq')

  

      def test_renders(self):

@@ -107,7 +116,7 @@ 

          self.assertEqual(response.status_code, 200)

  

  

- class InspirationViewTest(TestCase):

+ class TestInspirationView(TestCase):

      url = reverse('messaging:inspiration')

  

      def test_renders(self):

@@ -115,7 +124,7 @@ 

          self.assertEqual(response.status_code, 200)

  

  

- class ArchiveViewTest(TestCase):

+ class TestArchiveView(TestCase):

      url = reverse('messaging:archive')

  

      def test_renders_no_public_messages(self):

@@ -160,7 +169,7 @@ 

          self.assertNotContains(response, msg.recipient_email)

  

  

- class BlacklistViewTest(TestCase):

+ class TestBlacklistView(TestCase):

      url_name = 'messaging:blacklist_email'

  

      def setUp(self):

@@ -190,11 +199,11 @@ 

          self.assertFalse(BlacklistedEmail.objects.count())

  

  

- class SendViewTest(TestCase):

+ class TestSendView(TestCase):

      url = reverse('messaging:send')

  

      def setUp(self):

-         super(SendViewTest, self).setUp()

+         super(TestSendView, self).setUp()

          self.post_data = {

              'sender_name': 'sender name',

              'sender_email': 'SEN.DER+FOOBAR@erik.io',

@@ -205,9 +214,7 @@ 

              'sender_approved_public': True,

              'sender_approved_public_named': True,

          }

-         self.user = User.objects.create_user("erikio", "sender@erik.io", "helloworld", first_name="Erik",last_name="Doe")

-         self.user.save()

-         self.client.login(username = "erikio", password = "helloworld")

+         self.client.force_login(User.objects.get_or_create(username="erikio",email="sender@erik.io", password="helloworld", first_name="Erik",last_name="Doe")[0])

  

      def test_renders(self):

          response = self.client.get(self.url)

@@ -229,6 +236,7 @@ 

          response = self.client.post(self.url, self.post_data)

          self.assertEqual(response.status_code, 200)

          self.assertEqual(len(response.context['form'].errors), 1)

+         self.assertFormError(response, 'form', 'sender_approved_public_named', "If you want us to publish the message including your names, you must also check 'I agree to publish this message anddisplay it publicly in the Happiness Archive'")

          self.assertEqual(len(mail.outbox), 0)

  

      def test_post_blacklisted_sender(self):

@@ -243,6 +251,7 @@ 

          response = self.client.post(self.url, self.post_data)

          self.assertEqual(response.status_code, 200)

          self.assertEqual(len(response.context['form'].errors), 1)

+         self.assertFormError(response, 'form', field=None, errors="We can't send emails to this address at this time. You can try again in 24 hours.")

          self.assertEqual(len(mail.outbox), 0)

  

      def test_post_ratelimited_recipient(self):

@@ -251,10 +260,18 @@ 

          response = self.client.post(self.url, self.post_data)

          self.assertEqual(response.status_code, 200)

          self.assertEqual(len(response.context['form'].errors), 1)

+         self.assertFormError(response, 'form', 'recipient_email', "We can't send emails to this address at this time. You can try again in 24 hours.")

          self.assertEqual(len(mail.outbox), 0)

  

+     def test_sender_email_equals_recipient_email(self):

+         self.post_data['recipient_email'] = 'SEN.DER+FOOBAR@erik.io'

+         response = self.client.post(self.url, self.post_data)

+         self.assertEqual(response.status_code, 200)

+         self.assertEqual(len(response.context['form'].errors), 1)

+         self.assertFormError(response, 'form', field=None, errors="You cannot send a Fedora Happiness Packet to yourself!")

+         self.assertEqual(len(mail.outbox), 0)

  

- class MessageSentViewTest(TestCase):

+ class TestMessageSentView(TestCase):

      url = reverse('messaging:sender_confirmation_sent')

  

      def test_renders(self):

@@ -262,10 +279,11 @@ 

          self.assertEqual(response.status_code, 200)

  

  

- class MessageSenderConfirmationView(TestCase):

+ class TestMessageSenderConfirmationView(TestCase):

      url_name = 'messaging:sender_confirm'

  

      def setUp(self):

+         self.client.force_login(User.objects.get_or_create(username="erikio",email="sendersender@null", password="helloworld")[0])

          self.message = MessageModelFactory(sender_email_token='a-b-c', status=Message.STATUS.pending_sender_confirmation)

          url_kwargs = {'identifier': self.message.identifier, 'token': self.message.sender_email_token}

          self.url = reverse(self.url_name, kwargs=url_kwargs)

@@ -301,7 +319,7 @@ 

          self.message.save()

  

          response = self.client.get(self.url)

-         self.assertEqual(response.status_code, 200)

+         self.assertEqual(response.status_code, 404)

          self.assertTrue(response.context['not_found'])

  

      def test_bad_status(self):

@@ -319,7 +337,7 @@ 

          self.assertEqual(len(mail.outbox), 0)

  

  

- class MessageSenderConfirmedView(TestCase):

+ class TestMessageSenderConfirmedView(TestCase):

      url = reverse('messaging:sender_confirmed')

  

      def test_renders(self):

@@ -327,7 +345,7 @@ 

          self.assertEqual(response.status_code, 200)

  

  

- class MessageRecipientMessageUpdate(TestCase):

+ class TestMessageRecipientMessageUpdate(TestCase):

      url_name = 'messaging:recipient_message_update'

  

      def setUp(self):

@@ -36,8 +36,7 @@ 

  class MessageSearchView(SearchView):

      template_name = 'search/search.html'

      form_class = SearchForm

-     paginate_by = 5

- 

+     paginate_by = 10

  class ArchiveListView(ListView):

      model = Message

      paginate_by = 5

@@ -151,13 +150,17 @@ 

          try:

              message = Message.objects.get(identifier=kwargs['identifier'], sender_email_token=kwargs['token'])

          except Message.DoesNotExist:

-             return render(request, self.template_name, {'not_found': True})

- 

+             response = render(request, self.template_name, {'not_found': True})

+             response.status_code = 404

+             return response

+  

          if message.sender_email != self.request.user.email:

-             return render(request, self.template_name, {'not_sender': True})

+             response = render(request, self.template_name, {'not_sender': True})

+             response.status_code = 400

+             return response

          if message.status != Message.STATUS.pending_sender_confirmation:

-             return render(request, self.template_name, {'already_confirmed': True})

- 

+             response = render(request, self.template_name, {'already_confirmed': True})

+             return response

          message.send_to_recipient(self.request.is_secure(), self.request.get_host())

  

          sender_name = self.request.user.username if message.sender_named else "Anonymous"

@@ -173,9 +176,13 @@ 

          try:

              publish(message)

          except PublishReturned:

-             return render(request, self.template_name, {'publish_returned': True})

+             response = render(request, self.template_name, {'publish_returned': True})

+             response.status_code = 500

+             return response

          except ConnectionException:

-             return render(request, self.template_name, {'connection_exception': True})

+             response = render(request, self.template_name, {'connection_exception': True})

+             response.status_code = 500

+             return response

          return HttpResponseRedirect(reverse('messaging:sender_confirmed'))

  

  

This PR aims to migrate existing tests to Pytest, pass the existing failing tests and add new test cases.

Metadata Update from @jflory7:
- Pull-request tagged with: PASSED, needs testing, new change, type - quality assurance, type - summer coding

7 months ago

In commit 3648d34, why did you remove paginate_by = 5 in views.py? I did not follow this change.

Oh, I wonder why this was a 200 status code originally. Seems like a bug in the original test? :sweat_smile:

Just curious, why does response.status_code need to be split out separately?

Is there a reason you used unittest over django.test here? I ask because I was not sure myself and noticed django.test is used by other tests.

This is minor, but receipient => recipient.

Hey @alishapapun, nice work! :raised_hands:

This is almost ready to merge. I have a few questions above, and I also hoped you could walk me through commit ffc41ce.

It was awesome to see all of the test cases passing again. :smile: This should close #128.

bash-4.4# pytest
========================================================================= test session starts ==========================================================================
platform linux -- Python 3.6.8, pytest-4.6.3, py-1.8.0, pluggy-0.12.0
Django settings: happinesspackets.settings.dev (from environment variable)
rootdir: /app, inifile: setup.cfg
plugins: django-3.5.0, celery-4.2.1, cov-2.7.1
collected 63 items                                                                                                                                                     

happinesspackets/messaging/tests/test_models.py .                                                                                                                [  1%]
happinesspacket_schema_package/happinesspacket_schema/tests/test_schema.py ...........                                                                           [ 19%]
happinesspackets/messaging/tests/test_forms.py ..                                                                                                                [ 22%]
happinesspackets/messaging/tests/test_urls.py ..............                                                                                                     [ 44%]
happinesspackets/messaging/tests/test_views.py ..................................                                                                                [ 98%]
happinesspackets/utils/tests/test_misc.py .                                                                                                                      [100%]

Please take a look at my comments above. Pytest is also fairly new to me, so I am still learning too. I would also appreciate if you could explain what commit ffc41ce is doing, since I was not able to follow everything happening there.

Metadata Update from @jflory7:
- Pull-request untagged with: needs testing
- Pull-request tagged with: needs info
- Request assigned

7 months ago

2 new commits added

  • Resolve typo in TestIsReceipientEqualsSenderEmail and change unittest to django.test
  • Add test case for Pagination in Search View
7 months ago

In commit 3648d34, why did you remove paginate_by = 5 in views.py? I did not follow this change.

Earlier, the test test_named_message_indexed was not passing because of the paginate_by = 5, so I thought to remove it, but it seems pagination is necessary. So, added pagination in the latest commit and also added a test case test_pagination_is_ten to check if the search page is paginated. And the ensured the tests pass. Do give it a check @jflory7 :)

Oh, I wonder why this was a 200 status code originally. Seems like a bug in the original test? 😅

The response generated has status_code 200 initially, so in order to pass the test case, it's checked against status_code 200. So, I explicitly changed the status_code to 404 in case the Message doesn't exist. .

This is minor, but receipient => recipient.

Rectified the typo. 😅 in new commits

Is there a reason you used unittest over django.test here? I ask because I was not sure myself and noticed django.test is used by other tests.

Not a specific reason Justin. Since its better to maintain uniformity in all test cases I switched to django.test from unittest in the latest commits.

Hey @alishapapun, nice work! 🙌

Thanks @jflory7

This is almost ready to merge. I have a few questions above, and I also hoped you could walk me through commit ffc41ce.

See, in the commit earlier, the recipient_email was assigned to None while testing causing the TestCase to fail. Even if the request contain the recipient_email which was passed on to the forms method, it undergoes validation (validate_email_is_rate_limited) thus deleting the recipient_email value. Since validation is called before clean method, no entires of recipient_email is present when the scope comes to clean method, thus recipient_email value is assigned to None. (This type of error is not encountered by test_post_ratelimited_sender
since there was no validation. So, I put an if condition here which checks if the recipient_email value is present or not. I renamed the method validate-email to validate_email_is_rate_limited since, the method checks if messages count exceeds MAX_MESSAGES. Am I able to make you understand? Do let me know where more clarification is needed.

It was awesome to see all of the test cases passing again. 😄 This should close #128.
bash-4.4# pytest
========================================================================= test session starts ==========================================================================
platform linux -- Python 3.6.8, pytest-4.6.3, py-1.8.0, pluggy-0.12.0
Django settings: happinesspackets.settings.dev (from environment variable)
rootdir: /app, inifile: setup.cfg
plugins: django-3.5.0, celery-4.2.1, cov-2.7.1
collected 63 items

happinesspackets/messaging/tests/test_models.py . [ 1%]
happinesspacket_schema_package/happinesspacket_schema/tests/test_schema.py ........... [ 19%]
happinesspackets/messaging/tests/test_forms.py .. [ 22%]
happinesspackets/messaging/tests/test_urls.py .............. [ 44%]
happinesspackets/messaging/tests/test_views.py .................................. [ 98%]
happinesspackets/utils/tests/test_misc.py . [100%]

Please take a look at my comments above. Pytest is also fairly new to me, so I am still learning too. I would also appreciate if you could explain what commit ffc41ce is doing, since I was not able to follow everything happening there.

@alishapapun:
See, in the commit earlier, the recipient_email was assigned to None while testing causing the TestCase to fail. Even if the request contain the recipient_email which was passed on to the forms method, it undergoes validation (validate_email_is_rate_limited) thus deleting the recipient_email value. Since validation is called before clean method, no entires of recipient_email is present when the scope comes to clean method, thus recipient_email value is assigned to None. (This type of error is not encountered by test_post_ratelimited_sender since there was no validation.

Nice work on figuring out the root cause. :thumbsup:

@alishapapun:
So, I put an if condition here which checks if the recipient_email value is present or not. I renamed the method validate-email to validate_email_is_rate_limited since, the method checks if messages count exceeds MAX_MESSAGES. Am I able to make you understand? Do let me know where more clarification is needed.

I follow now. While this change makes the test pass, it comes at a cost of adding complexity. It is harder to understand what the clean() method is doing in forms.py. Adding if recipient_email gets the test to pass but does not solve the underlying problem.

I refactored two methods, clean() and is_recipient_email_equals_sender_email(). I'll write them first and then explain what I did:

def clean(self):
    super(MessageSendForm, self).clean()

    sender_emails = []
    sender_emails.append(self.user.email)
    sender_emails.append(strip_email(self.user.email))
    sender_emails.append(self.user.username + '@fedoraproject.org')
    recipient_emails = []
    recipient_emails.append(self.cleaned_data.get('recipient_email'))
    recipient_emails.append(strip_email(recipient_emails[0])

    if check_recipient_is_sender(sender_emails, recipient_emails):
        raise forms.ValidationError(
            "You cannot send a Fedora Happiness Packet to yourself!")
    elif self.cleaned_data.get('sender_approved_public_named') and not self.cleaned_data.get('sender_approved_public'):
        self.add_error('sender_approved_public_named', "If you want us to publish the message including your names, "
                                                        "you must also check 'I agree to publish this message and"
                                                        "display it publicly in the Happiness Archive'")
    validate_email_is_rate_limited(self.user.email)

def check_recipient_is_sender(sender_emails: List[str], recipient_email: List[str]) -> bool:
    if not set(sender_emails).isdisjoint(recipient_emails)
        return True
    else:
        return False

I made the check_recipient_is_sender() compare two lists for any common elements. In the clean() method, we add all sender and recipient emails to their own lists. Then we pass these lists to check_recipient_is_sender(). This makes check_recipient_is_sender() easier to test and also makes the code easier to read.

Also, I used a cool feature of Python 3.5 called typing (docs here, cheatsheet here). Typing creates type hints for expected data types. Since we always expect two lists in the above example and the method always returns a boolean, we can use typing for writing for robust unit tests (e.g. some tools will make sure object types are always correct). See this article for a longer explanation for why.

Lastly, to check if the two lists have overlapping content, I used isdisjoint() from Python built-in types (docs here). This is a better-performing way of comparing contents of two sets in Python.

This is a cleaner way of writing these two methods. It should make it easier to test, if it does not work with the existing tests already. I did not run the test suite with these changes. We should get the test to pass with the way these are written above. ^^

Does my feedback make sense? It was late when I typed this out, so let me know if you have doubts or if something is confusing.

@jflory7:
In commit 3648d34, why did you remove paginate_by = 5 in views.py? I did not follow this change.

@alishapapun:
Earlier, the test test_named_message_indexed was not passing because of the paginate_by = 5, so I thought to remove it, but it seems pagination is necessary. So, added pagination in the latest commit and also added a test case test_pagination_is_ten to check if the search page is paginated. And the ensured the tests pass. Do give it a check @jflory7 :)

Thanks for writing a test for this one! :thumbsup: It makes sense to me. However, in the Happiness Archive, pagination happens every five packets. Do you know why this is? I have a screenshot below from when I tested locally:

Screenshot of Happiness Archive page, where pages are paginated by every five messages

1 new commit added

  • Modify clean method in forms.py
7 months ago

13 new commits added

  • Make testcase robust by checking with correct error messages
  • Modify clean method in forms.py
  • Resolve typo in TestIsReceipientEqualsSenderEmail and change unittest to django.test
  • Add test case for Pagination in Search View
  • Add unittest for TestIsReceipientEqualsSenderEmail
  • Add status code to response in MessageSenderConfirmationView
  • Test sender email equals to recipient email
  • Pass the TestClass TestMessageSenderConfirmationView
  • Pass the TestCase function of test_post_ratelimited_sender
  • In Testing TestSendView make a user login by using force_login
  • Migrate test_models.py to pytest
  • Passing Test Class TestSearchView
  • Rename Class names to match Pytest requirements
7 months ago

Squashed few commits, so had to force push the commits, now it turns the PR is now flooded with commits. :(

@alishapapun Props again on figuring this one out, this was tricky. :grin:

To clarify, is this PR ready for final review or is it still a work-in-progress? If it's ready, I can run through it Thursday evening US CDT time zone.

@alishapapun Props again on figuring this one out, this was tricky. 😁
To clarify, is this PR ready for final review or is it still a work-in-progress? If it's ready, I can run through it Thursday evening US CDT time zone.

It would be great if you test it once more since I just changed the test case for test_forms.py. After, its done, its all ready to get merged. And for the FASID-Search, I guess opening a new PR will be a good idea. What do you think @jflory7 ?

1 new commit added

  • Update the testcase in test_forms.py
7 months ago

Metadata Update from @jflory7:
- Pull-request untagged with: needs info
- Request assigned

7 months ago

I ran through the newest changes and all tests pass. The tests are sensible to me. I believe we are ready for merging in the Pytest test suite! :raised_hands: Excellent work on turning this PR around @alishapapun. You covered a lot of ground and learned a few different skills to pull this off. I hope you are proud of your work here too.

Before merging, could you please squash all commits down to one and write a summary of changes in the single squashed commit? Once the commits are squashed, we can merge it! :smile:

rebased onto ec61203

7 months ago

This commit address the following changes :100:
- Modify clean method in forms.py
- Add test case for Pagination in Search View
- Add unit test for check_recipient_is_sender (Test sender email equals to recipient email)
- Pass the TestClass TestMessageSenderConfirmationView
- Pass the TestCase function of test_post_ratelimited_sender
- Work on user authentication (previously not working) in Testing
- Migrate tests to pytest
- Rename Tests to match Pytest norms.
I guess all good for merging :ocean: @jflory7 :)

Super. Let's ride that wave right into master! :sunglasses: :joy:

Merging!

Pull-Request has been merged by jflory7

7 months ago