Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@terencehonles
Copy link
Contributor

@terencehonlesterencehonles commentedMar 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 alwaysTuple[bytes, str] rather thanTuple[Union[bytes,str], str]

Copy link
Collaborator

@auvipyauvipy 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 changesMay 9, 2019
Copy link
Contributor

@rpkilbyrpkilby 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+?"

auvipy reacted with thumbs up emoji
@terencehonlesterencehonlesforce-pushed theautomatically-serialize-test-client-json branch from4607f28 to4ed1e0eCompareMay 11, 2019 01:24
@terencehonles
Copy link
ContributorAuthor

@rpkilby updated with a test

@terencehonlesterencehonlesforce-pushed theautomatically-serialize-test-client-json branch 2 times, most recently fromd368885 to0a99e02CompareOctober 8, 2020 14:47
@stale
Copy link

stalebot commentedMay 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.

@stalestalebot added the stale labelMay 1, 2022
@terencehonlesterencehonlesforce-pushed theautomatically-serialize-test-client-json branch from0a99e02 to9ee6d46CompareMay 6, 2022 17:55
@stalestalebot removed the stale labelMay 6, 2022
@terencehonles
Copy link
ContributorAuthor

Updated tomaster

@terencehonlesterencehonlesforce-pushed theautomatically-serialize-test-client-json branch from9ee6d46 to09fafd2CompareMay 6, 2022 20:32
@stale
Copy link

stalebot commentedJul 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.

@stalestalebot added the stale labelJul 10, 2022
@terencehonlesterencehonlesforce-pushed theautomatically-serialize-test-client-json branch from09fafd2 tod28724eCompareJuly 14, 2022 01:24
@terencehonles
Copy link
ContributorAuthor

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

@stalestalebot removed the stale labelJul 14, 2022
Copy link
Collaborator

@auvipyauvipy 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

terencehonles reacted with thumbs up emoji
@auvipyauvipy self-assigned thisDec 10, 2022
@auvipyauvipy added this to the3.15 milestoneDec 10, 2022
@auvipyauvipy removed this from the3.15 milestoneJul 11, 2023
@stale
Copy link

stalebot commentedSep 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.

@stalestalebot added the stale labelSep 17, 2023
@terencehonlesterencehonlesforce-pushed theautomatically-serialize-test-client-json branch fromd28724e tod579cc1CompareOctober 4, 2023 20:29
@stalestalebot removed the stale labelOct 4, 2023
@terencehonles
Copy link
ContributorAuthor

The failing test should be fixed by#9129

@auvipyauvipy changed the titlesupport django 2.1 test client json data automatically serializedsupport django 2.1+ test client json data automatically serializedOct 5, 2023
Copy link
Collaborator

@auvipyauvipy 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

@auvipyauvipy added this to the3.15 milestoneOct 5, 2023
@terencehonlesterencehonlesforce-pushed theautomatically-serialize-test-client-json branch fromd579cc1 to803e089CompareOctober 5, 2023 08:53
@terencehonles
Copy link
ContributorAuthor

terencehonles commentedOct 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 herehttps://docs.djangoproject.com/en/4.2/topics/testing/tools/#django.test.Client.post(specifically), and it's mentioned in the 2.1 release noteshttps://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
ContributorAuthor

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

Copy link
Collaborator

@auvipyauvipy 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 changesDec 14, 2024
@terencehonles
Copy link
ContributorAuthor

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 therequest.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
Copy link
ContributorAuthor

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

@auvipy
Copy link
Collaborator

Thanks, will review it again soon

@auvipyauvipy dismissedrpkilby’sstale reviewDecember 28, 2024 10:21

review comment addressed

@auvipyauvipy merged commit089f6a6 intoencode:masterDec 28, 2024
8 checks passed
@browniebrokebrowniebroke modified the milestones:3.15,3.16Jan 16, 2025
@terencehonlesterencehonles deleted the automatically-serialize-test-client-json branchFebruary 10, 2025 09:25
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@auvipyauvipyauvipy approved these changes

+1 more reviewer

@rpkilbyrpkilbyrpkilby left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@auvipyauvipy

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
[8]ページ先頭
©2009-2025 Movatter.jp