perl-critic/test-perl-critic

Run Perl::Critic as a unit test

test-perl-critic's Issues

Issues link in POD is wrong

it points to RT, should point to https://github.com/Perl-Critic/Test-Perl-Critic/issues

Wishlist: better use of PPI::Cache

In some situations in testing, if a file has a valid PPI::Cache entry, it should not be necessary to run the tests on that file again. What would be required to add this feature (I'd probably get around it it with some guidance).

should require newer version of MCE

Version 1.827 has fixes for running under taint mode. As the error appears in the END block, it may lead to errors even if tests if skipped after loading Test::Perl::Critic. See for ex. http://www.cpantesters.org/cpan/report/606aa06d-6c1b-1014-b32f-3adcd52a00bf

[libtest-perl-critic-perl] lintian FTBFS with new version

We have the following bug reported to the Debian package of
Test-Perl-Critic (https://bugs.debian.org/795716):

It doesn't seem to be a bug in the packaging, so you may want to take
a look. Thanks!

(Summarising the relevant part from the bug log)

------8<-----------8<-----------8<-----------8<-----------8<-----

> > Package: libtest-perl-critic-perl
> > Version: 1.03-1
> > Severity: serious
> > control: affects -1 lintian
> > 
> > Sid version FTBFS whereas testing work fine see
> > https://jenkins.debian.net/job/lintian-tests_sid/

The symptom is:

"""

> $ perl -Ilib  t/scripts/01-critic/docs-examples.t
> 1..1
>     # Subtest: Critic all code in doc/examples/checks
>     ok - Test::Perl::Critic for "doc/examples/checks/my-vendor/another-check.pm"
>     ok - Test::Perl::Critic for "doc/examples/checks/my-vendor/some-check.pm"
>     1..2
> not ok 1 - No tests run for subtest "Critic all code in doc/examples/checks"
> # Failed test 'No tests run for subtest "Critic all code in doc/examples/checks"'
> # at t/scripts/01-critic/docs-examples.t line 47.
> # Looks like you failed 1 test of 1.
> 
> """

We run the all_critic_ok in a subtest like so:

"""
            # all_critic_ok emits its own plan, so run it in a subtest
            # so we can just count it as "one" test.
            subtest "Critic all code in $arg" => sub {
                all_critic_ok($arg);
            };
"""

This seems to be broken by upstream, which is now running all the
"critic_ok" tests in a subprocess (via mce_grep).  This means that
all_critic_ok in total do 0 tests!

------8<-----------8<-----------8<-----------8<-----------8<-----

Thanks for considering,
gregor herrmann,
Debian Perl Group

Support external perlcritic command?

At my workplace we have a special Perl package installation and usage mechanism that, we install CPAN packages to non-standard paths and we need to explicitly specify the paths of all the dependency packages (like by PERL5LIB or @INC) before using them. So when we develop/build/test our internal Perl library, the exact list of its dependencies are decided. This causes an annoying problem with Test::Perl::Critic that, if we use Test::Perl::Critic in our tests, we would need to add its dependencies, most of which are indirectly introduced by Perl::Critic, into the list. If there be the option to use an external perlcritic command, we would be able to decouple the non-trivial list of dependencies of Perl::Critic from that of our own libraries, and have our re-engineered perlcritic command to handle its own dependencies.

I know it sounds like a non-standard requirement. But I would like to bring it up for discussion here and hope it can be implemented in Test::Perl::Critic. In case you don't have interest, I will write a new module myself and put onto CPAN.

Test::Builder needs to be at least version 0.88

Now that Test::Perl::Critic calls $TEST->done_testing(), the version requirement of Test::Builder needs to be at least 0.88, where done_testing was introduced.

Fresh install fails to install PPI because of missing File::Remove dependency

A fresh install on a reasonably pristine Perl 5.22 system will fail because PPI requires File::Remove, but probably doesn't name it within its dependencies. While this is more a PPI issue, it does break Test::Perl::Critic, so I'm mentioning it here.

Please make the MCE dependency optional

Would it be possible to add the MCE prereq to 'recommends' instead of 'requires', and only make use of it when installed? This is to prevent dependency bloat on systems where it's not wanted.

Perl::Critic being instantiated for each file is very slow

Not sure if this is intended or not, but instantiating Perl::Critic for each file to be criticized makes critic_ok pretty slow.

Running perlcritic upon set of files is roughly 4 times faster than Test::Perl::Critic.

Would it be possible to initialize Perl::Critic just once for all files?

Thanks!

critic_ok() in xt/96_perlcritic.t doesn't work with MCE running

When MCE is installed, xt/96_perlcritic.t fails because no filenames get passed to critic_ok

$ prove -b xt/96_perlcritic.t 
[23:07:49] xt/96_perlcritic.t .. no file specified at xt/96_perlcritic.t line 15.
[23:07:49] xt/96_perlcritic.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
No subtests run 
[23:07:51]

wishlist: Option to allow excluding some files

Perhaps something like:

all_critic_ok({exclude_filename_pattern => qr/FOO/});
all_critic_ok({exclude_path_pattern => qr(lib/Bencher/ScenarioR/)}, "lib", "other-lib");

I generate source code in some of my distributions, and these source code can be unwieldy or large that they choke perlcritic. It'd be nice if Test::Perl::Critic provides a way to do this. Of course, I could also generate the filtered list of files myself before passing it to all_critic_ok() I guess.