ingydotnet / inline-c-pm

10 stars 19 forks source link

t/09parser.t fails with Inline-0.83: Failed test 'Match number of lines in log' #88

Closed ppisar closed 5 years ago

ppisar commented 5 years ago

After upgrading Inline from 0.82 to 0.83, Inline-C t/09parser.t fails like this:

ok 21 - Returned product
not ok 22 - Match number of lines in log
#   Failed test 'Match number of lines in log'
#   at /home/test/fedora/perl-Inline-C/Inline-C-0.80/t/09parser.t line 180.
#          got: '13'
#     expected: '21'

This happens since Inline commit:

commit 13fd62a675c9de3c1fdb734898a689155647c5b1 (HEAD, origin/config-accum)
Author: Ed J <mohawk2@users.noreply.github.com>
Date:   Tue Apr 2 18:37:12 2019 +0100

    merge per-language config, not overwrite
ppisar commented 5 years ago

The test counts lines in $TestInlineSetup::DIR/parser_id log and this the difference of the log content:

--- /tmp/good   2019-04-30 12:41:05.053000000 +0200
+++ /tmp/bad    2019-04-30 12:38:30.994000000 +0200
@@ -1,18 +1,10 @@
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
-Inline::C::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
 Inline::C::Parser::RecDescent::get_parser called
 Inline::C::get_parser called

Either the test should be removed or adjusted based on the Inline version.

wbraswell commented 5 years ago

@ingydotnet @sisyphus @perlpunk This is a critical error, causing widespread installation failure:

https://travis-ci.org/wbraswell/rperl/builds/526940831 https://api.travis-ci.org/v3/job/526940833/log.txt

http://www.cpantesters.org/cpan/report/f7081960-69c8-11e9-bd7c-299ba5ac7024

https://github.com/wbraswell/rperl/pull/99

wbraswell commented 5 years ago

@mohawk2 may also be able to shed some light on this new issue?

sisyphus commented 5 years ago

Just a note on the original purpose of t/09Parser.t. It was introduced (by me) in Inline-0.47 in response (I think) to the fix for https://rt.cpan.org/Public/Bug/Display.html?id=49669. I wanted to be sure that I hadn't stuffed up Inline::C's capacity to use different parsers inside different packages. I don't know why anyone would want to use different parsers in different packages, but it was a (documented ?) behaviour that I didn't want to inadvertently alter.

So, about all it originally did was to check that each package used the specified parser.

AFAIK those tests have never failed, until now. Those 09Parser tests always took more time than I would have liked .... they seem to take even longer these days. I guess they could probably be trimmed back a bit, but at the time it was no big deal.

FAIRK, some intelligent clear-thinking person with a good understanding of perl might even be able to deduce that they are only testing something that is going to inevitably pass - and that t/09Parser.t is entirely pointless. Or maybe it's just so unlikely and/or of such little consequence that it's not worth persevering with. I am (obviously) unable to make that judgement - but I would encourage the removal of 09Parser.t if such a thing is deemed pertinent.

(I'm assuming that Inline::C's behaviour has not changed in this regard and that the failure is in the construction of the 09Parser.t tests.)

Cheers, Rob

On Fri, May 3, 2019 at 4:15 AM William N. Braswell, Jr. < notifications@github.com> wrote:

@mohawk2 https://github.com/mohawk2 may also be able to shed some light on this new issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/88#issuecomment-488776315, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAR3PEWIVTUMJ2N52RTEWLPTMVSTANCNFSM4HJKMPBA .

wbraswell commented 5 years ago

@sisyphus Can it be due to the numerous recent changes in PPI?

mohawk2 commented 5 years ago

@wbraswell You could try to repro the failure on your system, then install different PPI versions and report back.

Otherwise, my suggested strategy is at https://github.com/ingydotnet/inline-c-pm/pull/89#issuecomment-489141099

wbraswell commented 5 years ago

@mohawk2 Yes I have already tried many different PPI versions, but I have found no solutions myself at this time. And yes, I agree we should stop the failing tests immediately before making a final decision.

perlpunk commented 5 years ago

Waiting until PAUSE is working again to make a release...

perlpunk commented 5 years ago

Released 0.80_001

wbraswell commented 5 years ago

@perlpunk So far, so good! We are now successfully installing Inline::C 0.80_001 and passing all RPerl tests. :-) https://travis-ci.org/wbraswell/rperl/builds/527176073

Now if only CPAN Testers were working properly, no results yet, even in fast matrix... :-/ http://matrix.cpantesters.org/?dist=Inline-C+0.80_001 http://fast-matrix.cpantesters.org/?dist=Inline-C%200.80_001

So I guess we will have to wait until CPAN Testers starts working before we can make a full CPAN release.

perlpunk commented 5 years ago

We're getting Argument "0.82_001" isn't numeric in numeric lt (<) at t/09parser.t line 181, for example in http://www.cpantesters.org/cpan/report/afa6c32c-6f23-11e9-8b1c-ff12ac00720b I removed that condition from the test and just required Inline 0.83.

Inline::C 0.81 released.