Opened 9 years ago

Last modified 3 months ago

#25889 assigned Cleanup/optimization

Organize tests in tests/queries

Reported by: Claude Paroz Owned by: Wassef Ben Ahmed
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: Anssi Kääriäinen, Ülgen Sarıkavak Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

There are a bunch of test_ticket_<number> tests in tests/queries/tests.py or classes named Ticket<number>Tests, sometimes without any docstring at all. I think that this is very poor naming, and make it difficult to browse the file researching for specific tests or test domains.
I'm sure other test apps also suffer from this, but I was struck by this specific file and this could be a starting point for a larger cleanup.

Change History (14)

comment:1 by Tim Graham, 9 years ago

Cc: Anssi Kääriäinen added
Summary: Rename tests in queries testsOrganize tests in queries tests
Triage Stage: UnreviewedAccepted

I think the issue is that for queries tests, they are not easily categorized or described. Many of the tickets are "here is query that regressed." If we can break up the single tests.py file into multiple tests files (perhaps based on the models they use) and models.py into a package similar to tests/auth_tests/models, I think that would be a useful task. If we can do some renaming along the way, that seems okay but it would be easiest to review in a separate commit from one that moves things around.

comment:2 by Tim Graham, 9 years ago

Summary: Organize tests in queries testsOrganize tests in tests/queries

comment:3 by reficul31, 8 years ago

Owner: changed from nobody to reficul31
Status: newassigned

comment:4 by reficul31, 8 years ago

After cleaning most of the method names and adding some context in comments. There are some methods that have multiple tests. Eg. test_ticketno_1 then we have test_ticketno_2. What should be done with such tests. We could group them into a single class maybe and provide some additional information in the comments? Or we could have like test_context_of_test_1?

in reply to:  1 comment:5 by reficul31, 8 years ago

Replying to Tim Graham:

If we can break up the single tests.py file into multiple tests files (perhaps based on the models they use) and models.py into a package similar to tests/auth_tests/models, I think that would be a useful task. If we can do some renaming along the way, that seems okay but it would be easiest to review in a separate commit from one that moves things around.

I think this sort of thing won't be easy. As most of the models aren't reused. If the patch required something there was a high chance that the contributor made the model similar to that given in the ticket. According to me the maximum we can do is to rename the tests such that they provide the context of what the test is for without loosing the information provided by the original author of the test.

comment:6 by reficul31, 8 years ago

Has patch: set
Patch needs improvement: set

comment:7 by Mariusz Felisiak, 4 years ago

Owner: reficul31 removed
Status: assignednew

comment:8 by Ülgen Sarıkavak, 8 months ago

Cc: Ülgen Sarıkavak added

comment:9 by Wassef Ben Ahmed, 4 months ago

Needs documentation: set

comment:10 by Wassef Ben Ahmed, 4 months ago

Needs documentation: unset
Owner: set to Wassef Ben Ahmed
Status: newassigned

comment:11 by Wassef Ben Ahmed, 4 months ago

Patch needs improvement: unset

comment:12 by Sarah Boyce, 4 months ago

Patch needs improvement: set

comment:13 by Wassef Ben Ahmed, 4 months ago

Patch needs improvement: unset

comment:14 by Sarah Boyce, 3 months ago

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top