On Fri, Feb 15, 2013 at 3:23 PM, Nick Wellnhofer <[email protected]> wrote:

> The class Clownfish::Test::TestEngine incorporates the features of
> Lucy::Test::TestBatch but with pluggable output formats which use the
> abstract class Clownfish::Test::TestFormatter.

The TestFormatter class seems like a nice step forward.

> Every test class now inherits from Clownfish::Test::TestBatch instead of
> being an inert class.

+1

> Lucy::Test::TestEngineLucy inherits from Clownfish::Test::TestEngine and
> populates a VArray with all the TestBatch classes. This makes it possible to
> lookup tests by name so we don't need Perl bindings for every single test
> class.

I see why it makes sense to avoid specifying bindings for every test class.
It's worse than you think -- each test batch needs a binding not just for
Perl, but for every host language!

Working around that problem is straightforward, though -- see code for an ad
hoc run_test_batch() function below.

I'm more concerned about extracting all the test routines from TestBatch into
the new class TestEngine.  (Is the idea to support alternative TestEngine
implementations, e.g. one that provides xUnit-style assertions?)

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.

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?

> For now, I only ported a single test to the new layout. If there aren't any
> objections, I'll port the rest of the tests and switch to the new test
> infrastructure.

Thanks for bringing the issue to the list and building consensus.

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.

Marvin Humphrey


int32_t
LucyTest_run_test_batch(const char *name, const char *format) {
    // Obtain a TestFormatter.
    TestFormatter *formatter = NULL;
    if (strcmp (format, "tap") == 0) {
        formatter = (TestFormatter*)TestFormatterTAP_new();
    }
    // ...
    else {
        THROW(ERR, "Can't find formatter for '%s'", format);
    }

    // Find the test class to run.
    TestBatch *batch = NULL;
    if (strcmp(name, "Lucy::Test::Highlighter::TestHeatMap") == 0) {
        batch = (TestBatch*)TestHeatMap_new(formatter);
    }
    // ...
    else {
        THROW(ERR, "Can't find test class '%s'", name);
    }

    // Run tests, clean up, and return success/failure.
    int32_t result = TestBatch_Run_Tests(batch);
    DECREF(batch);
    DECREF(formatter);
    return result;
}

Reply via email to