Opened 2 years ago

Last modified 16 months ago

#25889 assigned Cleanup/optimization

Organize tests in tests/queries

Reported by: Claude Paroz Owned by: reficul31
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: Anssi Kääriäinen 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 (6)

comment:1 Changed 2 years ago by Tim Graham

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 Changed 2 years ago by Tim Graham

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

comment:3 Changed 16 months ago by reficul31

Owner: changed from nobody to reficul31
Status: newassigned

comment:4 Changed 16 months ago by reficul31

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?

comment:5 in reply to:  1 Changed 16 months ago by reficul31

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 Changed 16 months ago by reficul31

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