Opened 10 months ago

Closed 10 months ago

Last modified 9 months ago

#22909 closed Cleanup/optimization (fixed)

Fix camelCase test names

Reported by: timo Owned by: brylie
Component: Uncategorized Version: master
Severity: Normal Keywords:
Cc: brylie 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)

issue_22909.patch (76.1 KB) - added by brylie 10 months ago.
Patch containing changes for test names in several test suites.

Download all attachments as: .zip

Change History (26)

comment:1 follow-up: Changed 10 months ago by brylie

DONE -- The admin_custom_urls package has camel case tests.

Last edited 10 months ago by brylie (previous) (diff)

comment:2 Changed 10 months ago by brylie

  • Cc brylie added
  • Owner changed from nobody to brylie
  • Status changed from new to assigned

comment:3 Changed 10 months ago by brylie

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.

Last edited 10 months ago by brylie (previous) (diff)

comment:4 Changed 10 months ago by brylie

DONE -- admin_views has several camel case test names.

Last edited 10 months ago by brylie (previous) (diff)

comment:5 Changed 10 months ago by brylie

DONE -- admin_widgets needs cleanup.

Last edited 10 months ago by brylie (previous) (diff)

comment:6 Changed 10 months ago by brylie

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)?

Last edited 10 months ago by brylie (previous) (diff)

comment:7 Changed 10 months ago by brylie

DONE -- conditional_processing needs cleanup.

Last edited 10 months ago by brylie (previous) (diff)

comment:8 Changed 10 months ago by brylie

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?

Last edited 10 months ago by brylie (previous) (diff)

comment:9 Changed 10 months ago by brylie

IGNORE -- file_uploads has set up and tear down classes that are named setUpClass and tearDownClass. Should these be renamed to setUp and tearDown?

Last edited 10 months ago by brylie (previous) (diff)

comment:10 Changed 10 months ago by brylie

IGNORE -- fixtures has two test functions named testClassFixtures. Should these be renamed?

Last edited 10 months ago by brylie (previous) (diff)

comment:11 Changed 10 months ago by brylie

IGNORE -- fixtures_model_package has one function named testClassFixtures. Should this be renamed?

Last edited 10 months ago by brylie (previous) (diff)

comment:12 Changed 10 months ago by brylie

DONE -- generic_inline_admin has camel case test names.

Last edited 10 months ago by brylie (previous) (diff)

comment:13 Changed 10 months ago by brylie

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?

Last edited 10 months ago by brylie (previous) (diff)

comment:14 Changed 10 months ago by brylie

DONE -- nested_foreign_keys has camel case test names.

Last edited 10 months ago by brylie (previous) (diff)

comment:15 Changed 10 months ago by brylie

DONE -- raw_query has camel case test names.

Last edited 10 months ago by brylie (previous) (diff)

comment:16 Changed 10 months ago by brylie

DONE -- utils_tests/test_http has camel case test names.

Last edited 10 months ago by brylie (previous) (diff)

comment:17 Changed 10 months ago by brylie

DONE -- view_tests/tests/test_i18n has camel case test names.

Last edited 10 months ago by brylie (previous) (diff)

comment:18 follow-up: Changed 10 months ago by timo

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 in reply to: ↑ 1 Changed 10 months ago by brylie

Replying to brylie:

The admin_custom_urls package has camel case tests.

Fixed.

comment:20 in reply to: ↑ 18 Changed 10 months ago by brylie

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 two reasons:

  1. It is consistent with other tests/test suites
  2. It is consistent with the Django API
  3. It improves readability, which seems to be a priority here

Does this seem reasonable?

Version 2, edited 10 months ago by brylie (previous) (next) (diff)

Changed 10 months ago by brylie

Patch containing changes for test names in several test suites.

comment:22 Changed 10 months ago by brylie

  • Has patch set

comment:23 Changed 10 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 89b9e6e5d68297e7fe10baea6abcd96e24de0e09:

Fixed #22909 -- Removed camelCasing in some tests.

Thanks brylie.

comment:24 Changed 10 months ago by Tim Graham <timograham@…>

In 28962c57f3f2b4543cd67d55ca635e9198f585dd:

[1.7.x] Fixed #22909 -- Removed camelCasing in some tests.

Thanks brylie.

Backport of 89b9e6e5d6 from master

comment:25 Changed 9 months ago by timo

I moved the dispatch tests in 136a3ffe21988d49e443867d129cc01fb62b34cd and created #22979 for moving the bug tests.

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