[ckan-dev] Testing: a helper?

[Toby Dacre]

On 19 March 2013 23:01, Ryan Clark <ryan.clark at azgs.az.gov> wrote:
> I just wanted to see if this TestHelper concept has gone any farther? Toby's
> gist looks EXTREMELY helpful from my vantage point, and I'd like to utilize
> it right away!

I'm going to do another push on it probably next week.  This week
things are a bit up against the wall.  The last decision was for me to
do a bit more work on it with some real world tests and then go back
to the team for review/comment.

Thanks for your positive thoughts.

Toby

>
> Thanks!
> ____________________
>
> Ryan Clark
> Arizona Geological Survey
> ryan.clark at azgs.az.gov
> (520) 302-4871
>
>
>
>
>
>
>
> On Mar 6, 2013, at 11:23 AM, Vitor Baptista <vitor at vitorbaptista.com> wrote:
>
> Hey Sean,
>
> 2013/3/6 Sean Hammond <sean.hammond at okfn.org>
>>
>> I completely disagree with both of you! This might be a good thing to
>> discuss at our dev meeting on Thursday..
>
>
> :P
>
>>
>> I don't understand why these helper methods would be used only for
>> setup and not for actually testing.
>
>
> The thing is that simply needing a user (or organization, whatever) created
> for a test is much more common than trying to test that the user creation
> actually works. So, while I think that a create_user() is useful for many
> tests, I would say that create_user_via_api() is only useful if we're
> actually testing the user creation via the api (and, as such, I would rather
> put that logic in the test, and not in a helper).
>
> But, obviously, the helper itself needs tests, so we guarantee that it's
> create_user(), whatever it's using (if the API or the code), works.
>
>>
>> In ckan you can create
>> users via the api, or by the web interface, so we need functional
>> tests for both ways, so we need create_user_via_web() and
>> create_user_via_api(). Admittedly create_user_via_api() would just by
>> a trivial wrapper around call_action_api() so maybe it's not even
>> needed, but I think it might make things more readable.
>
>
> Again, these tests are important, but only when you're actually testing that
> functionality, and not in every random test that needs an user.
>
>> ...
>>
>> I hear what you're saying: if we are just creating a user as part of
>> the setup for our test then we don't care about whether it's created
>> via the api or web interface. But for testing the create user
>> functionality itself, we're going to need create_user_via_api() and
>> create_user_via_web() anyway, so why not just reuse them for setup?
>> Why make an additional create_user() method? We could name them better
>> though e.g. it could be two helper modules like api.create_user() and
>> web.create_user()
>
>
> Because that adds noise to the test code. I agree that explicit is better
> than implicit, but that's not the case all the time. We need abstractions,
> so we focus on what's important. If we follow your line of thought, we would
> end creating something like create_user_via_api_v1().
>
> I understand that, if we just have create_user(), we would have to repeat
> most of its code whenever we actually need to test the user creation via the
> API. Although I would like to keep the tests DRY, I'm OK with this
> repetition.
>
>> If we really need to reset ckan after each test module (and recreating
>> the wsgi and test apps doesn't do it) then that's a pain, but I'd make
>> a ticket to fix that in ckan, and in the meantime do it in each test
>> module, rather than using a base class to do it by magic. It seems to
>> be that in trying to write tests for CKAN we've discovered something
>> awkward about using CKAN (the need to reset stuff), and we should fix
>> that in CKAN rather than papering-over it with test cleverness that
>> makes it look (from the point of view of a test module) like you don't
>> have to reset CKAN when in fact you do.
>
>
> Totally agree with you in the case of resetting the app.
>
> But I guess some of that setUp/teardown code will be really hard to get rid
> of, mainly the _original_config. We need to keep the tests as independent as
> possible from each other, so we need to clear up anything that we create, be
> it into the database (using and rolling back from transactions), or be it
> config changes (by saving the original and rolling back into the teardown).
> You could argue that each test that changes the config is responsible of
> clearing it up afterwards, but I guess this pattern is common enough to
> justify putting it into a common (helper?) class.
>
> Cheers!
> Vítor.
> _______________________________________________
> 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
>
>
>
> _______________________________________________
> 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
>