syrusakbary / snapshottest

Snapshot Testing utils for Python 📸
MIT License
530 stars 103 forks source link

Snapshots doesn't actually look into the response with DRF #154

Open C0DK opened 3 years ago

C0DK commented 3 years ago

versions

Explaination

It might be due to me running django rest framework, but the JSON response doesn't seem to be checked when i use the same testing code as in your example..

This example: (ignore my new_admin stuff, i've just developed some custom code to handle permissions in a DRY way)

    def test_exchanges_return_valid_response(self):
        admin = new_admin("admin")

        response = admin.api_client.get(reverse("external_api:exchanges"))

        self.assertMatchSnapshot(response)

Doesn't change if i change the response.. It works all fine if i change my code to this instead:

    def test_exchanges_return_valid_response(self):
        admin = new_admin("admin")

        response = admin.api_client.get(reverse("external_api:exchanges"))

        self.assertMatchSnapshot(response.json())

This however doesn't check statuscode, which the first example does, so in theory i have to assert both.

The snapshot basically just looks like this:

snapshots['EndpointTests::test_exchanges_return_valid_response 1'] = GenericRepr('<Response status_code=200, "application/json">')

Which then makes it obvious: It's doing a string comparison between the to string version of the class, rather than the actual response body.

I've tried looking at your source code for a few minutes, and figuring out where a fix could be made, but i basically failed hehe. The current code is also good, if you simply change to examples to actually show a comparison of the body of the responses, rather than what you are doing right now, because it might lead to others thinking that they are "safe" when in fact they are not. One could also raise exceptions (or warnings) when snapshot testing on a class where the above will be the case.

jonathankao97 commented 3 years ago

How would snapshottest know to compare response.json() and not the stringified versions? I think this is the intended behavior, your second snippet seems to be working correctly

rennerocha commented 1 year ago

I was thinking how to handle this today (I just started using this library today). One thing that I noticed is that if I call self.assertMatchSnapshot more than once in my test, it will create two entries in the snapshot file, so if you do:

    def test_exchanges_return_valid_response(self):
        admin = new_admin("admin")

        response = admin.api_client.get(reverse("external_api:exchanges"))

        self.assertMatchSnapshot(response.status_code)
        self.assertMatchSnapshot(response.json())

You will have something like:

snapshots['test_exchanges_return_valid_response 1'] = 200

snapshots['test_exchanges_return_valid_response 2'] = {"my_json_response": "content"}

Doing that way, you are able to evaluate your DRF response.