#33990 closed Cleanup/optimization (fixed)
Inconsistent capitalization of Set in assertion functions
| Reported by: | John Litborn | Owned by: | Michael Howitz | 
|---|---|---|---|
| Component: | Testing framework | Version: | 4.1 | 
| Severity: | Normal | Keywords: | naming, capitalization, camelcase | 
| Cc: | David Sanders | 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
FormSet and QuerySet for some reason doesn't have a capitalized S in assertFormsetError  and assertQuerysetEqual, whereas all other instances of camel-cased names in the codebase containing either of them have capitalized S's: (InlineAdminFormSet, Base[...]FormSet, EmptyQuerySet, RawQuerySet).
Looking at the other assertion methods inherited from unittest.TestCase they're incredibly cased (e.g. assertMultiLineEqual) so I'm not finding any convention of not uppercasing liberally.
My suggestion is to rename assertFormsetError->assertFormSetError and assertQuerysetEqual->assertQuerySetEqual in django.test.TestCase and add aliases for the old names so as not to break existing code. In the code base this is a quite small change, but there's likely some docs (and e.g. the tutorial) that might warrant updating to new spelling. Could also (down the line) add a deprecationwarning to get rid of the old spelling if you want a cleaner API interface.
This bit me as I was following the tutorial and re-typing the code instead of copy-pasting, so it's plausible that others might encounter it similarly if they don't have an IDE that suggests the alternate spelling.
Change History (17)
comment:1 by , 3 years ago
| Owner: | changed from to | 
|---|---|
| Status: | new → assigned | 
comment:2 by , 3 years ago
| Triage Stage: | Unreviewed → Accepted | 
|---|---|
| Type: | Uncategorized → Cleanup/optimization | 
comment:3 by , 3 years ago
| Owner: | changed from to | 
|---|
Taking over the assignment as I am on a sprint and able to work on it.
comment:4 by , 3 years ago
| Has patch: | set | 
|---|
comment:5 by , 3 years ago
Just checking the docs to see how things are named there compared to the code: Looks like QuerySet is capitalised as per code but "Formset" tends to use classic sentence casing:
ie see how QuerySet is used in a sentence: https://docs.djangoproject.com/en/4.1/ref/models/querysets/
compared to how "formset" is used in a sentence: https://docs.djangoproject.com/en/4.1/topics/forms/formsets/
comment:6 by , 3 years ago
| Cc: | added | 
|---|
comment:7 by , 3 years ago
The issue in django-upgrade is https://github.com/adamchainz/django-upgrade/issues/252.
comment:8 by , 3 years ago
The PR in django-upgrade is https://github.com/adamchainz/django-upgrade/pull/253.
comment:10 by , 3 years ago
| Needs documentation: | unset | 
|---|---|
| Patch needs improvement: | unset | 
comment:12 by , 3 years ago
| Patch needs improvement: | set | 
|---|
comment:13 by , 3 years ago
| Patch needs improvement: | unset | 
|---|---|
| Triage Stage: | Accepted → Ready for checkin | 
comment:15 by , 3 years ago
| Resolution: | → fixed | 
|---|---|
| Status: | assigned → closed | 
OK — I'm not 100% convinced this is worth the change, so if someone wants to mark it
wontfixI won't complain, but… — let's accept as a cleanup for theassertFormsetErrorandassertQuerysetEqualmethods.