couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Dionne <dio...@dionne-associates.com>
Subject Re: [PATCH] Eunit Tests
Date Mon, 16 Feb 2009 11:49:18 GMT

Robert Dionne
Chief Programmer
dionne@dionne-associates.com
203.231.9961



On Feb 16, 2009, at 5:27 AM, Jan Lehnardt wrote:

> Hi Robert,
>
> thanks for your feedback.
>
> On 16 Feb 2009, at 03:42, Robert Dionne wrote:
>
>> If I read the EUnits docs correctly you can do both, have them in  
>> the files as well as in separate file. I believe if you run  
>> eunit:test([some_module]) it will run both tests in the module as  
>> well as look for some_module_tests and run those also.
>
> Both is possible, my original version (from the stats patch)
> used inline test code and this patch now uses external
> modules.
>
>
>> I think there are some modules where the amount of complexity  
>> among all the private methods warrants tests in the modules, and  
>> others where tests outside are more practical.
>
>
> I was contemplating that as well. The stats code uses a record
> to hold aggregate values. When testing the get() functions inline,
> I could just make use of the already defined `-record()`. When
> I moved the tests to a separate file, the record definition was
> missing and I had to create a `couch_stats.hrl` file to include
> the record in both places.
>
> The stats module also takes care of converting the record to
> the JSON-Erlang-term structure that gets send to the HTTP
> handler. The public API never sees the record.
>
> At first this seemed awkward to me as an implementation
> detail spewed over to the test code. I then realized though,
> that while the HTTP API never makes use of the record, it
> is very much part of the public API of the stats module and it
> is correct to explicitly import that part of the API into the test
> module.
>
> Lesson learned: There is a CouchDB-public API that sends
> JSON-Erlang-Terms to HTTP clients and a module-public
> API that other parts of CouchDB can use.
>
> Why this story: I think one basic premise* of unit testing is
> testing only the public API of a module, the amount of
> internal, private complexity is of no business to the test

Hi Janl,

   I think I largely agree with you though I'm not quite sure about  
this last point. You discovered certain aspects like the -record were  
indeed public parts of the module, though not necessarily public to  
the HTTP API, and thus ought to be tested with separate modules. This  
makes good sense and of course having all the tests in a separate  
location has a lot of advantages.

  Take  couch_btree for instance.  It already has a test() function  
which if you look closely exercises most of the public functions, so  
one could readily put these in a separate module ( which I actually  
did when I first started playing with runner) .  However consider  
chunkify or modify_node. I suppose one could have a separate  
couch_btree_tests module that just includes couch_btree in order to  
test these or one could export those functions. I'm not familiar with  
Erlang internals to know if these approaches significantly alter the  
code paths. How would you test something like chunkify?

In any event my point is only that you need to do both. I think the  
EUnit folks intended this judging from the documentation[1].
Anyway this may all be moot if the lawyers opine against the LGPL. If  
not I can go either way.

Regards,

Bob



[1] "if you want to separate your test code from your normal code (at  
least for testing the exported functions), you can simply write the  
test functions in a module named m_tests (note: not m_test), if your  
module is named m. Then, whenever you ask EUnit to test the module m,  
it will also look for the module m_tests and run those tests as well."




> code. And parts of the code in a module that should be
> unit-tested while not being part of the module's public
> API probably better live in a separate module. YMMV :)
>
> * it's the very idea that you can swap out implementations
> without breaking your API's contract.
>
> Cheers
> Jan
> --
>


Mime
View raw message