On Sat, Feb 16, 2013 at 3:30 AM, Nick Wellnhofer <[email protected]> wrote:
> The idea of the TestEngine class is to provide a replacement for Perl's
> 'prove' tool. For the Perl bindings we only ever run a single test batch
> with TestEngine but it's also possible to run multiple batches and print a
> summary at the end. I plan to use this feature for the C bindings.
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?
As a bonus, it's nice not to have to create stub .t files to drive the test
runner under the C bindings as we have to under Perl. (FWIW, there are ways
to do away with those stub .t files under Perl using recent versions of
TAP::Parser, but it's not worth either the surprise factor of the
unconventional configuration or the prereq.)
>> It seems unfortunate that users must provide *both* a subclass of TestBatch
>> *and* a subclass of TestEngine up front. For the sake of simplicity, I think
>> we should only be asking people to learn one class when starting out -- so
>> subclassing Clownfish::Test::TestBatch ought to be enough.
>
> 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.
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.
> It's basically a registry of all test batches replacing the huge 'if … else
> if'-chain in Lucy/Build/Binding/Misc.pm. There are other ways to achieve
> this like adding a Register_Test_Batch method to TestEngine or simply using
> an 'inert' function per parcel like you proposed.
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);
// ...
>> FWIW, take away the test methods, and TestEngine becomes effectively an xUnit
>> TestSuite. The xUnit system isn't perfect, but the division of roles into
>> TestCase (TestBatch for us), TestSuite and TestRunner makes sense IMO. Shall
>> we formalize our own variants?
>
> I'm not familiar with xUnit, but it seems that my TestBatch class actually
> corresponds to xUnit's TestSuite, and my TestEngine to xUnit's TestRunner.
> xUnit's TestCase seems to encapsulate only a single test.
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".
For the record, the name "TestBatch" originated under the Charmonizer test
harness as an approximation of a Perl test script, inspired partly by
famililarity I'd gained with Test::Simple's guts by hacking on David Wheeler's
JavaScript port, Test.Simple. "TestBatch" was supposed to convey "a batch of
test subroutines" -- but if the name is causing confusion because it seems to
imply "a batch of test *files*", perhaps our base test class should be named
TestCase instead, a la xUnit.
The parallel between a Perl test script and an xUnit TestCase is not perfect,
as there is more of an emphasis on setting up and tearing down the "fixture"
or "test context" in xUnit. However, those are nice features, and I think it
makes sense to move in that direction.
> 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?
(FWIW, what you describe sounds similar to the "subtest" feature available in
recent versions of Test::More.)
>> PS: One other thing you did in the CFC tests is you used `OK` and friends
>> instead of `TEST_TRUE` and so on. Would you prefer to see Clownfish::Test
>> and thus all of Lucy's tests do the same? I wouldn't mind.
>
> Yes, I'd prefer 'OK' inspired by Perl's Test::More. For the equality test
> macros, I used 'INT_EQ' and 'STR_EQ' in CFC which don't correspond to other
> Perlish names.
`INT_EQ` is good. I'd argue that it should start taking an `int64_t` rather
than a `long`.
`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.
A `PTR_EQ` which takes two `void*` arguments might also come in handy.
> I also found that in many places the Lucy test suite uses 'TEST_TRUE' where
> the equality test macros could be used. Now would be a good opportunity to
> fix this.
If I recall correctly, that's often because of precision limits, or because an
Equals() method is being invoked.
> 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.
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.
...
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.
Marvin Humphrey
