Opened 7 years ago

Closed 7 years ago

#27426 closed Cleanup/optimization (invalid)

Test Client shouldn't subclass RequestFactory

Reported by: Ana Balica Owned by: Ana Balica
Component: Testing framework Version: 1.10
Severity: Normal Keywords: test, client, requestfactory
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently the test Client is inheriting from RequestFactory for code reuse. Client is not a specialisation of the RequestFactory and the relationship between the two is conceptually incorrect.

Much easier would be to use composition. With that it also becomes possible to use any other type of request factory with the Client. Open for discussion if there is actually any need for a different request factory besides the default one.

Change History (5)

comment:1 by Ana Balica, 7 years ago

Owner: changed from nobody to Ana Balica
Status: newassigned

comment:2 by Tim Graham, 7 years ago

I'm not sure I understand the motivation or exactly how things would change. For reference, eec45e8b710b97201db106a6460fe051f8917833 is the commit that created RequestFactory.

comment:3 by Ana Balica, 7 years ago

This is a design improvement. Now besides using composition by initialising the RequestFactory in Client's __init__, it's also possible to pass your own custom request factory to be used by the client. A possible use case for having a custom factory of requests is to test against HTTP/2.0. Even though this is not a strong argument I still consider it a good improvement, that makes a lot sense.

Backwards compatibility considerations: I've checked the codebase of pytest-django, django-nose and django-test-plus and it doesn't seem like anyone is hacking the Client, so the following change shouldn't break others' code. However there must be someone who isn't going to be happy :(

I'm thinking to document this behaviour as it might be useful for others.

comment:4 by Tim Graham, 7 years ago

Has patch: set
Patch needs improvement: set
Summary: Client is not a RequestFactoryTest Client shouldn't subclass RequestFactory
Triage Stage: UnreviewedAccepted

WIP PR

comment:5 by Ana Balica, 7 years ago

Resolution: invalid
Status: assignedclosed

By introducing some design changes got a more confusing API for Client and RequestFactory.
Also proper way of fixing that would break other people's code too much. Not worth it.

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