Allison Randal wrote:
To answer the questions for 03-revision.t:
Could you take a step back and explain what you're testing? At first
glance, I don't see why we would test the revision number. Just to be
sure that Parrot::Revision got some value during the configure process?
I. The testing tactic:
As I understand it, there are 4 possible cases for which it must be tested.
1. You've checked out trunk -- so file 'DEVELOPING' is present in the
top-level of your Parrot sandbox -- but you have not yet called 'perl
Configure.pl' -- so Parrot::Config has not yet been created.
2. Same as (1), except that you have run Configure.pl, so
Parrot::Config now exists.
3. You've checked out a release version of Parrot. 'DEVELOPING' is not
supposed to be included in release versions. You have not yet
configured, so Parrot::Config has not yet been created.
4. Same as (3): release version; no 'DEVELOPING' file -- but you have
run Configure.pl and Parrot::Config now exists.
Among the following tests I was trying to test each of these 4 possible
cases:
t/configure/17-revision_no_DEVELOPING.t
t/configure/18-revision.t
t/postconfigure/02-revision_no_DEVELOPING.t
t/postconfigure/03-revision.t
(The order in which these tests is run corresponds to Cases 3-1-4-2
above. I selected that order to get Devel::Cover to consolidate results
properly.)
II. The testing strategy:
Parrot::Revision is a piece of code that falls into the category of "I
Wouldn't Have Written It This Way Myself ... But Other People Rely on It
So I Must Not Break It If I Attempt to Refactor It."
It is supposed to make available "parrot's current and configure time
revision." These are only available as fully qualified variables:
print $Parrot::Revision::current;
print $Parrot::Revision::config;
$current is (apparently) intended to hold the current revision number as
determined by a complicated search of your SVN or SVK file systems. If
you've checked out a release version of Parrot (rather than working from
the repository), then presumably whatever is in your SVN or SVK systems
is irrelevant, so $current is immediately set to 0. If you're a Parrot
developer, your SVN and SVK systems are searched and the revision number
is placed in $current. Ah, but what if that search is unsuccessful?
Then $current is set to 0.
Then, whatever value has been assigned to $current is assigned to
$config. So, if at this point, $current is '18411', then $config's
value becomes '18411' as well. More problematically, if $config's value
at this point is '0', then $config gets assigned a value of '0' as well.
This is why I originally felt that, in certain of the cases described
above, it would be appropriate to test for the equality of $current and
$config. But I can tell that if (a) you're working from the repository
but (b) the search of your SVN or SVK systems is unsuccessful and (as we
shall soon see) (c) you have not yet run Configure.PL, you can get a
meaningless value of '0' for $config at this point.
But wait! There's more!
At this point, Parrot::Revision asks, "Have you run Configure.pl?" If
the answer is "Yes," it then looks up the value of $PConfig{revision}
(which, under the hood, is
$Parrot::Config::Generated::PConfig{revision}). It then assigns this
value to $Parrot::Revision::config, overwriting whatever was previously
assigned thereto. But if you have *not* yet configured, then you are
stuck with whatever value of $config came as a result of the preceding
code, whether that value is meaningful or not.
Clear as mud, right?
My hunch is that while Parrot's *version* number (found in top-level
file 'VERSION' and reported by Parrot::BuildUtils::parrot_version()) is
important for building Parrot, Parrot's repository *revision* numbers
are less important. But *how much* less important, I cannot yet say.
FWIW, here are locations where these $current and $config are used
(setting aside my 4 test files above).
config/gen/revision.pm:30: my $revision = $Parrot::Revision::current;
t/distro/file_metadata.t:219: unless ( $Parrot::Revision::current or
`svk ls .` ) {
tools/build/revision_c.pl:46: return ${Parrot::Revision::current};
tools/build/revision_c.pl:51: return ${Parrot::Revision::config};
kid51
