Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

t/05_utils.t started to fail (with newest PPI?) #858

Closed
eserte opened this issue Apr 25, 2019 · 7 comments

Comments

Projects
None yet
6 participants
@eserte
Copy link

commented Apr 25, 2019

The following test failure looks new:

#   Failed test 'parse_arg_list: foo( { bar() }, {}, 'blah' )'
#   at t/05_utils.t line 433.
#     Structures begin differing at:
#          $got->[0] = ARRAY(0x55b84e6440f8)
#     $expected->[0] = ' { bar() }'
# Looks like you failed 1 test of 156.
t/05_utils.t .................................. 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/156 subtests 

There are no pass reports with the recent PPI releases (@wchristian: FYI):

****************************************************************
Regression 'mod:PPI'
****************************************************************
Name           	       Theta	      StdErr	 T-stat
[0='const']    	      1.0000	      0.0684	  14.62
[1='eq_1.230'] 	     -0.0000	      0.0838	  -0.00
[2='eq_1.232'] 	     -0.0000	      0.0818	  -0.00
[3='eq_1.234'] 	     -0.1500	      0.0734	  -2.04
[4='eq_1.236'] 	     -0.0070	      0.0688	  -0.10
[5='eq_1.248'] 	     -1.0000	      0.1082	  -9.24
[6='eq_1.250'] 	     -1.0000	      0.0802	 -12.46

R^2= 0.683, N= 330, K= 7
****************************************************************
@preaction

This comment has been minimized.

Copy link

commented Apr 26, 2019

I think this was caused by this change to PPI: adamkennedy/PPI@7fd14b0

So, PPI now parses this correctly as a hashref constructor, which means the test needs to change. If I apply the patch below, the test passes, but I have a lot of other failing tests on my machine for some reason...

diff --git a/t/05_utils.t b/t/05_utils.t
index 4c642473..3df53794 100644
--- a/t/05_utils.t
+++ b/t/05_utils.t
@@ -418,7 +418,7 @@ sub test_parse_arg_list {
         [
                 q/foo( { bar() }, {}, 'blah' )/
             =>  [
-                    ' { bar() }',
+                    [ '{ bar() }' ],
                     [ qw< {} > ],
                     [ q<'blah'> ],
                 ],
@wchristian

This comment has been minimized.

Copy link

commented Apr 26, 2019

I've done a lot of releases yesterday, you might want to work your way forwards from https://metacpan.org/release/MITHALDU/PPI-1.238

@preaction

This comment has been minimized.

Copy link

commented Apr 26, 2019

A git bisect confirms: adamkennedy/PPI@7fd14b0 is the thing that changed to break this. I feel PPI is correctly parsing this, so this has to be fixed in Perl-Critic.

@shlomif

This comment has been minimized.

Copy link

commented Apr 30, 2019

Hi! Please fix this issue as it causes build failures such as https://travis-ci.org/shlomif/Dir-Manifest/jobs/526162733 .

@petdance

This comment has been minimized.

Copy link
Member

commented May 14, 2019

Fixed: 6e6ce95

@petdance petdance closed this May 14, 2019

petdance added a commit that referenced this issue May 14, 2019

@jkeenan

This comment has been minimized.

Copy link

commented May 23, 2019

Do you have an ETA for a CPAN release addressing this problem?

Today I went to install many 100s of CPAN modules against perl-5.30.0. The FAIL for Perl::Critic was the most prominent failure.

Thank you very much.
Jim Keenan

@petdance

This comment has been minimized.

Copy link
Member

commented May 23, 2019

I've got an open issue at #871 for the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.