Opened 8 months ago

Closed 8 months ago

Last modified 8 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 8 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 8 months ago by brylie

DONE -- The admin_custom_urls package has camel case tests.

Version 1, edited 8 months ago by brylie (previous) (next) (diff)

comment:2 Changed 8 months ago by brylie

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

comment:3 Changed 8 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 8 months ago by brylie (previous) (diff)

comment:4 Changed 8 months ago by brylie

DONE -- admin_views has several camel case test names.

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

comment:5 Changed 8 months ago by brylie

DONE -- admin_widgets needs cleanup.

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

comment:6 Changed 8 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 8 months ago by brylie (previous) (diff)

comment:7 Changed 8 months ago by brylie

DONE -- conditional_processing needs cleanup.

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

comment:8 Changed 8 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 8 months ago by brylie (previous) (diff)

comment:9 Changed 8 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 8 months ago by brylie (previous) (diff)

comment:10 Changed 8 months ago by brylie

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

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

comment:11 Changed 8 months ago by brylie

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

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

comment:12 Changed 8 months ago by brylie

DONE -- generic_inline_admin has camel case test names.

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

comment:13 Changed 8 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 8 months ago by brylie (previous) (diff)

comment:14 Changed 8 months ago by brylie

DONE -- nested_foreign_keys has camel case test names.

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

comment:15 Changed 8 months ago by brylie

DONE -- raw_query has camel case test names.

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

comment:16 Changed 8 months ago by brylie

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

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

comment:17 Changed 8 months ago by brylie

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

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

comment:18 follow-up: Changed 8 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 8 months ago by brylie

Replying to brylie:

The admin_custom_urls package has camel case tests.

Fixed.

comment:20 in reply to: ↑ 18 Changed 8 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 three 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?

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

Changed 8 months ago by brylie

Patch containing changes for test names in several test suites.

comment:22 Changed 8 months ago by brylie

  • Has patch set

comment:23 Changed 8 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 8 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 8 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