Re: Minor feature: Make TestCase pass the testcase instance to self.client's constructor

[Russell Keith-Magee]

On Thu, Jan 3, 2013 at 1:56 AM, Malcolm Box <mal...@tellybug.com> wrote:

Hi,

When creating self.client in TestCase, it would be very useful if the testcase instance was passed to the client. 

I'm using a replacement client class that does various validation checks, so wants to use assert* functions on TestCase, thus takes a testcase instance as a parameter at creation time. However this means it can't be used as the client_class attribute on TestCase, as this is instantiated with no parameters (https://github.com/django/django/blob/master/django/test/testcases.py#L475)

There are work-arounds, but it feels to me like a reasonably generic requirement that a test client may want to refer to the test case it's being used with. I think the change can be made largely backwardly compatible by changing to:

self.client = self.client_class(testcase=self)

which would only break an existing replacement client class that had an __init__ method without **kwargs.

I'm happy to code this up, but wanted to check for any objections first.

Personally, I'm suspicious of any "A owns B, but B knows about A" relationships. There are certainly occasions when they're required, but whenever they pop up, it's generally an indication of a set of interfaces that are closely coupled.

In this case, I'm not sure I see why this coupling is required. A test case is a test case. A test client is a test client. Their concerns are completely separate (evidenced by the fact that you can have a test case without any client usage, and vice versa). I very much see the test client as a utility tool for the test case -- really not much more than a convenient factory for requests -- not an integral part of a test case. 

Your feature request seems to be stem entirely from the fact that you want to invoke assertions in the test client code itself -- something that you're doing because you've got a bunch of extensions in your test client. I'll leave it up to you to determine if this is the right approach for your own project, but I'm not convinced it's a general requirement that warrants binding the test client to the test case. 

Yours,

Russ Magee %-)

[Malcolm Box]

On Wednesday, January 2, 2013 11:58:04 PM UTC, Russell Keith-Magee wrote:

On Thu, Jan 3, 2013 at 1:56 AM, Malcolm Box <mal...@tellybug.com> wrote:

Hi,

When creating self.client in TestCase, it would be very useful if the testcase instance was passed to the client. 

I'm using a replacement client class that does various validation checks, so wants to use assert* functions on TestCase, thus takes a testcase instance as a parameter at creation time. However this means it can't be used as the client_class attribute on TestCase, as this is instantiated with no parameters (https://github.com/django/django/blob/master/django/test/testcases.py#L475)

There are work-arounds, but it feels to me like a reasonably generic requirement that a test client may want to refer to the test case it's being used with. I think the change can be made largely backwardly compatible by changing to:

self.client = self.client_class(testcase=self)

which would only break an existing replacement client class that had an __init__ method without **kwargs.

I'm happy to code this up, but wanted to check for any objections first.

Personally, I'm suspicious of any "A owns B, but B knows about A" relationships. There are certainly occasions when they're required, but whenever they pop up, it's generally an indication of a set of interfaces that are closely coupled.

 

In this case, I'm not sure I see why this coupling is required. A test case is a test case. A test client is a test client. Their concerns are completely separate (evidenced by the fact that you can have a test case without any client usage, and vice versa). I very much see the test client as a utility tool for the test case -- really not much more than a convenient factory for requests -- not an integral part of a test case. 

I don't disagree - and this feature wouldn't require any test client to care about the testcase. 

Your feature request seems to be stem entirely from the fact that you want to invoke assertions in the test client code itself -- something that you're doing because you've got a bunch of extensions in your test client. I'll leave it up to you to determine if this is the right approach for your own project, but I'm not convinced it's a general requirement that warrants binding the test client to the test case. 

The general pattern I want to implement is have a test client that makes assertions about all the requests made during a set of tests. For example, it could check that every get() returned cache headers, or that content_type is always specified in responses. Or that the response has certain HTML, uses certain templates etc - ie all of the assertion testing goodness in django.test.TestCase.

My concrete use case is a JSONClient that adds a json_get / json_post methods which makes sure to setup content_type etc on the call, and then validates that what came back was valid JSON.

The simple, wrong way is to do:

def check_response(self, response):

   self.assertContains(response, ....)

   ....

def test():

   r = self.client.get(...)

   self.check_response(r)

but this is error prone, verbose etc etc. 

The right thing is that within a test suite, the get()/post() etc to do the checks for me - and so it should be possible to create a testclient that does this, and be able to use this testclient in various test suites.

Is there a simple, clean way to solve this that I'm missing?

Malcolm

[Shai Berger]

On Friday 04 January 2013, Malcolm Box wrote:
>
> The general pattern I want to implement is have a test client that makes
> assertions about all the requests made during a set of tests. For example,
> it could check that every get() returned cache headers, or that
> content_type is always specified in responses. Or that the response has
> certain HTML, uses certain templates etc - ie all of the assertion testing
> goodness in django.test.TestCase.
>
> My concrete use case is a JSONClient that adds a json_get / json_post
> methods which makes sure to setup content_type etc on the call, and then
> validates that what came back was valid JSON.
>
> The simple, wrong way is to do:
>
> def check_response(self, response):
> self.assertContains(response, ....)
> ....
>
> def test():
> r = self.client.get(...)
> self.check_response(r)
>
> but this is error prone, verbose etc etc.
>
> The right thing is that within a test suite, the get()/post() etc to do the
> checks for me - and so it should be possible to create a testclient that
> does this, and be able to use this testclient in various test suites.
>

No, you're mixing concerns.

> Is there a simple, clean way to solve this that I'm missing?
>

class MyTestCase(TestCase):

def check_response(self, response):
self.assertContains(response, ....)

def checked_get(self, *args, **kw):
r = self.client.get(*args, **kw)
self.check_response(r)
return r

class SpecificTest(MyTestCase):

def test():
r = self.checked_get(...)
...

That's how I would do it.

Shai.

[Malcolm Box]

On Sat, Jan 5, 2013 at 9:11 AM, Shai Berger <sh...@platonix.com> wrote:

On Friday 04 January 2013, Malcolm Box wrote:
>
> The general pattern I want to implement is have a test client that makes
> assertions about all the requests made during a set of tests. For example,
> it could check that every get() returned cache headers, or that
> content_type is always specified in responses. Or that the response has
> certain HTML, uses certain templates etc - ie all of the assertion testing
> goodness in django.test.TestCase.

<snip> 

>
> def test():
>    r = self.client.get(...)
>    self.check_response(r)
>
> but this is error prone, verbose etc etc.
>
> The right thing is that within a test suite, the get()/post() etc to do the
> checks for me - and so it should be possible to create a testclient that
> does this, and be able to use this testclient in various test suites.
>

No, you're mixing concerns.

> Is there a simple, clean way to solve this that I'm missing?
>

class MyTestCase(TestCase):

        def check_response(self, response):
                self.assertContains(response, ....)

        def checked_get(self, *args, **kw):
                r = self.client.get(*args, **kw)
                self.check_response(r)
                return r

class SpecificTest(MyTestCase):

        def test():
                r = self.checked_get(...)
                ...

That's how I would do it.

Which is lovely, right up to the point where someone writes a test with:

self.client.get(....)

Extremely likely to occur (in fact 100% probable in any sizeable test suite), won't show up as an error and yet suddenly the tests aren't testing what they should be. 

I don't understand your "mixing concerns" argument - the concern of the TEST client should be testing just as much as it's the concern of the TESTcase and TESTsuite. The hint is in the name :)

Now if the test client was truly decoupled (ie expected to be used in other places) then I'd totally agree with you/Russ about mixing concerns and coupling. But AFAIK it's not - it's explicitly a utility that does all sorts of jiggery-pokery to support testing. Adding a parameter to the initialiser seems a very minor sin (if a sin at all) in comparison.

However, if I'm not convincing anyone I'll shut up about this - the "fix" is simply:

class Tests(TestCase):

  def setUp(self):

     self.client = MyBetterTestClient(self)

(although this does then require any derived test case to remember to call super(Derived, self).setUp(*args, **kw))

I'd just have liked to be able to do that using the existing machinery:

class Tests(TestCase):

   client_class = MyBetterTestClient

Cheers,

Malcolm

[Shai Berger]

Hi again Malcolm,

I think making assertions in the test client is wrong for most situations,
though I can't (of course) say much about your specific case; so I'm -1 on
adding the testcase as a client initializer argument (this would send the
message that such things are encouraged).

However, note that the only use of the client_class attribute in the Django
code is in the simple invocation:

self.client = self.client_class()

which means you can achieve your goal, with no change to Django code, using
this (admittedly, a little abusive) hack:

class MyTestBase(TestCase):

def client_class(self):
return MyTestClient(self)

If your own code refers to self.client_class and expects to have access to its
attributes, you can still arrange it with a little more effort (an elaborate
and transparent way, using a descriptor, and a simple way, using another
attribute, are both left as an exercise to the reader).

HTH,
Shai.

[Michael Hudson-Doyle]

On 5 January 2013 05:23, Malcolm Box <mal...@tellybug.com> wrote:
>
> The general pattern I want to implement is have a test client that makes assertions about all the requests made during a set of tests. For example, it could check that every get() returned cache headers, or that content_type is always specified in responses. Or that the response has certain HTML, uses certain templates etc - ie all of the assertion testing goodness in django.test.TestCase.
>
> My concrete use case is a JSONClient that adds a json_get / json_post methods which makes sure to setup content_type etc on the call, and then validates that what came back was valid JSON.
>
> The simple, wrong way is to do:
>
> def check_response(self, response):
> self.assertContains(response, ....)
> ....
>
> def test():
> r = self.client.get(...)
> self.check_response(r)
>
> but this is error prone, verbose etc etc.
>
> The right thing is that within a test suite, the get()/post() etc to do the checks for me - and so it should be possible to create a testclient that does this, and be able to use this testclient in various test suites.

I don't know that an appeal to authority is useful here, but I
disagree with your "wrong" and "right" assessments. Have you heard of
the concept of the "four phase test"?
http://xunitpatterns.com/Four%20Phase%20Test.html

One of the things about tests is that they need to be understandable
at a glance. Having the "exercise" and "verify" steps clearly
separated helps with this IMHO, and the approach you're proposing
doesn't really lend itself to this.

(If your local library or a friend or someone has the book "xUnit Test
Patterns", the first 150 pages or so are extremely well worth reading)

Cheers,
mwh