[ckan-dev] Testing: a helper?

[Toby Dacre]

On 5 March 2013 10:00, Sean Hammond <sean.hammond at okfn.org> wrote:
>> I've been looking at the tests as I need to write a few.  Here is a
>> possible approach, after some previous feedback
>>
>> https://gist.github.com/tobes/5081840
>>
>> The idea is to have a testing helper that means that we can write
>> simpler tests more resistant tests.  Also having simple test classes
>> not inheriting seems to be a good idea as trying to understand which
>> code was being called did get complicated and some things got secretly
>> cached which were hard to find.
>>
>> Currently things like the create test data just stuff things into the
>> database rather than going through logic actions so the data may not
>> match what would actually created.
>> Also many tests are having to worry about creating the app and doing
>> things like changing the config - this can all be simplified.
>>
>> Sean has suggested that this should be not a class but rather a bunch
>> of helper functions - this could be done but then tests would need to
>> ensure that things were reset but this would be a minor issue.
>>
>> It would be good to get more feedback on this.
>
> Here's my thoughts on this, pretty much repeating what I said in the
> (previous) gist and on irc:
>
> I agree we should be using webtest for our functional tests.
>
yes webtest seems much nicer.  We should also decide how we want to
interface with it as far as html responses are concerned.  We get
several options such as beautiful soup, and pyquery (really nice
behaves like jquery) but these add requirements like lxml to testing
so we should be aware of that and do we also want to limit the
interfaces so that the tests are more understandable without having to
know every possible lib

> I think this looks really promising, if we can just build them out of
> calls to simple functions like create_user() etc and then asserts, the
> functional tests should be really easy to read and write. I think we
> should keep the tests themselves pretty high level, so they're almost
> like user stories translated directly to unit tests, and almost
> human-readable. Then developing a new feature could go: Idea -> User
> stories -> Functional tests -> Implementation.
>
> I think we should follow most of what the Pylons guide says about unit
> tests (http://docs.pylonsproject.org/en/latest/community/testing.html)
> in our functional tests: each test method only testing one thing, each
> method being standalone, good method names that completely describe
> the purpose of the test, etc.
>

It would be good if we could make our tests more readable and simpler
to understand and debug when they fail

> Also from the Pylons guide, I think it's important for test modules to
> be standalone and not contain too much cleverness or generic shared
> code that obscures the tests, and a big part of this is not sharing
> code between test modules.
>
> One thing about functional tests (that's maybe different from unit
> tests) is that they tend to do the same thing, e.g. creating a user,
> hundreds or thousands of times across all the different functional
> tests, most of the times creating the user is not what you're testing
> just something you need first in order to get to your test, and each
> time that's several lines of code, so having a shared function that
> reduces that to one line of code each time is worth it. (The lines of
> code saved can be ridiculous:
> https://github.com/okfn/ckan/pull/189/files
>
> But I think we should limit it to a shared helpers module containing
> functions, no classes, to reduce the potential for cleverness. In
> Toby's example helper class there's already quite a bit of cleverness
> creeping in and I think the temptation will mean that this only gets
> worse over time.
>
> Some other things:
>
> I think helper methods need to be named really explicitly, e.g.
> create_user_via_api() not create_user().
>
The reason that this is create_user() not create_user_via_api() is
that it is designed to create a user correctly.  Now at the moment it
uses the api but that is just an implementation detail.  If I wanted
to use the api to create a user (say to test the user api) I would use
api_action('create_user', ...).  The idea is that it is a little
helper to setup test data correctly.

> They should take params not dicts, I should be able to do
> create_user_via_api(name='seanh', email='sean.hammond at okfn.org',
> password='xxx'), this is much nicer than having to pass in a dict.
>

The reason I have done this is so that we do not have name clashes and
also partly because how it is done in existing tests.
I'm not sure about this it would be a simple change but I fear it
might bite us you can use dict(name='...', password='...') for similar
effect.
> Toby's create_user() function lets you create a user passing only the
> username and it'll generate an email and password for you. I think
> this is bad as it obscures what the test code is actually doing. In
> CKAN, the fact is that to create a user you have to give a username,
> email and password, so the functional tests should reflect this
> explicitly.
>
Again this is about quickly setting up a user for tests - this is NOT
a way for testing user api functionality
Likewise the other create functions are for setting up test data maybe
the names need to be clearer

> I don't like the _app, _sys_admin, _config and _original_config in the
> helper class, I would let each test class deal with all this itself,
> and just provide helper functions so that e.g. making a sysadmin is a
> really simple and explicit one-liner.
>

The reason for _app and others is for the test writer not to have to
do too much work.  If I want to just make some api_action() calls why
should I have to mess about setting up the app and what if I want to
load a test extension or change the config? Why should I need to
understand all the intricacies of those systems and risk doing it
wrong in my tests.  What if we make changes to how the app gets set-up
why should we have to then alter every test?

I understand the concern over magic and hidden stuff - I've been
bitten enough times by this sort of thing too.  So yes we need to be
careful but It feels that if this is all in one place and self
documented that at least we can keep them to a minimal.

Also the other option is to keep passing app around which feels a bit
of a magical thing anyhow, and again relies on the tests creating the
correct app.

If we went for this approach we could 'lock down' the test helper to
stop it just expanding on a whim and make sure it remains consistent
and understandable.

Toby
> _______________________________________________
> ckan-dev mailing list
> ckan-dev at lists.okfn.org
> http://lists.okfn.org/mailman/listinfo/ckan-dev
> Unsubscribe: http://lists.okfn.org/mailman/options/ckan-dev