Hi, Christian,

Thanks for your informative and quick response.  To the Sourceforge list:
Hi!
I didn't realize Test::Unit was hosted on Sourceforge.  There's no mention
of it in the documentation.

> In general, at the current time, there is no hard-and-fast "rule" 
> which methods are public and which not. The only thing I can say is
> which methods seem "stable" to me based on the discussions on the
> sourceforge mailing list. 

O.K., this is definitely helpful.  I have some more comments, below...

> >     Test::Unit::TestResult::run_count
> >     Test::Unit::TestResult::failure_count
> >     Test::Unit::TestResult::error_count
> >     Test::Unit::TestResult::failures
> >     Test::Unit::TestResult::errors

> > Stable - access point for fundamental properties of a TestResult object

This makes sense to me, but it disagrees with the TestResult POD:  "This
class is not intended to be used directly".  If you're building a custom UI,
though, you need to access it directly.  In fact, I just noticed that a lot
of classes you say are public have this message in their POD...

> >     Test::Unit::TestFailure::to_string
> 
> Discussion - somebody wanted to change to_string() to 
> overloaded "" operator

I see no reason not to support both.  The same goes for
Test::Unit::TestCase::to_string.  The TestFailure POD says "This class is
not intended to be used directly".

> >     Test::Unit::TestFailure::thrown_exception
> 
> Stable - access point for fundamental property of a TestFailure object
> 
> >     Test::Unit::ExceptionFailure::get_message
> >     Test::Unit::ExceptionError::get_message
> 
> Stable - access point for fundamental property of a Exception objects
>          (this method is inherited from Test::Unit::Exception)

O.K., good.  I wasn't sure whether Exceptions* were supposed to be used only
inside Test::Unit, or whether they were supposed to be accessible to users.
Actually, the POD for both ExceptionFailure and ExceptionError begins, "This
class is not intended to be used directly".

*Speaking of exceptions, I noticed one place they aren't used but probably
should be.  TestSuite::build_suite expects a TestCase but merely adds a
warning to $self if the argument is not a TestCase.  build_suite returns
self.  TestSuite::new then returns $self, but $self{tests} is missing, which
will later cause $self->run to fail.  Why doesn't build_suite throw an
exception instead (or at least return undef)?

> >     TestRunner::start
> >     TestRunner::do_run (TestSetup)
> 
> I'd say do_run is not public. There are start, run, and run_and_wait.
> These are intended to be public, and they all use do_run as the 
> primitive.

O.K.  I hadn't noticed TestRunner::run.  I see it will run do_run on any
object that is a Test:

if ($class->isa("Test::Unit::Test")) {
    $a_test_runner->do_run($class, 0);
}

Inspecting do_run, I see that it does indeed work on all Tests:  TestSuites,
TestCases, and TestSetups (even TestDecorators, though I'm not sure why you
would want to pass one).  Speaking of TestDecorators, I see you can pass
any instance of a Test to its constructor (i.e. anything that can("run")),
and get the right behavior.  Surely not all these permutations are intended?

This is the kind of thing that really confused me when I tried reading the
code.  There seem to be many ways to use the code, some of which are not
intended but will not break (at least in an obvious way)--but it's hard to
distinguish between intended and unintended uses.  For instance, I would
have thought that passing a TestSuite to TestSetup was not an intended use
if I hadn't seen it in fail_example.pm.

> >     TestSetup::run (TestResult)
> 
> Not public, I'd say. TestSetup is intended for overriding set_up()
> and tear_down(), not as an entry point.

O.K., this is problematic.  It means the only way to use a TestSetup is to
run it through TestRunner->start, which is bad if you want to make a
different interface for your program, as I'm doing.  You can get around the
problem by constructing a bare TestSuite instead of a TestSetup, but then
you lose the ability to specify a per-suite fixture using TestSetup::set_up
and TestSetup::tear_down (as opposed to per-TestCase fixtures, which you can
still implement by overriding TestCase::set_up and TestCase::tear_down).

> >     TestSuite::run (TestResult)
> 
> Public, IMHO. Entry point.

O.K., this is good because it allows you to run your TestSuite manually in
your own code, which in turn allows you to bypass TestRunner.  Again, this
is important if you want to write a custom UI.

While I was writing this, it occurred to me that you might want to subclass
TestRunner instead of bypassing it, overriding do_run, start_test, and
end_test (and maybe start, if you wanted to use that as an entry point).  On
inspection, I found that do_run contains code that a subclass would need,
intermingled with code it would want to override, such that overriding the
method would require cutting and pasting code from it:

sub do_run {
    my $self = shift;                           ##Needed
    my ($suite, $wait) = @_;                    ##Needed
    my $result = $self->create_test_result();   ##Needed
    $result->add_listener($self);               ##Needed
    my $start_time = new Benchmark();           ##Unwanted
    $suite->run($result);                       ##Needed
    my $end_time = new Benchmark();             ##Unwanted
    my $run_time = timediff($end_time, $start_time);  ##Unwanted
    $self->_print("\n", "Time: ", timestr($run_time), "\n");  ##Unwanted
    
    $self->print_result($result);                      ##Possibly unwanted
    
    if ($wait) {
        print "<RETURN> to continue"; # go to STDIN any case
        <STDIN>;
    }
    if (not $result->was_successful()) {
        die "\nTest was not successful.\n";
    }
    return 0;
}

In addition, you'd inherit a lot of superfluous methods like print_header,
print_errors, and print_failures, which a subclass might want to bypass,
depending on how it prints its headers.

I think we need one of two things:  A way to bypass TestRunner entirely, or
a clean model for subclassing it.  The first already exists, but it won't
allow you to use a TestSetup (since TestSetup->run is not supposed to be
public).
The second is somewhat more desirable esthetically, at least in my opinion,
but it doesn't work at present because of the way code is divided into
methods in TestRunner.

> >     TestCase::run (TestResult)
> 
> Public, too. Unusual entry point, I think, but possible.

Actually, I'm not sure why it would be a useful entry point at all, since
you can always construct a TestSuite with the case as an argument.  Again, I
have the sense that there are many ways to run the suite, some of which were
not intended by the authors.  If they weren't intended, there's a good
chance that they won't be preserved next time there is a change...

> >     TestResult::run (TestCase)
> 
> Not public, IMHO, since a TestResult should be passed to a Test object
> to be run and collect the results.

O.K.

> > As you can see, I had a lot of trouble figuring out how to 
> use the code.
> 
> Sorry for that. Somehow nobody of the developer group wanted to write 
> more documentation. Maybe you can contribute some now?

Yes, I can contribute some documentation.  I'm not sure when I will complete
it, though.  I can't do it on work time, and I want to take the time to do a
really polished version.

One thing that the developers could do that would help a lot with both my
documentation and general readability is to go through the API and decide
which parts should be private and which should be public.  This decision
needs to be made; otherwise there's no clear upgrade path for users.  People
need to know what they are and aren't supposed to call.

I can help with this process.

> > After reading
> > the man pages, I ended up running the example code (in 
> particular, perl
> > TestRunner.pl fail_example) through the debugger to find 
> out how it worked!
> 
> Aarrgh. OK, that is really not how it should be. Sorry again.

Apology accepted. :)
 
> Could you give an outline of what you would expect from such a
> tutorial? This could serve as a starting point for better docs.

The first part would enumerate all the possible ways to use Test::Unit, i.e.
all the entry points to the module:  Constructing a TestSuite and calling
run on it by hand, constructing a TestSuite from inside the suite callback
of a TestCase (then calling the TestRunner->start() on the TestCase),
constructing a TestSuite in the callback as above but instead returning a
TestSetup that wraps the TestSuite, et cetera.  This part of the
documentation should explain the different advantages of those different
approaches.  I think some API clarification would help here:  Which
approaches are actually useful?  The un-useful ones can be excluded from the
public API, and hence from the documentation.

One of the major motivations for using alternate entry points is to write a
custom interface.  Hence the first part would describe how to bypass
TestRunner (or subclass it?)

The second part would be a comprehensive, per-method documentation of each
module.  This exists to some extent, but some modules document only their
general use, not each of their public methods.

As you observe below, the first part of the documentation is much more
important than the second...

> There's more to that than a simple distinction between "public" and 
> "internal", I think. What is needed is documentation on what should
> be used for what purpose, and more examples of realistic usage.

Definitely!  The main thing is to let the users know where to begin.  Then
they can decide what classes they need to know about in detail.  My big
problem was that I didn't know where to start, so I had to delve into the
implementation details to see how things were supposed to work.

> I hope the comments above are somehow useful to you.
> 
> Maybe you can contribute parts of your effort (documentation, code,
> examples, custom TestRunners, ...)?

Yes, this has definitely been useful.  Thank you!  I can contribute
documentation; I'm not sure about the legalities of contributing code, but I
think I can do that (assuming I have anything useful), since my code is not
specific to the sensitive part of our business (investment).  I will find
out.

Thanks again,
--Q

_______________________________________________
Perlunit-devel mailing list
[EMAIL PROTECTED]
http://lists.sourceforge.net/lists/listinfo/perlunit-devel

Reply via email to