#22909 closed Cleanup/optimization (fixed)
Fix camelCase test names
Reported by: | Tim Graham | Owned by: | Brylie Christopher Oxley |
---|---|---|---|
Component: | Uncategorized | Version: | master |
Severity: | Normal | Keywords: | |
Cc: | Brylie Christopher Oxley | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
In some test files like tests/admin_widgets/tests.py
, there are tests with names like testDateField
where it should be test_date_field
. Note that it's fine for assertions to be camel cased, so there's no need to change things like assertFormfield
. I have seen this most often in the admin_*
tests, so those would be the main ones to check.
Attachments (1)
Change History (26)
comment:2 Changed 4 years ago by
Cc: | Brylie Christopher Oxley added |
---|---|
Owner: | changed from nobody to Brylie Christopher Oxley |
Status: | new → assigned |
comment:3 Changed 4 years ago by
SKIPPED -- admin_filters may need consistent spacing in test functions in the ListFiltersTests class. Please review and comment as to whether spacing/underscores should be changed.
comment:4 Changed 4 years ago by
admin_views has several camel case test names.
comment:6 Changed 4 years ago by
DONE -- bug639 has one camel case test name.
Q: Should this test be moved into another, more general, test module (likewise with "bug8245" test module)?
comment:8 Changed 4 years ago by
DONE -- dispatch has camel case test names.
Also, the tests were created in a sub-directory: dispatch/tests/test_dispatcher.py
Q: Should the tests be moved up to the dispatch directory, and possibly renamed tests.py?
comment:9 Changed 4 years ago by
IGNORE -- file_uploads has set up and tear down classes that are named setUpClass and tearDownClass. Should these be renamed to setUp and tearDown?
comment:10 Changed 4 years ago by
IGNORE -- fixtures has two test functions named testClassFixtures. Should these be renamed?
comment:11 Changed 4 years ago by
IGNORE -- fixtures_model_package has one function named testClassFixtures. Should this be renamed?
comment:13 Changed 4 years ago by
DONE -- model_fields contains a function without a camel case for the field name: test_slugfield_max_length
This is inconsistent with other field names in test functions. Should this be changed to test_SlugField_max_length?
There are several similar functions in class ValidationTest. Should field names in these functions be changed to camel case?
comment:18 follow-up: 20 Changed 4 years ago by
Thanks for looking at this. Let's just focus on camel case test names for this ticket. We can open a separate one for some of the other issues you raised.
To address some specific questions:
- admin_filters -- think it's fine to leave as-is.
- bug* -- yes, let's move them, but as a separate item.
- dispatch -- probably move them as a separate item.
- file_uploads -- setUpClass/tearDownClass are fine.
- model_fields -- I'd remove the existing camel case for field names.
comment:19 Changed 4 years ago by
comment:20 Changed 4 years ago by
Replying to timo:
- model_fields -- I'd remove the existing camel case for field names.
Ah, dang. I just went with the opposite and added camel case to field names in tests. I think this is a good strategy for three reasons:
- It is consistent with other tests/test suites
- It is consistent with the Django API
- It improves readability, which seems to be a priority here
Does this seem reasonable?
comment:21 Changed 4 years ago by
OK, I have modified the files where I found camel case test names. If you will, please review the following changes.
- admin_custom_urls/tests.py
- admin_views/tests.py
- admin_widgets/tests.py
- conditional_processing/tests.py
- dispatch/tests/test_dispatcher.py
- generic_inline_admin/tests.py
- model_fields/tests.py
- nested_foreign_keys/tests.py
- raw_query/tests.py
- utils_tests/test_http.py
- view_tests/tests/i18n/tests.py
Changed 4 years ago by
Attachment: | issue_22909.patch added |
---|
Patch containing changes for test names in several test suites.
comment:22 Changed 4 years ago by
Has patch: | set |
---|
comment:23 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:25 Changed 4 years ago by
I moved the dispatch tests in 136a3ffe21988d49e443867d129cc01fb62b34cd and created #22979 for moving the bug tests.
DONE -- The admin_custom_urls package has camel case tests.