lib/Parrot/Revision.pm appears to have no unit tests written
specifically for it. Write some, then perform coverage analysis to
make sure all code is covered by tests.
$ ack --nohtml --nophp 'Parrot::Revision'
config/gen/revision.pm:23:use Parrot::Revision;
config/gen/revision.pm:30: my $revision = $Parrot::Revision::current;
config/gen/revision.pm:31: my $entries =
$Parrot::Revision::svn_entries;
lib/Parrot/Revision.pm:6:Parrot::Revision - SVN Revision
lib/Parrot/Revision.pm:10: use Parrot::Revision;
lib/Parrot/Revision.pm:12: print $Parrot::Revision::current;
lib/Parrot/Revision.pm:13: print $Parrot::Revision::config;
lib/Parrot/Revision.pm:21:package Parrot::Revision;
t/distro/file_metadata.t:13:use Parrot::Revision;
t/distro/file_metadata.t:219: unless ( $Parrot::Revision::current
or `svk ls .` ) {
tools/build/revision_c.pl:25:use Parrot::Revision;
tools/build/revision_c.pl:46: return ${Parrot::Revision::current};
tools/build/revision_c.pl:51: return ${Parrot::Revision::config};
Here's the result of ack-ing for svn_entries in trunk:
#####
[parrot] 502 $ ack --nohtml --nophp svn_entries
config/gen/revision.pm
31: my $entries = $Parrot::Revision::svn_entries;
lib/Parrot/Revision.pm
27:our $svn_entries = undef;
42: elsif ( defined $svn_entries and -r $svn_entries ) {
43: open FH, '<', $svn_entries
44: or die "Unable to open file ($svn_entries). Aborting. Error returned was: $!";
#####
If $svn_entries starts out undef-ed and is never assigned to, it can never pass the test for
definedness in line 42 above, which means that the subsequent elsif stanza can never be
reached -- and should therefore be refactored out of existence.
Is my interpretation correct?
Thank you very much.
kid51
this code should be ripped out.
~jerry
Okay, but we'll have to look at what to do with config/gen/revision.pm as well. It contains
this code:
sub runstep {
my ( $self, $conf ) = @_;
my $revision = $Parrot::Revision::current;
my $entries = $Parrot::Revision::svn_entries;
$conf->data->set(
revision => $revision,
SVN_ENTRIES => $entries
);
...
Here is the output of 'ack' for SVN_ENTRIES in trunk (post-Configure.pl, in this instance):
[parrot] 521 $ ack --nohtml --nophp SVN_ENTRIES
config/gen/revision.pm
35: SVN_ENTRIES => $entries
config_lib.pasm
274: set P0["SVN_ENTRIES"], P1
lib/Parrot/Config/Generated.pm
30: 'SVN_ENTRIES' => undef,
The 'undef' value for key 'SVN_ENTRIES' in the configuration hash is, now, not at all surprising
and will simply go away when we eliminate the offending code from lib/Parrot/Revision.pm.
But do we have to do anything with config_lib.pasm? (I've never patched any .pasm files).
Thanks.
kid51
I see that config_lib.pasm is generated by running Configure.pl. So its SVN_ENTRIES key goes
away as well.
kid51
I use Subversion as my local version control client, so I'm not currently able to write tests for
the svk-associated stanza. Hence, testing coverage is lower than I would normally be
satisfied with. Patches from svk users would be welcome.
As with my other patch submissions, unless I hear screams I'll apply to trunk in 3 days or so.
They've already been applied and tested in the 'reconfigure' branch.
Everything worked fine for me with Svk, barring some fuzz when updating
MANIFEST.SKIP and the lack of Svn properties set in the diff. Nice work.
-- c
>
>
> Everything worked fine for me with Svk, barring some fuzz when
> updating
> MANIFEST.SKIP and the lack of Svn properties set in the diff. Nice
> work.
>
This was the first time that I used tools/dev/mk_manifest_and_skip.pl
to (re-)generate MANIFEST and SKIP. The diff itself was generated
via svn diff. What could I have done differently to get a cleaner
patch?
jimk
I'm leaving the ticket open in the hope that some project member who uses SVK for version
control can add tests for that VCS under t/configure/ and t/postconfigure/.
I see that I should have also patched config/gen/revision.pm to eliminate this line:
my $entries = $Parrot::Revision::svn_entries;
...
SVN_ENTRIES => $entries
The patch attached accomplishes this.
kid51
4 months later ... and no response from SVK users! Can anyone give me
some feedback here? (I'd like to close the ticket.)
I'm throwing in the towel on the question of getting unit tests for the
SVK or GIT portions of this package. Closing ticket.