[perl #41455] [NEW] and [PATCH]: tools/build/ops2pm.pl refactored

0 views
Skip to first unread message

James Keenan

unread,
Feb 5, 2007, 9:58:27 PM2/5/07
to bugs-bi...@rt.perl.org
# New Ticket Created by James Keenan
# Please include the string: [perl #41455]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=41455 >


Please find attached new files and patches:

New files:

lib/Parrot/Ops2pm/Auxiliary.pm
lib/Parrot/Ops2pm/Utils.pm
t/tools/ops2pmutils/00-qualify.t.txt
t/tools/ops2pmutils/01-ops2pmutils.t.txt
t/tools/ops2pmutils/02-usage.t.txt
t/tools/ops2pmutils/03-new.t.txt
t/tools/ops2pmutils/04-prepare_ops.t.txt
t/tools/ops2pmutils/05-renum_op_map_file.t.txt
t/tools/ops2pmutils/06-load_op_map_files.t.txt
t/tools/ops2pmutils/07-no_ops_skip.t.txt
t/tools/ops2pmutils/08-sort_ops.t.txt
t/tools/ops2pmutils/09-prepare_real_ops.t.txt
t/tools/ops2pmutils/10-print_module.t.txt
t/tools/ops2pmutils/11-print_h.t.txt
t/tools/ops2pmutils/testlib/Capture.pm

The test files above have been named with a '.txt' extension so as to
show up properly in browsers. In reality, their names should end in
'.t'.

Patches

MANIFEST.patch.txt
tools/build/ops2pm.pl.patch.txt
config/gen/makefiles/root.in.patch.txt

These files represent a refactoring of the functionality currently
found in tools/build/ops2pm.pl. Most of that functionality is now
found in the methods of lib/Parrot/Ops2pm/Utils.pm; some auxiliary
subroutines are found in lib/Parrot/Ops2pm/Auxiliary.pm.

The purpose of this refactoring is to increase the testability and,
by implication, the future maintainability and refactorability of the
ops2pm functionality. tools/build/ops2pm.pl is a Perl 5 program
invoked by 'make' at the very beginning of the build process. While
successful execution of 'make' implies successful execution of
ops2pm.pl, having its functionality stored in subroutines inside Perl
modules means that that functionality can be tested by test scripts
and that future refactoring can proceed in a test-driven, more fine-
grained manner.

This is component-focused testing in the same way as was my
refactoring two months ago of another Parrot build tool -- tools/
build/pmc2c.pl -- into a Perl module -- Parrot:Pmc2c::Utils. As was
the case with those revisions, the accompanying test suite -- t/tools/
ops2pmutils/*.t -- is meant to be run when your Parrot filesystem is
in a "post-Configure.pl, pre-make" state. To facilitate that, the
tests in this suite have been added to those targeted by "make
buildtools_tests".

If you would like to get a tarball of the new files and the patch to
tools/build/ops2pm.pl, you may do so here:
http://thenceforward.net/parrot/ops2pm.refactored.tar.gz

If you would like to see how well the test suite covers the code in
the new Perl modules, you may do so here:
http://thenceforward.net/parrot/coverage/ops2pm/

If you would like to read the POD in .html format, you may do so
starting here:
http://thenceforward.net/parrot/pod/

The places where code in the new Perl modules is *not* covered
(ignoring code to handle failures to open and close files) are places
where the operations of ops2pm.pl were so mysterious that I couldn't
figure out how to test them. Suggestions for additional tests which
would boost the test coverage are welcome.

Patches for the POD in lib/Parrot/Ops2pm/Utils.pm are particularly
welcome. When I was refactoring pmc2c.pl late last year, I was
starting from an advantageous position: almost all the code was
already refactored into subroutines, and almost all the subroutines
had documentation. So although moving those subs into a separate
package and, later, converting them into methods was a delicate
process, the direction was clear from the outset. With ops2pm.pl,
only about one-third of the original code was chunked into
subroutines. In order to provide subroutines for calls to Test::More
functions in the test suite to chew on, I had to transfer a lot of
code that wasn't in subs into subs, then write documentation for
those subs. Where the documentation is sparse or inaccurate -- well,
that's the point where my understanding of Parrot goes off a cliff.

I've been working on this code in the 'buildtools' branch of our SVN
repository so as not to cripple ops2pm.pl in trunk. Join me there
for all the fun! (But beware: kid51 gets pissed if you add code
without adding additional tests -- because that makes the coverage
ratio go down!)

Note: I would just like to add that the code I was starting from was
high-quality Perl code -- as it should have been, given all the work
people have put into it over a five-year period. My thanks to all
those who have done so; I've listed your names (as based on svn log)
in the POD. And thanks to the Perl hackers -- schwern, chromatic,
petdance, pjcj, et.all. -- who have contributed to the testing
apparatus that enabled all this refactoring to take place safely!

kid51

00-qualify.t.txt
01-ops2pmutils.t.txt
02-usage.t.txt
03-new.t.txt
04-prepare_ops.t.txt
05-renum_op_map_file.t.txt
06-load_op_map_files.t.txt
07-no_ops_skip.t.txt
08-sort_ops.t.txt
09-prepare_real_ops.t.txt
10-print_module.t.txt
11-print_h.t.txt
Auxiliary.pm
Capture.pm
MANIFEST.patch.txt
ops2pm.pl.patch.txt
root.in.patch.txt
Utils.pm

James E Keenan

unread,
Feb 19, 2007, 1:39:41 PM2/19/07
to perl6-i...@perl.org, bugs-bi...@rt.perl.org
James Keenan wrote:

> # New Ticket Created by James Keenan
> # Please include the string: [perl #41455]
> # in the subject line of all future correspondence about this issue.
> # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=41455 >
>
>

Following discussion with particle today, these patches and new files
were committed to trunk in r17061. They have been tested on Darwin and
Win32.

kid51

James E Keenan

unread,
Feb 19, 2007, 2:25:47 PM2/19/07
to perl6-i...@perl.org, bugs-bi...@rt.perl.org
James E Keenan wrote:

And the patches have passed* on Linux (in attached configuration) as well.

*passed => perl Configure.pl and make worked flawlessly; make test
failed the one test we know is failing in t/perl/Parrot_Distribution.t

kid51

myconfig.thenceforward

James Keenan via RT

unread,
Feb 23, 2007, 9:10:37 PM2/23/07
to perl6-i...@perl.org
Following discussion with particle on Feb 19 2007, these patches and new files were committed
to trunk in r17061. Ticket may be closed.

James Keenan via RT

unread,
Mar 5, 2007, 6:42:24 AM3/5/07
to perl6-i...@perl.org
[Same note as in RT 41688]

I am pulling this patch from submission for the time being. When I
originally submitted the refactored tools/build/ops2c.pl and
lib/Parrot/Ops2c/Utils.pm over a week ago, they were passing their own
tests as well as those in 'make test'.

However, something outside these patches must have changed in that time,
because when I went to perform 'make test' last night, I got this
strange error:

t/pmc/object-mro.........................
# Failed test (t/pmc/object-mro.t at line 243)
# got: 'Class Object already registered!
# current instr.: 'main' pc 0
(/Users/jimk/work/bt/t/pmc/object-mro_5.pir:27)
# '
# expected: 'Vulcan Intelligent Sentient Humanoid BiPedal LifeForm
Object R
# '
# './parrot -D40 --gc-debug
"/Users/jimk/work/bt/t/pmc/object-mro_5.pir"' failed with exit code 1
# Looks like you failed 1 test of 6.
dubious
Test returned status 1 (wstat 256, 0x100)
DIED. FAILED test 5
Failed 1/6 tests, 83.33% okay

Since I never made any changes in the vicinity of object-mro, this is
going to take some time to debug. So, for now, I am withdrawing the patch.

Reply all
Reply to author
Forward
0 new messages