Question about ticket #20456: Easier unit testing for class-based views

[Felipe Lee]

Hi everyone,

I was looking into ticket #20456 (https://code.djangoproject.com/ticket/20456), which is to improve the ease of testing class-based views (cbvs). It's been a while since anyone has touched it. There was a pull request a while back (https://github.com/django/django/pull/2368/) which added a "setup" method that could be used from a few different places. It would move a few lines from the "view" function created in "as_view":

if hasattr(self, 'get') and not hasattr(self, 'head'):
 self.head = self.get
self.request = request
self.args = args
self.kwargs = kwargs

and it would put these lines in "setup". The idea was that you could then instantiate a cbv in your tests, call setup on its instance, and you'd be set to test it.

There was support in the pr for this, and basically it seemed to mainly need documentation, and to remove some of the refactoring of existing tests, which should be done in another pr (according to comments).

Unfortunately the pr was not followed up on, so it was closed. Last year, another pr was merged that pulled some of this same logic out of "view" in "as_view" into a new method called "setup": https://github.com/django/django/pull/10427

The only part it didn't pull out was the part that sets "self.head = self.get". I'm not sure why that part wasn't included, it wasn't really addressed in the pr or the ticket (https://code.djangoproject.com/ticket/29750). These changes were done for different reasons, but in essence can work towards this goal of easier unit testing. 

 

All of that to say, should I move the "head"/"get" from "view" into "setup" as the original pr had attempted to do? And add in the necessary documentation?

An alternative approach could also be to add a subclass of TestCase, kind of like this https://github.com/revsys/django-test-plus/blob/master/test_plus/test.py#L386

I can see arguments for both, in that finishing up the modification of "view" and "setup" would let you use any testing framework you choose to use, so you aren't limited to using whatever subclass of TestCase is created. 

On the other hand, since this is specifically about making testing easier, not really improving or changing the functionality of cbvs, should the changes really be to how cbvs themselves work? In this case it'd be a minor change, and one that people already thought was a good idea at some point. Not sure what everyone's thoughts are on this. 

Once I get some feedback though, I can proceed with making whatever change is preferred. Or if any other change is preferred that I haven't covered.

Best,

Felipe

[Adam Johnson]

Moving the head=get line sounds sensible to me!

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/382d578a-ad84-4d8b-ad01-800db7fbad98%40googlegroups.com.

--

Adam

[Carlton Gibson]

Hi Felipe, 

Thanks for the question! 

For me, yes, this seems reasonable given that we already have the `setup()` hook. Please do proceed. 

Kind Regards,

Carlton

On Monday, 7 October 2019 09:20:12 UTC+2, Adam Johnson wrote:

Moving the head=get line sounds sensible to me!

[Felipe Lee]

Sounds good, thanks for the input. I'll work on that this week/weekend.

Best,

Felipe