Skip to content

Conversation

math-a3k
Copy link
Contributor

@math-a3k math-a3k commented Jan 23, 2024

This PR (based on #9230) fixes the test suite for supporting Django 5.0, enables it on the CI, and updates the documentation.

It also fixes the tests for djangomain (up to django/django@e412d85)

@math-a3k math-a3k marked this pull request as ready for review January 23, 2024 15:52
@math-a3k math-a3k mentioned this pull request Jan 23, 2024
@loicgasser
Copy link

I think you might to update the setup.py file as file
https://github.com/encode/django-rest-framework/blob/master/setup.py

@auvipy auvipy added this to the 3.15 milestone Feb 20, 2024
@auvipy
Copy link
Collaborator

auvipy commented Feb 20, 2024

sorry for taking soo long to respond! had other pro n personal priorities

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.

good job so far. have some comments and suggestions to move forward

url_field = URLField(max_length=100)
custom_field = ModelField(model_field=<tests.test_model_serializer.CustomField: custom_field>)
file_path_field = FilePathField(path=%r)
expected = dedent(r"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

usage of regex instead of old string format seems interesting to me. can you share more insight about it? pros and cons regarding this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The need arises from the integer validator generated from Django 5 integer fields. That validator breaks the assertion against the repr. In other tests, there is a conditional on Django version, an "if" to add the extra string to match. I find regex to be more flexible against changes that are not the object of the test - i.e. the correct functionality asserted from the repr in which the integer validator does not play a role. This would also prevent cluttering if, in future versions of Django, similar changes arrive (preventing the discussion for each version).

The cons I find would be the escaping cluttering in the strings because otherwise, it is pretty straight with the current testing suite ("the dedent of strings for testing the repr" way) and not harder to read.

@auvipy auvipy merged commit d4016d8 into encode:master Feb 28, 2024
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

Successfully merging this pull request may close these issues.

4 participants