Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BulkSerializerMixin makes Serializers difficult to test #39

Open
hanssto opened this issue Jul 31, 2015 · 4 comments
Open

BulkSerializerMixin makes Serializers difficult to test #39

hanssto opened this issue Jul 31, 2015 · 4 comments

Comments

@hanssto
Copy link

hanssto commented Jul 31, 2015

BulkSerializerMixin.to_internal_values(...) does this:

        request_method = getattr(getattr(self.context.get('view'), 'request'), 'method', '')

So, given the serializer code:

from rest_framework import serializers
from rest_framework_bulk import BulkListSerializer, BulkSerializerMixin
class SimpleSerializer(BulkSerializerMixin, serializers.Serializer):
    simple_field = serializers.CharField()
    class Meta:
        list_serializer_class = BulkListSerializer

And a test case:

from django.test import TestCase
from myproject.serializers import SimpleSerializer
class SimpleSerializerTestCase(TestCase):

    def test_single(self):
        data = {
            "simple_field": "foo",
        }
        serializer = SimpleSerializer(data=data)
        self.assertTrue(serializer.is_valid())

    def test_multiple(self):
        data = [
            {"simple_field": "foo"},
            {"simple_field": "bar"},
        ]
        serializer = SimpleSerializer(data=data, many=True)
        self.assertTrue(serializer.is_valid())

both of these tests will fail with

Traceback (most recent call last):
...
File ".../local/lib/python2.7/site-packages/rest_framework_bulk/drf3/serializers.py", line 19, in to_internal_value
request_method = getattr(getattr(self.context.get('view'), 'request'), 'method', '')
AttributeError: 'NoneType' object has no attribute 'request'

One can initialize the Serializer with a mock, to get rid of the problem:

        import mock
        view = mock.Mock()
        view.request.method = "POST"
        serializer = SimpleSerializer(data=data, context={"view": view})
        self.assertTrue(serializer.is_valid())

but ideally, a Serializer should be testable in isolation, with no notion of a view involved.

I suggest one or more of:

  • Make to_internal_value() look at instance(s) somehow to determine which of the cases is appropriate. (I honestly have no clue if this is possible)
  • Ignore the lookup field if there is no view. Also not ideal for testing.
  • Add a note to the docs.
  • Add test cases to the project which tests against Serializers in isolation to discover other issues.

My apologies if I missed something obvious.

@miki725
Copy link
Owner

miki725 commented Jul 31, 2015

but ideally, a Serializer should be testable in isolation, with no notion of a view involved.

I agree with that.

Make to_internal_value() look at instance(s) somehow to determine which of the cases is appropriate. (I honestly have no clue if this is possible)

Ill have to think about this. Maybe we can simply check for existence of instance on the serializer. I can see some issues with that as well. Checking for self.instance might not work since to_internal_value does not have direct access to the self.instance in the case of serializer being used as nested serializer in some other serializer. You can probably do self.root.instance but that might not work either since its plausible that in the update of a root serializer, a child serializer resource might need to be created.....

Ignore the lookup field if there is no view. Also not ideal for testing.

You are right and that is not ideal since in this case update will not work.

Add a note to the docs.

I can certainly do that.

Add test cases to the project which tests against Serializers in isolation to discover other issues.

good idea. I am swamped at work so if you are willing, you can submit a PR!

@kevin-brown
Copy link
Contributor

We just ran into this when creating tests for the bulk serializers. Is there any reason why it is looking for view in the context instead of just request? The request is passed in as the serializer context, and it's considerably easier to mock in tests.

@sww314
Copy link

sww314 commented Nov 25, 2016

This is not just a problem for testing. I am using the serializer to push messages across AMQP. In this case, there is no view. I get the same exception.

@morufajibike
Copy link

Has anyone found a fix for this issue? My DRF post APIView was working fine until i changed it to ListBulkCreateUpdateAPIView.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants