On Wed Jul 16 06:27:16 2008, julianalbo wrote:
> On Wed, Jul 16, 2008 at 1:12 PM, James Keenan via RT
[snip]
>
> > If no one gets to this today I will try to work on this this evening.
>
> Go for it.
>
On Wed Jul 16 04:12:35 2008, [EMAIL PROTECTED] wrote:
> [snip]
> The control flow here is somewhat awkward and creates additional
> branches which will need coverage in testing (cf t/configure/017 and 018):
>
> 33 if (defined ($prev) && ($revision ne $prev)) {
> 34 $revision = 'unknown' unless defined $revision;
>
This is a case where things can be improved by (a) making the control
flow a bit more explicit and (b) using Devel::Cover for coverage
analysis during testing.
Suppose we rewrite line 34 as:
> if (! defined $revision) {
> $revision = 'unknown';
> } else {
> # do something else
> }
If we were to try to write tests for both the defined and undefined
cases for $revision, we'd soon see that in the undefined case we would
get an uninitialized value warning on line 33 in the second part of the
outer 'if' branch:
> ($revision ne $prev)
Line 33 strongly assumes that $revision is defined, so to question its
definedness on Line 34 is dubious.
So I re-ordered this part of the module, then wrote tests which cover
each branch and placed them at the end of
t/configure/017-revision_from_cache.t and in new test file
t/configure/061-revision_from_cache.t. The code is more verbose, less
idiomatic -- those things we can fix later. But in the meantime we can
see the flow better and write tests which exercise more of its nooks and
crannies.
Reini et al: Please evaluate the attached patch. Alternatively, you
can do: svn co https://svn.perl.org/parrot/branches/revisionpm.
As usual, I'll apply in a couple of days if there is no objection.
Thank you very much.
kid51
