[Django] #32959: URLValidator test cases can be defined in a Python module
Django
------------------------------------------------+------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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/blob/012f38f9594b35743e9ab231757b7b62db638323/tests/validators/tests.py
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32959>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Testing framework | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Chris Jerdonek:
Old description:
> 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/blob/012f38f9594b35743e9ab231757b7b62db638323/tests/validators/tests.py
>
> 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.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:1>
Django
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Testing framework | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
We could also keep them in `.py` modules, e.g. `valid_urls.py`,
`invalid_urls.py`:
{{{
URLS = [
...
]
}}}
What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:2>
Django
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Testing framework | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
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.)
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:3>
Django
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
OK, let's move them directly to the `TEST_DATA` in
`tests/validators/tests.py`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:4>
Django
Reporter: Chris Jerdonek | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Bal Krishna Jha):
* owner: nobody => Bal Krishna Jha
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:5>
Django
Reporter: Chris Jerdonek | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Bal Krishna Jha):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:6>
Django
Reporter: Chris Jerdonek | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:7>
Django
Reporter: Chris Jerdonek | Owner: Bal
Type: | Krishna Jha
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"033636286a7e1507e5471b653b074c1981738110" 03363628]:
{{{
#!CommitTicketReference repository=""
revision="033636286a7e1507e5471b653b074c1981738110"
Fixed #32959 -- Moved tests URLs to validators.tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32959#comment:8>