Opened 2 years ago

Closed 2 years ago

Last modified 15 months ago

#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 Anvansh Singh, 2 years ago

Owner: changed from nobody to Anvansh Singh
Status: newassigned

comment:2 by Carlton Gibson, 2 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

OK — I'm not 100% convinced this is worth the change, so if someone wants to mark it wontfix I won't complain, but… —  let's accept as a cleanup for the assertFormsetError and assertQuerysetEqual methods.

  • Uses in Django will need to be updated.
  • The old names will need to be deprecated. (Let's not have two versions floating around...)
  • A matching Issue/PR should be created on The django-upgrade project when this goes in.

comment:3 by Michael Howitz, 2 years ago

Owner: changed from Anvansh Singh to Michael Howitz

Taking over the assignment as I am on a sprint and able to work on it.

comment:4 by Michael Howitz, 2 years ago

Has patch: set

comment:5 by David Sanders, 2 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 David Sanders, 2 years ago

Cc: David Sanders added

comment:7 by Michael Howitz, 2 years ago

comment:8 by Michael Howitz, 2 years ago

comment:9 by Mariusz Felisiak, 2 years ago

Needs documentation: set
Patch needs improvement: set

comment:10 by Michael Howitz, 2 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 564b317f:

Refs #33990 -- Renamed SimpleTestCase.assertFormsetError() to assertFormSetError().

Co-Authored-By: Michael Howitz <mh@…>

comment:12 by Mariusz Felisiak, 2 years ago

Patch needs improvement: set

comment:13 by Mariusz Felisiak, 2 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In f0c06f8a:

Refs #33990 -- Renamed TransactionTestCase.assertQuerysetEqual() to assertQuerySetEqual().

Co-Authored-By: Michael Howitz <mh@…>

comment:15 by Mariusz Felisiak, 2 years ago

Resolution: fixed
Status: assignedclosed

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 15 months ago

In c35fd9e2:

Refs #33990 -- Removed SimpleTestCase.assertFormsetError() per deprecation timeline.

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 15 months ago

In 69af3bea:

Refs #33990 -- Removed TransactionTestCase.assertQuerysetEqual() per deprecation timeline.

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