Feature/create unit tests #42

serhii73 · 2018-08-15T09:23:07Z

No description provided.

.pre-commit-config.yaml

blog/tests/models_test.py

webknjaz · 2018-08-15T21:09:50Z

blog/tests/models_test.py

@@ -0,0 +1,64 @@
+"""Tests for Post model's Blog app."""


why does blog belong to model?

"""Tests for models of Blog app."""

My decision.

webknjaz · 2018-08-15T21:10:23Z

blog/tests/models_test.py

+
+
+class PostModelTest(TestCase):
+    """Tests for Post model."""


Anything specific, except for copy-paste from module docstring?

This is still ignored.

In top

"""Tests for models of Blog app."""

Doc's class

"""Tests for Post model."""

Now I think it good. Maybe I still wrong.

blog/tests/models_test.py

webknjaz · 2018-09-01T10:15:31Z

Any progress on this?

blog/tests/views_test.py

webknjaz · 2018-11-19T00:59:52Z

Ref #12

serhii73 · 2018-11-19T10:02:08Z

@webknjaz I think now all good. Can we to merge this branch to master?

webknjaz · 2018-11-19T12:08:51Z

Now, I'll review it again.

webknjaz · 2018-11-19T12:09:52Z

blog/tests/models_test.py

@@ -0,0 +1,69 @@
+"""Tests for models' Blog app."""


What is a blog app of the models?

"""Tests for models of Blog app."""

Fixed

blog/tests/__init__.py

webknjaz · 2018-11-19T12:35:05Z

blog/tests/models_test.py

+        """Clean-up test data."""
+        del self.post
+
+    def test_title_label(self):


This and further similar tests test that Django's internal ORM classes work (which is not needed), they don't check whether the code that you wrote works.

webknjaz · 2018-11-19T12:37:00Z

blog/tests/models_test.py

+        self.assertEqual(field_label, 'published date')
+
+    def test_str_method(self):
+        """Test for __str__ method."""


__str__ is an essential part of stringification protocol, invoked internally when str() function is called on objects.
So you're supposed to test that the class supports that protocol well by using the protocol.

def test_right_title(self): """Test right title output.""" assert str(self.post) == "Title"

But I think it needs to remove, I think I do not need to test Django functional. Or not?

I thought you added __str__ method to your model

webknjaz · 2018-11-19T12:37:54Z

blog/tests/models_test.py

+
+    def test_str_method(self):
+        """Test for __str__ method."""
+        self.assertEqual('Title', self.post.__str__())


Also, the requirement to use assert statement is global. Style should be the same across the project, otherwise, it's hard to read it.

Removed assertEqual

webknjaz · 2018-11-19T14:07:56Z

blog/tests/views_test.py

+        del self.post_third
+
+    def test_post_list(self):
+        """Test show post list on the page."""


No, test that appropriate posts listed in the correct order.

def test_post_list_correct_order(self): """Test show post list on the page and test correct order.""" response = self.client.get(reverse('post_list')) assert response.status_code == 200 self.assertTemplateUsed( response, 'blog/post_list.html', 'blog/base.html' ) assert len(response.context['posts']) == 2 assert [ ['Title Bob', 'Text bob'], ['Title John', 'Text John'], ['Title John 2', 'Text John 2'], ] == [[i.title, i.text] for i in Post.objects.all()] assert [ ['Title John', 'Text John'], ['Title Bob', 'Text bob'], ['Title John 2', 'Text John 2'], ] == [ [i.title, i.text] for i in Post.objects.order_by('published_date') ]

webknjaz · 2018-11-19T14:08:54Z

blog/tests/views_test.py

+    def test_post_detail(self):
+        """Test post detail."""
+        response = self.client.get(reverse('post_detail', kwargs={'pk': 1}))
+        self.assertEqual(response.status_code, 200)


Removed assertEqual

webknjaz · 2018-11-19T14:09:19Z

blog/tests/views_test.py

+        self.assertEqual(len(response.context['posts']), 2)
+
+    def test_post_detail(self):
+        """Test post detail."""


post contents displayed correctly

webknjaz · 2018-11-19T14:10:31Z

blog/tests/views_test.py

+        response = self.client.post(
+            reverse('post_new'),
+            {
+                'author': paul,


You should probably use a string value, not some object.

I have not understood this moment :(
Please tell me more what I need to do, thank you.

paul.username

webknjaz · 2018-11-19T14:11:18Z

blog/tests/views_test.py

+        self.assertIn(b'Title Bob', response.content)
+        self.assertIn(b'Text bob', response.content)
+
+        self.assertEqual(str(response.context['post']), 'Title Bob')


Why not compare to the post object?

webknjaz · 2018-11-21T12:40:16Z

blog/tests/__init__.py

@@ -0,0 +1 @@
+"""This package contains tests for the models and views of Blog app."""


Suggested change

"""This package contains tests for the models and views of Blog app."""

"""Tests for the models and views of the Blog app."""

webknjaz · 2018-11-21T12:40:36Z

blog/tests/models_test.py

+
+
+class PostModelTest(TestCase):
+    """Tests for Post model."""


This is still ignored.

webknjaz · 2018-11-21T14:16:06Z

blog/tests/models_test.py

+        self.assertEqual(field_label, 'published date')
+
+    def test_str_method(self):
+        """Test for __str__ method."""


webknjaz · 2018-11-21T14:16:19Z

blog/tests/models_test.py

+        ),
+    )
+    def test_post_publish(self):
+        """Test for publish method."""


blog/tests/models_test.py

webknjaz · 2018-11-21T14:24:11Z

blog/tests/views_test.py

+            str.encode(Post.objects.filter(pk=1)[0].text), response.content
+        )
+        assert (
+            str(response.context['post']) == Post.objects.filter(pk=1)[0].title


Use a var for showing what you try to do.

webknjaz · 2018-11-21T14:25:37Z

blog/tests/views_test.py

+            str.encode(Post.objects.filter(pk=1)[0].title), response.content
+        )
+        self.assertIn(
+            str.encode(Post.objects.filter(pk=1)[0].text), response.content


Don't use unbound version of str method, just call it on the str itself. Also, don't query the DB for random things. Tests shouldn't do magical stuff.

webknjaz · 2018-11-21T14:25:51Z

blog/tests/views_test.py

+            response, 'blog/post_detail.html', 'blog/base.html'
+        )
+
+        self.assertIn(


assert .. in ..

webknjaz · 2018-11-21T14:26:15Z

blog/tests/views_test.py

+        response = self.client.post(
+            reverse('post_edit', kwargs={'pk': 1}),
+            {
+                'author': paul,


Still there.

webknjaz · 2018-11-21T14:27:09Z

blog/tests/views_test.py

+        self.assertIn(b'Post with post edit', response.content)
+        self.assertIn(b'Text with post edit', response.content)
+
+        response = self.client.get(reverse('post_edit', kwargs={'pk': 1}))


What do you test here?

@webknjaz Maybe now all good.

webknjaz · 2018-11-21T14:28:02Z

blog/tests/models_test.py

+
+    def test_str_method(self):
+        """Test for __str__ method."""
+        assert self.post.__str__() == 'Title'


Still wrong: see the explanation above.

serhii73 · 2018-11-23T11:27:24Z

@webknjaz I think now all good.
I fixed all places with assert.
I checked templates in all places.
Also, I re-read your comments and my code.

webknjaz

Still ignoring stale comments.

webknjaz · 2018-11-27T21:17:52Z

blog/tests/models_test.py

+        del self.post
+
+    def test_right_title(self):
+        """Test right title output."""


It's not output, it's just stringification protocol

webknjaz · 2018-11-27T21:18:36Z

blog/tests/models_test.py

+        ),
+    )
+    def test_post_publish(self):
+        """Test for publish method."""


still there

blog/tests/models_test.py

webknjaz · 2018-11-27T21:25:39Z

blog/tests/views_test.py

+
+        assert len(response.context['posts']) == 2
+
+        first_title_on_the_page = response.context['posts'][0].title


This is too verbose and manual comparison. You can just compare post lists, not individual attributes.

Here's a good example of how to check what's rendered in different points of time: https://github.com/kpi-web-guild/django-girls-blog-makkaronis4e/pull/27/files#diff-8e9b1eecb0153fa41f8d69ca4922dc61R35

webknjaz · 2018-11-27T21:26:33Z

blog/tests/views_test.py

+        title_detail_db = Post.objects.filter(pk=1)[0].title
+        title_detail_page = response.context['post'].title
+
+        assert title_detail_db == title_detail_page


Compare post objects instead of some of their properties.

ipdb> response.context["post"] <Post: Title post first> ipdb> response.context["post"].author <User: bob> ipdb> response.context["post"].text 'Text post first'

I do not understand how to do it. I read all the internet, but not find an answer.
What do you mean post objects ?

webknjaz · 2018-11-27T21:27:17Z

blog/tests/views_test.py

+            response, 'blog/post_detail.html', 'blog/base.html'
+        )
+
+        title_detail_db = Post.objects.filter(pk=1)[0].title


This is saved in the attribute, no need to query db.

webknjaz · 2018-11-27T21:28:01Z

blog/tests/views_test.py

+            response, 'blog/post_detail.html', 'blog/base.html'
+        )
+
+        title_new_in_db = Post.objects.filter(pk=4)[0].title


Compare models instead.

webknjaz · 2018-11-27T21:28:51Z

blog/tests/views_test.py

+        response = self.client.post(
+            reverse('post_new'),
+            {
+                'author': paul,


paul.username

webknjaz · 2018-11-27T21:28:59Z

blog/tests/views_test.py

+        response = self.client.post(
+            reverse('post_edit', kwargs={'pk': 1}),
+            {
+                'author': paul,


Still there.

webknjaz · 2018-11-27T21:29:14Z

blog/tests/views_test.py

+        self.assertIn(b'Post with post edit', response.content)
+        self.assertIn(b'Text with post edit', response.content)
+
+        response = self.client.get(reverse('post_edit', kwargs={'pk': 1}))


Co-Authored-By: Sviatoslav Sydorenko <[email protected]>

webknjaz · 2019-07-27T18:37:57Z

blog/tests/models_test.py

-            author=User.objects.create(username='bob'),
-            title='Title',
-            text='A few lines of text',
+            author=User.objects.create(username="bob"),


I don't think it's an error. And I actually prefer single quotes.

webknjaz · 2019-07-27T18:41:37Z

blog/tests/views_test.py

+        """Test show post list on the page and test correct order."""
+        with patch(
+            'django.utils.timezone.now',
+            lambda: datetime(day=1, month=1, year=2018, tzinfo=self.time_z),


You need to test 3 cases:

the date is frozen before anything got posted

the date is frozen after everything got posted

the date is frozen in between of those

webknjaz · 2019-07-27T18:43:15Z

blog/tests/views_test.py


    def tearDown(self):
        """Clean-up test data."""
+        del self.time_z
+        del self.client


How is this related to that all quotes replacement thing?

webknjaz · 2019-07-27T18:44:03Z

blog/tests/views_test.py

    def test_post_detail(self):
        """Test post contents displayed correctly."""
-        response = self.client.get(reverse('post_detail', kwargs={'pk': 1}))
-        assert response.status_code == 200
+        resp = self.client.get(reverse('post_detail', kwargs={'pk': 1}))


Actually, the full var name reads better.

webknjaz · 2019-07-27T18:44:44Z

blog/tests/models_test.py

@@ -39,6 +39,7 @@ def test_post_publish(self):
        """Test post has a published date after publish method called."""
        assert self.post.published_date is None

+        assert self.post.published_date is None


This is odd