t/tools/ops2pm/*.t: Failures in 3 test files

[jkeenan]

Running perl Configure.pl --test at r37676 this morning, I got the following failures in the build tools tests on two different OSes:

t/tools/ops2pm/09-prepare_real_ops.............1/? op load_language_s: sequence mismatch: ops.num 235 vs. core.ops 1245 at /Users/jimk/work/parrot/lib/Parrot/Ops2pm.pm line 248.
# Looks like you planned 38 tests but only ran 16.
# Looks like your test died just after 16.
t/tools/ops2pm/09-prepare_real_ops............. Dubious, test returned 255 (wstat 65280, 0xff00)
 Failed 22/38 subtests 
t/tools/ops2pm/10-print_module.................3/? op load_language_s: sequence mismatch: ops.num 235 vs. core.ops 1245 at /Users/jimk/work/parrot/lib/Parrot/Ops2pm.pm line 248.
# Looks like you planned 42 tests but only ran 16.
# Looks like your test died just after 16.
t/tools/ops2pm/10-print_module................. Dubious, test returned 255 (wstat 65280, 0xff00)
 Failed 26/42 subtests 
t/tools/ops2pm/11-print_h......................1/? op load_language_s: sequence mismatch: ops.num 235 vs. core.ops 1245 at /Users/jimk/work/parrot/lib/Parrot/Ops2pm.pm line 248.
# Looks like you planned 23 tests but only ran 16.
# Looks like your test died just after 16.
t/tools/ops2pm/11-print_h...................... Dubious, test returned 255 (wstat 65280, 0xff00)
 Failed 7/23 subtests 
t/pharness/01-default_tests....................ok     
t/pharness/02-get_test_prog_args...............ok     
t/pharness/03-handle_long_options..............ok     
t/pharness/04-Usage............................ok   

Test Summary Report
-------------------
t/tools/ops2pm/09-prepare_real_ops         (Wstat: 65280 Tests: 16 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 38 tests but ran 16.
t/tools/ops2pm/10-print_module             (Wstat: 65280 Tests: 16 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 42 tests but ran 16.
t/tools/ops2pm/11-print_h                  (Wstat: 65280 Tests: 16 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 23 tests but ran 16.
Files=37, Tests=941, 115 wallclock secs ( 1.19 usr  0.97 sys + 71.85 cusr 11.18 csys = 85.19 CPU)
Result: FAIL
Failed 3/37 test programs. 0/941 subtests failed.

Since at least the first failure reported occurred in a test where we were testing what happens when a limited selection of op codes are used, this may be fallout from r37601, in which we removed the --pmc option from Configure.pl. At that time, we adjusted the tests in t/tools/pmc2c.

But since I ran the build tools tests at that time and did not get these test failures, I can't be entirely sure. It may also be the case that recent removal of certain PMCs and op codes has not been accompanied by renumbering of ops and PMCs.

I don't think these failures are having an impact yet on the building of Parrot, but further investigation is needed.

Thank you very much.
kid51

[jkeenan]

Replying to jkeenan:

Binary search indicates that the test failures emerged overnight as a consequence of r37670.

Have to go to $job now. Allison, can you take a look?

Thank you very much.
kid51

[jkeenan]

Replying to jkeenan:

Replying to jkeenan: Binary search indicates that the test failures emerged overnight as a consequence of r37670. Have to go to $job now. Allison, can you take a look?

Perhaps make opsrenumber needs to be run.

[jkeenan]

Replying to jkeenan:

Replying to jkeenan:

Replying to jkeenan: Binary search indicates that the test failures emerged overnight as a consequence of r37670. Have to go to $job now. Allison, can you take a look?

Perhaps make opsrenumber needs to be run.

This should do it. Locally, having renumbered I'm now passing all tests in make test as well as make buildtools_tests. Can someone do this and look for any unexpected side effects?

Thank you very much.
kid51

[coke]

The post-1.0 world should involve renumbering ops as infrequently as possible. Adding new opcodes with the highest numbers seems reasonable to me.

I would recommend examining the failing tests to see what exactly they are testing.

[jkeenan]

Here is something I noticed in the course of working on this ticket.

Changeset 37670 includes the following:

Index: trunk/src/ops/core.ops
===================================================================
--- a/trunk/src/ops/core.ops
+++ b/trunk/src/ops/core.ops
@@ -165,4 +165,16 @@
     Parrot_load_bytecode(interp, $1);
 }
+
+=item B<load_language>(in STR)
+
+Load the compiler libraries for a language $1. Search the library path to
+locate the main compiler file in the standard locations.
+
+inline op load_language(in STR) :load_file {
+    Parrot_load_language(interp, $1);
+}
+
+
+=cut
 
 =back

But this puts these two lines inside the POD:

  inline op load_language(in STR) :load_file {
     Parrot_load_language(interp, $1);

... which means they're not active code. Shouldn't it have been the following?

+=item B<load_language>(in STR)
+
+Load the compiler libraries for a language $1. Search the library path to
+locate the main compiler file in the standard locations.
+
+=cut
+
+inline op load_language(in STR) :load_file {
+    Parrot_load_language(interp, $1);
+}
+

make and make test completed successfully either way. But I suspect only the latter is correct.

Thank you very much.
kid51

[jkeenan]

The failure occurs in the test of prepare_real_ops(). Here's the error message we get when we call prove -v t/tools/ops2pm/09-prepare_real_ops.t:

op load_language_s: sequence mismatch: ops.num 235 vs. core.ops 1245 at /home/jimk/work/parrot/lib/Parrot/Ops2pm.pm line 248.

Here's the method in which the failure occurred:

sub prepare_real_ops {
    my $self = shift;

    my $real_ops = Parrot::OpsFile->new( [], $self->{nolines} );
    $real_ops->{PREAMBLE} = $self->{ops}{PREAMBLE};
    $real_ops->version( $self->{ops}->version );

    # verify opcode numbers
    my $seq = 0;
    for my $el ( @{ $self->{ops}{OPS} } ) {
        next if $el->{CODE} < 0;    # skip
        my $opname = $el->full_name;
        my $n      = $self->{optable}{$opname};    # former global
        if ( $n != $el->{CODE} ) {
            die "op $opname: number mismatch: ops.num $n vs. core.ops $el->{CODE}";
        }
        if ( $seq != $el->{CODE} ) {
            die "op $opname: sequence mismatch: ops.num $seq vs. core.ops $el->{CODE}";  # <- line 248
        }
        push @{ $real_ops->{OPS} }, $el;
        ++$seq;
    }
    $self->{real_ops} = $real_ops;
}

Here are the parts of r37670 that triggered the test failure:

Index: trunk/src/ops/ops.num
===================================================================
--- a/trunk/src/ops/ops.num
+++ b/trunk/src/ops/ops.num
@@ -1267,2 +1267,4 @@
 find_sub_not_null_p_s          1243
 find_sub_not_null_p_sc         1244
+load_language_s                1245
+load_language_sc               1246

So two new opcodes were added. In the pre-1.0 world, we would have wanted to run make opsrenumber to renumber the opcodes such that they would have the same order as imposed by Parrot::Ops2pm:sort_ops() -- the method called immediately before prepare_real_ops(). This condition was enforced by this part of prepare_real_ops():

        if ( $seq != $el->{CODE} ) {
            die "op $opname: sequence mismatch: ops.num $seq vs. core.ops $el->{CODE}";
        }

I deleted this if block in r37816. In that revision, I also corrected the apparent error discussed in comments 4 and 5 of this ticket, thereby making the load_language opcodes available.

Thank you very much.
kid51

[jkeenan]

These tests continue to pass on the two OSes to which I have access, and there have been no other complaints. So I'm closing this ticket.

Thank you very much.
kid51

[coke]

Per allison, renumbering in a 1.0 world is just fine; You may want to re-add the block I had you remove here so the tests work as they did before.

Apologies.

[jkeenan]

Replying to coke:

Per allison, renumbering in a 1.0 world is just fine; You may want to re-add the block I had you remove here so the tests work as they did before. Apologies.

Well, before I do that, can you take a look at my latest post in a related ticket?

[jkeenan]

Replying to jkeenan:

Replying to coke:

I decided to just restore that code, as it goes into a method which is called in an entirely different context from that being tested in the other file.

Applied in r38110. Re-closing ticket.

Thank you very much.
kid51