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/01-critic.t fails if Perl::Critic::Nits is installed #1

Closed
eserte opened this issue Aug 3, 2016 · 3 comments

Comments

@eserte
Copy link

@eserte eserte commented Aug 3, 2016

See subject. Test failure looks like this:

#   Failed test 'Critique for lib/Weasel/FindExpanders/Dojo.pm'
#   at t/01-critic.t line 16.
# Private Member Data shouldn't be accessed directly
# Private Member Data shouldn't be accessed directly
# Looks like you failed 1 test of 4.
t/01-critic.t .. 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/4 subtests 
@ehuelsmann

This comment has been minimized.

Copy link
Contributor

@ehuelsmann ehuelsmann commented Sep 14, 2018

The only line which could be causing this issue is https://github.com/perl-weasel/weasel-widgets-dojo/blob/master/lib/Weasel/FindExpanders/Dojo.pm#L110

And that line doesn't access private member data: it accesses the hash elements of the hashref held in $_, which is neither private (to any non-local module) nor member data (of an object). It's my opinion that this is a bug in Perl::Critic::Nits.

@ehuelsmann ehuelsmann closed this Sep 14, 2018
@ehuelsmann

This comment has been minimized.

Copy link
Contributor

@ehuelsmann ehuelsmann commented Sep 14, 2018

Looking to report this issue upstream to Perl::Critic::Nits, I see it has been reported 10 years ago (and not acted upon): https://rt.cpan.org/Ticket/Display.html?id=35440

I'd recommend not to use the Nits critique. It's broken and won't be fixed.

@eserte

This comment has been minimized.

Copy link
Author

@eserte eserte commented Nov 2, 2019

Unfortunately this issue is still causing test failures: http://matrix.cpantesters.org/?dist=Weasel-Widgets-Dojo%200.05
Worse, Weasel-Widgets-Dojo may show up in http://analysis.cpantesters.org/beforemaintrelease?pair=5.30.0:5.31.5 as a potential bleadperl victim.

I think it's best to make sure that Nits is not loaded within the critic test, or, if this is not easily possible, make the critic test an author-only test. (There are a lot of people in the CPAN community proposing the latter)

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