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

Use api_client, move fixtures around to share basic defaults #3560

Merged

Conversation

sarayourfriend
Copy link
Collaborator

Some potential changes for Dhruv's PR.

@github-actions github-actions bot added the 🧱 stack: api Related to the Django API label Dec 20, 2023
@openverse-bot openverse-bot added the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label Dec 20, 2023
Copy link
Member

@dhruvkb dhruvkb left a comment

Choose a reason for hiding this comment

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

All changes LGTM and while they may not fix the async errors, they're an improvements to the test suite overall 👍!

Comment on lines -70 to +68
"application/x-www-form-urlencoded",
"multipart",
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's part of DRF's api_client's testing request renderer formats. It takes a slug corresponding to the renderer rather than the content type string: https://www.django-rest-framework.org/api-guide/testing/#setting-the-available-formats

Here's an SO answer explaining it as well: https://stackoverflow.com/questions/68528408/difference-between-content-type-application-json-and-format-json-in-drf-apiclien

DRF's testing APIClient actually gave a pretty easy to understand error message about this, I was not aware of this until fixing that error when I changed this test to api_client.

@sarayourfriend sarayourfriend marked this pull request as ready for review December 20, 2023 19:08
@sarayourfriend sarayourfriend requested a review from a team as a code owner December 20, 2023 19:08
@sarayourfriend sarayourfriend requested review from obulat and stacimc and removed request for a team December 20, 2023 19:08
@sarayourfriend sarayourfriend merged commit b5eb751 into int_test_using_client Dec 20, 2023
50 of 57 checks passed
@sarayourfriend sarayourfriend deleted the int_test_using_client---potential_fixes branch December 20, 2023 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧱 stack: api Related to the Django API 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants