On Feb 16, 2013, at 20:01 , Marvin Humphrey <[email protected]> wrote:

> OK, got it.
> 
> The rationale for running all the tests within a single process, despite the
> potential for interference, is that we won't have to write a TAP parser.
> 
> And the rationale for passing around the TestEngine as the first argument to
> individual test subroutines rather than the TestBatch is that the TestEngine
> is maintaining pass/fail statistics for the entire suite.
> 
> Right?

Exactly.

>> TestEngine only has to be subclassed once per parcel.
> 
> That's true, but under the proposed changes TestEngine becomes mandatory -- so
> you have to deal with it up front.  In other words, the startup development
> costs are higher, though they are amortized over time.
> 
> That's the reverse of what we ought to be doing -- we should strive to
> minimize the amount of effort it takes to write and run a single test case in
> isolation.

I don't think my previous implementation of TestEngine would increase startup 
costs considerably more than other solutions.

> Another downside is that it would be nice to customize the behavior of the
> test routines per test class, but if the first argument to the test routine is
> a TestEngine rather than a TestBatch, we can only customize the behavior at
> the suite level.

This is a valid concern.

> I appreciate that we want a convenient way to run all the tests for the C
> bindings.  However, I'd like to avoid changing the first argument to our
> test routines.
> 
> I believe that it's possible to solve this dilemma by having the test runner
> inspect the TestBatch after Run_Tests() completes.
> 
>    TestBatch_Run_Tests(batch);
>    test_runner->num_passed  += TestBatch_Get_Num_Passed(batch);
>    test_runner->num_failed  += TestBatch_Get_Num_Failed(batch);
>    test_runner->num_skipped += TestBatch_Get_Num_Skipped(batch);
>    // …

See my new branch 'clownfish-test-v2' for a solution based on this idea. I 
agree that it's better than my previous approach.

> OK, I think I see what you mean.  The individual test subroutines are
> "assertions" in xUnit parlance, but an xUnit "TestCase" corresponds roughly to
> a Perl test script -- which is what I think you mean by "a single test".

OK, I thought a TestCase was a single "assertion". If it's equivalent to a test 
script, I didn't understand the xUnit terminology.

>> But xUnit's TestSuite can also contain a collection of other TestSuites
>> which is what I'm trying achieve with the registry in TestEngine.
> 
> Hmm, I don't really grok why that's needed right away.  Why isn't a flat
> collection of test subclasses good enough for now?

A flat collection of subclasses is OK. The confusion comes from my 
misunderstanding of the xUnit terminology.

> `INT_EQ` is good.  I'd argue that it should start taking an `int64_t` rather
> than a `long`.

+1

> `STR_EQ` is good as implemented in CFC, but the main string type in Clownfish
> is an object (named "CharBuf" at this time) rather than a NUL-terminated C
> string.  If we have `STR_EQ` take two CharBuf objects, we'll find a lot more
> use for it.

Maybe we should have STR_EQ, CB_EQ and even CB_EQ_STR?

> A `PTR_EQ` which takes two `void*` arguments might also come in handy.

I don't think this would be useful. It doesn't save typing and a diagnosis like 
"Got pointer 0xABCD, expected 0x1234" doesn't really help.

>> I'd also like to get rid of the first parameter of the test macros. IIRC,
>> you already proposed a global test object.  Another solution could be based
>> on variadic macros but these aren't supported on all platforms, are they?
> 
> Variadic macros are not a blocker, as I'll explain in a moment.  However, I'm
> not enthusiastic about macros which would rely on having local variables named
> according to conventions -- too fragile and fiddly.

Fair enough.

> It turns out that we almost never need the printf-style formatted printing in
> test routines -- a single "message" argument suffices in almost all cases.  On
> those rare occasions where a message needs to be customized, it can be
> prepared in a separate sprintf() operation before the test routine is called.
> 
> I got rid of the variadic routines in the Charmonizer test harness a while
> back:
> 
>    http://s.apache.org/gK
> 
> We can likewise get rid of all the "V" routines in TestBatch, replacing e.g.
> the inert routine `pass` and the method `VPass` with a single `Pass`
> method.

In the CFC tests, the printf-style is used quite a lot. IMO it's quite 
convenient so I'd like to keep it.

> I feel like there are a lot of good ideas in this thread, but I'm concerned
> about trying to do too much at once.  Does it make sense to go after some
> low-hanging fruit?
> 
> For instance, we agree on these two things:
> 
> *   `TEST_TRUE` should be renamed `OK`.
> *   The first argument to `OK` should be hidden.
> 
> Maybe we can start there?  It would make the other changes under consideration
> less consequential.

My main concern for now is to adapt the tests for the C bindings. My second 
approach in 'clownfish-test-v2' is less invasive than the first one and to be 
honest I lost a bit of my enthusiasm of making bigger changes to the test 
infrastructure ;)

The only thing I really care about is that adding a new test class and making 
it work with every host language requires the minimum amount of changes to the 
rest of the code. For the C bindings, a VArray with all the test classes would 
be ideal. In my new branch, I moved that code from TestEngine to Lucy::Test but 
I'm not sure if that's the better idea.

Nick

Reply via email to