Skip to content

support django 2.1+ test client json data automatically serialized#6511

Merged
auvipy merged 1 commit intoencode:masterfrom
terencehonles:automatically-serialize-test-client-json
Dec 28, 2024
Merged

support django 2.1+ test client json data automatically serialized#6511
auvipy merged 1 commit intoencode:masterfrom
terencehonles:automatically-serialize-test-client-json

Conversation

@terencehonles
Copy link
Contributor

@terencehonles terencehonles commented Mar 15, 2019 •
edited by auvipy
Loading

Description

  • Support Django 2.1+ test client auto serializing data if the content type is a json type
  • Ensure the result of _encode_data is always Tuple[bytes, str] rather than Tuple[Union[bytes,str], str]

auvipy
auvipy requested changes Apr 2, 2019
View reviewed changes
Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to have a better review need to add a test for the proposed change.

rpkilby
rpkilby previously requested changes May 9, 2019
View reviewed changes
Copy link
Contributor

@rpkilby rpkilby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely going to agree with @auvipy here. A test case demonstrating the behavior change would be required for this to be accepted.

My immediate reaction is "why aren't our tests failing under Django 2.1+?"

@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch from 4607f28 to 4ed1e0e Compare May 11, 2019 01:24
@terencehonles
Copy link
Contributor Author

terencehonles commented May 11, 2019

@rpkilby updated with a test

@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch 2 times, most recently from d368885 to 0a99e02 Compare October 8, 2020 14:47
@stale
Copy link

stale bot commented May 1, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 1, 2022
@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch from 0a99e02 to 9ee6d46 Compare May 6, 2022 17:55
@stale stale bot removed the stale label May 6, 2022
@terencehonles
Copy link
Contributor Author

terencehonles commented May 6, 2022

Updated to master

@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch from 9ee6d46 to 09fafd2 Compare May 6, 2022 20:32
@stale
Copy link

stale bot commented Jul 10, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 10, 2022
@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch from 09fafd2 to d28724e Compare July 14, 2022 01:24
@terencehonles
Copy link
Contributor Author

terencehonles commented Jul 14, 2022

bumped, I think the failed test was unrelated but this re-run the test suite

@stale stale bot removed the stale label Jul 14, 2022
auvipy
auvipy requested changes Oct 12, 2022
View reviewed changes
Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, please rebase, and I think we might get rid of some checks to eleminate older version check

@auvipy auvipy self-assigned this Dec 10, 2022
@auvipy auvipy added this to the 3.15 milestone Dec 10, 2022
@auvipy auvipy removed this from the 3.15 milestone Jul 11, 2023
@stale
Copy link

stale bot commented Sep 17, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 17, 2023
@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch from d28724e to d579cc1 Compare October 4, 2023 20:29
@stale stale bot removed the stale label Oct 4, 2023
@terencehonles
Copy link
Contributor Author

terencehonles commented Oct 4, 2023

The failing test should be fixed by #9129

@auvipy auvipy changed the title support django 2.1 test client json data automatically serialized support django 2.1+ test client json data automatically serialized Oct 5, 2023
auvipy
auvipy requested changes Oct 5, 2023
View reviewed changes
Copy link
Collaborator

@auvipy auvipy left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rebase? and check if any document or test update is needed to get this merged? also a reference to django docs related to this would be really helpful

@auvipy auvipy added this to the 3.15 milestone Oct 5, 2023
@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch from d579cc1 to 803e089 Compare October 5, 2023 08:53
@terencehonles
Copy link
Contributor Author

terencehonles commented Oct 5, 2023 •
edited
Loading

can you rebase? and check if any document or test update is needed to get this merged? also a reference to django docs related to this would be really helpful

It's mentioned here https://docs.djangoproject.com/en/4.2/topics/testing/tools/#django.test.Client.post (specifically), and it's mentioned in the 2.1 release notes https://docs.djangoproject.com/en/4.2/releases/2.1/#tests (point 2)

I'm not sure if you're asking for this to be included in the source, since there's not specifically a good place to put it (maybe the tests?), or if you're just asking for the release notes.

@terencehonles
Copy link
Contributor Author

terencehonles commented Dec 12, 2024

@auvipy friendly ping, I can rebase, but wondering about ^^^

auvipy
auvipy reviewed Dec 14, 2024
View reviewed changes
Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me. should we also mention this in DRF docs?

auvipy
auvipy previously approved these changes Dec 14, 2024
View reviewed changes
@terencehonles
Copy link
Contributor Author

terencehonles commented Dec 14, 2024

Sure, I'll rebase and mention it.

One thing that I realized with this request factory, when testing something yesterday, is that it returns a Django not a DRF request. This means that trying to access the request.data doesn't work. I had to wrap it in a DRF request manually so I switched back to the native factory to get the encoding and not to confuse my peers since this PR still hasn't landed.

I will look at the code, but would it be acceptable to return a DRF request instead? It proxies the native request so it's not a breaking change. I will look at the code, but from memory I think I can actually initialize it in way that would make sense. I will also look at the test client to make sure it does not rely on the factory or that if it does it works correctly.

@terencehonles terencehonles force-pushed the automatically-serialize-test-client-json branch from 803e089 to 4579eae Compare December 15, 2024 20:43
@terencehonles
Copy link
Contributor Author

terencehonles commented Dec 15, 2024

I've left out changing the return type to Response, so I've just rebased and updated the docs.

@auvipy
Copy link
Collaborator

auvipy commented Dec 19, 2024

Thanks, will review it again soon

@auvipy auvipy dismissed rpkilby’s stale review December 28, 2024 10:21

review comment addressed

@auvipy auvipy merged commit 089f6a6 into encode:master Dec 28, 2024
@sliverc sliverc mentioned this pull request Jan 7, 2025
Merged
@browniebroke browniebroke modified the milestones: 3.15, 3.16 Jan 16, 2025
@terencehonles terencehonles deleted the automatically-serialize-test-client-json branch February 10, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@auvipy auvipy auvipy approved these changes

+1 more reviewer

@rpkilby rpkilby rpkilby left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@auvipy auvipy

Labels

None yet

Projects

None yet

Milestone

3.16

Development

Successfully merging this pull request may close these issues.

4 participants

@terencehonles @auvipy @rpkilby @browniebroke