Add tests using default DRF classes #490

Alig1493 · 2018-10-09T10:18:40Z

Fixes #283

Description of the Change

Checklist

PR only contains one change (considered splitting up PR)
unit-test added
documentation updated
changelog entry added to CHANGELOG.md
author name in AUTHORS

Alig1493 · 2018-10-09T10:19:29Z

@sliverc Here's the initial progress.

sliverc · 2018-10-11T06:53:20Z

Thanks @Alig1493 This looks like a good start.

The test you are covering here is based on rendering. I think it will also be important to test parsing with DRF serializers resp. the full request/response cycle. This could be something like test_model_viewsets.py but with DRF classes.

Alig1493 · 2018-10-11T06:58:08Z

Alright I'll go take a look.

Alig1493 · 2018-10-11T12:36:01Z

@sliverc I had some issues regarding testing the response of the default drf serializer urls:
The test_model_viewsets.py file tested urls that called the viewsets which had json-api serializers.
Does this mean I'll be making my own custom viewsets and register it with the urls as new endpoints and test those new endpoints?

I took the approach of making new serializers for the Blog model using the default serializer and adding it as a viewset in urls. I didn't make new files for these and added the code to the existing code base. Should I push into the PR for you to check?

Thanks.

sliverc · 2018-10-11T13:25:59Z

@Alig1493 yep you will add custom DRF viewsets for this to work. I am fine when they are in the same file as it just a variant of the other tests.

So 👍 to push

Alig1493 · 2018-10-11T13:50:43Z

@sliverc pushed.

sliverc

Great. It's shaping up nicely. See my comments. Also it would still be great to have a test which either does a post or patch on the DRF view (I guess the same view could be used).

sliverc · 2018-10-12T06:40:18Z

example/serializers.py

@@ -50,6 +58,25 @@ class Meta:
        meta_fields = ('copyright',)


+# DRF default serializer to test default DRF functionalities
+class BlogDRFSerializer(serializers.ModelSerializer):


shouldn't this be derived from drf_serilazers.ModelSerializer as well? Also best move the comment into the class a block comment.

example/serializers.py

sliverc · 2018-10-12T06:45:18Z

example/tests/unit/test_default_drf_serializers.py

@@ -73,3 +76,24 @@ def test_render_format_keys(settings):

    result = json.loads(rendered.decode())
    assert result['data']['attributes']['json-field'] == {'json-key': 'JsonValue'}
+
+
+class TestDRFBlogViewSet(TestBlogViewSet):


as all other tests are written in pytest in this module I would write these tests the same way. This way we do not have a dependency on TestBlogViewSet (which basically only creates an entry during setup phase anyway).

sliverc · 2018-10-12T06:46:39Z

example/tests/unit/test_default_drf_serializers.py

+            'meta': {'apiDocs': '/docs/api/blogs'}
+        }
+        got = resp.json()
+        print(got)


left over print statement? I guess should be removed.

sliverc · 2018-10-12T06:49:15Z

example/tests/unit/test_default_drf_serializers.py

@@ -0,0 +1,99 @@
+import json
+
+from django.urls import reverse


Imports are in invalid order according to isort.

See https://travis-ci.org/django-json-api/django-rest-framework-json-api/jobs/440153239#L936

Alig1493 · 2018-10-14T06:50:30Z

example/tests/unit/test_default_drf_serializers.py

+@pytest.mark.django_db
+def test_get_object_gives_correct_blog(client, blog, entry):
+
+    url = reverse('drf-entry-blog', kwargs={'entry_pk': entry.id})


@sliverc I have used reverse here.

Alig1493 · 2018-10-14T06:51:09Z

example/tests/unit/test_default_drf_serializers.py

+    settings.JSON_API_FORMAT_KEYS = 'dasherize'
+    rendered = render_dummy_test_serialized_view(DummyTestViewSet)
+
+    result = json.loads(rendered.decode())


@sliverc as for json here's the usage

Alig1493 · 2018-10-14T06:54:30Z

@sliverc apologies for taking so long to get back to you I became occupied with something else for some time. One thing that I was wondering is that if it would be possible for you to guide me as to how to make put/patch requests using modelviewsets. I've mostly used generic class based views, although that's no excuse. My initial attempt in trying to patch the drf-entry-blog url endpoint of mine keeps giving me method not allowed error.

Alig1493 · 2018-10-14T08:39:41Z

@sliverc Update: I've figured out how to patch using routers and model viewsets. Hopefully you'll be able to review it soon. My remaining question is that when posting data should I be only creating a Blog object, or will I have to link the blog object to the entry object to which it's related.

sliverc

Create test looks good. Let's leave it as is for now we might add relationships creation later one when there is a need.

Some small comments remain. Would also be great if you could add yourself to AUTHORS.

sliverc · 2018-10-14T17:39:56Z

example/tests/conftest.py

+
+
+@pytest.fixture
+def blog():


This is actually not needed as this is automatically done with pytest-factoryboy where this factory is registered.

Oh that's my bad on not seeing the registered factories.

sliverc · 2018-10-14T17:43:13Z

example/tests/unit/test_default_drf_serializers.py

+    blog = Blog.objects.filter(name=name)
+
+    # check if blog exists in database
+    assert blog.exists()


I guess this asserts is not needed as the next asserts checks already whether it exists by counting it.

Good point.

sliverc · 2018-10-14T17:43:32Z

example/tests/unit/test_default_drf_serializers.py

+    assert blog.count() == 1
+
+    # get created blog from database
+    blog = blog[0]


blog.first() might be more elegant.

OF COURSE. Completely missed that train of thought.

Alig1493 · 2018-10-14T18:18:35Z

@sliverc I have made changes as per your review. Please have a look.

Alig1493 · 2018-10-14T18:22:05Z

@sliverc WIth your permission I've also included my name to authors. Put that in a seperate commit if that's okay.

sliverc · 2018-10-15T06:54:31Z

Looks good.

It needs to be rebased to current master and CI isort issue https://travis-ci.org/django-json-api/django-rest-framework-json-api/jobs/441354640 fixed and the in should be ready.

Alig1493 · 2018-10-15T08:24:25Z

@sliverc I can do the rebase, but I'll need more insight on the isort issue. It's been there since my first push!

Alig1493 · 2018-10-15T08:30:41Z

Isort is a package that sorts imports alphabetically. So I just have to fix my order of imports?

Alig1493 · 2018-10-15T09:13:42Z

@sliverc Travis CI seems to be stuck. Could you please give more insight on this issue? :s

sliverc · 2018-10-15T09:19:58Z

@Alig1493 Usually pushing again to the branch helps. I guess you can rebase to master and resolve the conflicts and push again.

…in request response cycle

Alig1493 · 2018-10-15T10:02:22Z

@sliverc it seems all's well for now.

sliverc · 2018-10-15T10:22:57Z

Great. Thanks for your contribution. Merged.

In case you want to extend your tests to address #298 that would be great. Feel free.

Alig1493 · 2018-10-15T10:55:02Z

I'd love to extend my help if possible. I'll take a look and ask if there's any questions.

Alig1493 force-pushed the test-default-drf-serializers branch 2 times, most recently from 1e71502 to b42b20b Compare October 9, 2018 10:22

sliverc requested changes Oct 12, 2018

View reviewed changes

Alig1493 commented Oct 14, 2018

View reviewed changes

Alig1493 force-pushed the test-default-drf-serializers branch 2 times, most recently from 315ab6c to ebc5e4f Compare October 14, 2018 11:21

sliverc requested changes Oct 14, 2018

View reviewed changes

Alig1493 force-pushed the test-default-drf-serializers branch 2 times, most recently from 136f7c1 to b7b6606 Compare October 15, 2018 09:05

Alig1493 added 5 commits October 15, 2018 15:30

Added rendering tests using default drf serializers

41e8de0

Added new serializers for BLogs to test default serializer behaviour …

a625a2e

…in request response cycle

Updated existing tests to pytest.

b46fb57

Tests for blog, create, update and delete

33807af

Removed extra blog, entry fixtures. Ammended assert checks.

09f34d5

Alig1493 added 2 commits October 15, 2018 15:30

Adding self to AUTHORS

bcbf07d

Fixing imports using isort

5780a68

Alig1493 force-pushed the test-default-drf-serializers branch from b7b6606 to 5780a68 Compare October 15, 2018 09:34

Fixing import issues in remaining example files

1a23890

sliverc approved these changes Oct 15, 2018

View reviewed changes

sliverc changed the title ~~Added rendering tests using default drf serializers~~ Add tests using default DRF classes Oct 15, 2018

sliverc merged commit bda8f63 into django-json-api:master Oct 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests using default DRF classes #490

Add tests using default DRF classes #490

Alig1493 commented Oct 9, 2018 •

edited by sliverc

Loading

Alig1493 commented Oct 9, 2018

sliverc commented Oct 11, 2018

Alig1493 commented Oct 11, 2018

Alig1493 commented Oct 11, 2018

sliverc commented Oct 11, 2018

Alig1493 commented Oct 11, 2018

sliverc left a comment

sliverc Oct 12, 2018

sliverc Oct 12, 2018

sliverc Oct 12, 2018

sliverc Oct 12, 2018

Alig1493 Oct 14, 2018

Alig1493 Oct 14, 2018

Alig1493 commented Oct 14, 2018

Alig1493 commented Oct 14, 2018

sliverc left a comment

sliverc Oct 14, 2018

Alig1493 Oct 14, 2018

sliverc Oct 14, 2018

Alig1493 Oct 14, 2018

sliverc Oct 14, 2018

Alig1493 Oct 14, 2018

Alig1493 commented Oct 14, 2018

Alig1493 commented Oct 14, 2018

sliverc commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

sliverc commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

sliverc commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

Add tests using default DRF classes #490

Add tests using default DRF classes #490

Conversation

Alig1493 commented Oct 9, 2018 • edited by sliverc Loading

Description of the Change

Checklist

Alig1493 commented Oct 9, 2018

sliverc commented Oct 11, 2018

Alig1493 commented Oct 11, 2018

Alig1493 commented Oct 11, 2018

sliverc commented Oct 11, 2018

Alig1493 commented Oct 11, 2018

sliverc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Alig1493 commented Oct 14, 2018

Alig1493 commented Oct 14, 2018

sliverc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Alig1493 commented Oct 14, 2018

Alig1493 commented Oct 14, 2018

sliverc commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

sliverc commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

sliverc commented Oct 15, 2018

Alig1493 commented Oct 15, 2018

Alig1493 commented Oct 9, 2018 •

edited by sliverc

Loading