Opened 10 years ago

Closed 10 years ago

Last modified 4 years ago

#22909 closed Cleanup/optimization (fixed)

Fix camelCase test names

Reported by: Tim Graham Owned by: Brylie Christopher Oxley
Component: Uncategorized Version: dev
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)

issue_22909.patch (76.1 KB ) - added by Brylie Christopher Oxley 10 years ago.
Patch containing changes for test names in several test suites.

Download all attachments as: .zip

Change History (27)

comment:1 by Brylie Christopher Oxley, 10 years ago

DONE -- The admin_custom_urls package has camel case tests.

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:2 by Brylie Christopher Oxley, 10 years ago

Cc: Brylie Christopher Oxley added
Owner: changed from nobody to Brylie Christopher Oxley
Status: newassigned

comment:3 by Brylie Christopher Oxley, 10 years ago

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 years ago by Brylie Christopher Oxley (previous) (diff)

comment:4 by Brylie Christopher Oxley, 10 years ago

DONE -- admin_views has several camel case test names.

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:5 by Brylie Christopher Oxley, 10 years ago

DONE -- admin_widgets needs cleanup.

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:6 by Brylie Christopher Oxley, 10 years ago

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 years ago by Brylie Christopher Oxley (previous) (diff)

comment:7 by Brylie Christopher Oxley, 10 years ago

DONE -- conditional_processing needs cleanup.

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:8 by Brylie Christopher Oxley, 10 years ago

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 years ago by Brylie Christopher Oxley (previous) (diff)

comment:9 by Brylie Christopher Oxley, 10 years ago

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 years ago by Brylie Christopher Oxley (previous) (diff)

comment:10 by Brylie Christopher Oxley, 10 years ago

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

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:11 by Brylie Christopher Oxley, 10 years ago

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

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:12 by Brylie Christopher Oxley, 10 years ago

DONE -- generic_inline_admin has camel case test names.

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:13 by Brylie Christopher Oxley, 10 years ago

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 years ago by Brylie Christopher Oxley (previous) (diff)

comment:14 by Brylie Christopher Oxley, 10 years ago

DONE -- nested_foreign_keys has camel case test names.

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:15 by Brylie Christopher Oxley, 10 years ago

DONE -- raw_query has camel case test names.

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:16 by Brylie Christopher Oxley, 10 years ago

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

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:17 by Brylie Christopher Oxley, 10 years ago

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

Last edited 10 years ago by Brylie Christopher Oxley (previous) (diff)

comment:18 by Tim Graham, 10 years ago

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.

in reply to:  1 comment:19 by Brylie Christopher Oxley, 10 years ago

Replying to brylie:

The admin_custom_urls package has camel case tests.

Fixed.

in reply to:  18 comment:20 by Brylie Christopher Oxley, 10 years ago

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 years ago by Brylie Christopher Oxley (previous) (next) (diff)

by Brylie Christopher Oxley, 10 years ago

Attachment: issue_22909.patch added

Patch containing changes for test names in several test suites.

comment:22 by Brylie Christopher Oxley, 10 years ago

Has patch: set

comment:23 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 89b9e6e5d68297e7fe10baea6abcd96e24de0e09:

Fixed #22909 -- Removed camelCasing in some tests.

Thanks brylie.

comment:24 by Tim Graham <timograham@…>, 10 years ago

In 28962c57f3f2b4543cd67d55ca635e9198f585dd:

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

Thanks brylie.

Backport of 89b9e6e5d6 from master

comment:25 by Tim Graham, 10 years ago

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

comment:26 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 58740c0d:

Refs #22909 -- Removed camelCasing in auth_tests.test_templates tests.

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