Opened 3 years ago

Closed 3 years ago

#32959 closed Cleanup/optimization (fixed)

URLValidator test cases can be defined in a Python module

Reported by: Chris Jerdonek Owned by: Bal Krishna Jha
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Chris Jerdonek)

I noticed that the URLValidator tests have a number of test cases defined in separate text files (valid_urls.txt and invalid_urls.txt): https://github.com/django/django/tree/012f38f9594b35743e9ab231757b7b62db638323/tests/validators

However, it seems like it would be more maintainable if these test cases were defined in a Python module (e.g. the same tests.py file containing the test code). One reason is that there aren't actually that many test cases to warrant a separate file. Another is that having them as part of a Python module would permit them to be annotated with code comments. Currently, none of the test cases have any comments elaborating on what they're testing, and having them in a text file precludes that possibility. Finally, if there are any concerns about mistranscribing from the text files to a Python module, the transcription could be done with a Python script to eliminate the possibility of transcription errors.

Change History (8)

comment:1 by Chris Jerdonek, 3 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 3 years ago

We could also keep them in .py modules, e.g. valid_urls.py, invalid_urls.py:

URLS = [
    ...
]

What do you think?

comment:3 by Chris Jerdonek, 3 years ago

That's a good thought and would indeed be better. But since the two files are only 76 lines and 87 lines, I think we could safely keep them next to / nearer the actual tests. In Django's code, it's not uncommon to find single functions that run to that length (and whole modules a great deal longer than that), so I don't see much problem in having test data of that length. (If we needed to split anything, splitting the URLValidator tests off from DecimalValidator, email tests, etc. into its own test_urls.py might be a more natural division.)

comment:4 by Mariusz Felisiak, 3 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted

OK, let's move them directly to the TEST_DATA in tests/validators/tests.py.

comment:5 by Bal Krishna Jha, 3 years ago

Owner: changed from nobody to Bal Krishna Jha
Status: newassigned

comment:6 by Bal Krishna Jha, 3 years ago

Has patch: set
Last edited 3 years ago by Bal Krishna Jha (previous) (diff)

comment:7 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 03363628:

Fixed #32959 -- Moved tests URLs to validators.tests.

Note: See TracTickets for help on using tickets.
Back to Top