You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@spamassassin.apache.org on 2022/06/01 08:35:49 UTC
[Bug 8002] New: Make perlcritic stricter to CPAN testing standards
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8002
Bug ID: 8002
Summary: Make perlcritic stricter to CPAN testing standards
Product: Spamassassin
Version: SVN Trunk (Latest Devel Version)
Hardware: All
OS: All
Status: NEW
Severity: enhancement
Priority: P2
Component: Regression Tests
Assignee: dev@spamassassin.apache.org
Reporter: sidney@sidney.com
Target Milestone: Undefined
The upload of 4.0.0-pre2 as a trial to CPAN is generating many errors on their
test machines because they use Perl::Critic with stricter policies than we use
in our perlcritic test. For example, they enable
Bangs::ProhibitBitwiseOperators warnings which trigger a test failure just for
using | or & properly.
CPAN's testing procedure is not something that we can change. What we can do is
to go through our code and comment every line that generates a perlcritic
warning after confirming that the usage is intended, for example,
..._flag | &AI_PASSIVE, ## no critic (Bangs::ProhibitBitwiseOperators)
This issue is for going through the perlcritic warnings that are found by
CPAN's testers and fixing any errors revealed or adding a ## no critic comment
It isn't being targeted for the 4.0 milestone because it may be too big a job
and does not fix any bugs or provide new functionality. If we do make enough
progress with it, it will be good to have so that CPAN test results are not
drowned out by the noise from perlcritic.
The first step will be to change the severity threshold in our perlcritic test
from 5 to 4, to match CPAN, and in the test disable all policies that reveals
in our code.
The next steps will be to go through the code dealing with one policy at a
time, such as Bangs::ProhibitBitwiseOperators, then remove the disabling of
that policy from our test.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 8002] Make perlcritic stricter to CPAN testing standards
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8002
Sidney Markowitz <si...@sidney.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|Undefined |4.0.0
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #3 from Sidney Markowitz <si...@sidney.com> ---
To test this properly, install the following CPAN modules:
Perl::Critic::Policy::Bangs::ProhibitBitwiseOperators
Perl::Critic::Policy::Perlsecret
Perl::Critic::Policy::Compatibility::ProhibitThreeArgumentOpen
Perl::Critic::Policy::Lax::ProhibitStringyEval::ExceptForRequire
Perl::Critic::Policy::ValuesAndExpressions::PreventSQLInjection
Perl::Critic::Policy::ControlStructures::ProhibitReturnInDoBlock
The only changes required were to exclude more warnings in the perlcritic test.
As with the ones that were already excluded, we should someday go through code
and check the warnings, adding ## no critic as we go.
trunk % svn ci -m "Bug 8002 - Exclude more PerlCritic policies that are checked
on CPAN test machines"
Sending t/perlcritic.pl
Transmitting file data .done
Committing transaction...
Committed revision 1901489.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 8002] Make perlcritic stricter to CPAN testing standards
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8002
--- Comment #2 from Sidney Markowitz <si...@sidney.com> ---
This is more complicated than I thought.
First, CPAN testers are not running perlcritic on their own. The failures are
in our own t/percritic.t test
Second, they are not enabling more policies than we are. The failures occur
running our own test with the same settings that we have.
The difference is that not all percritic policies are installed when one
installs Perl::Critic abd Test::Perl::Critic. I had to run
cpanm Perl::Critic::Policy::Bangs::ProhibitBitwiseOperators
to get the bitwiseoperator warnings to show up in t/perlcritic.t
So to get our test to get the same results as CPAN testing will find, we'll
have to install the policy modules that they have on their test machines.
The good news is that we can exclude the policies in t/perlcritic.t the same
way as the policies we already exclude and we don't have to sweep through all
the code right away.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 8002] Make perlcritic stricter to CPAN testing standards
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8002
--- Comment #5 from Sidney Markowitz <si...@sidney.com> ---
Found yet another Perl::Critic policy bundle module installed on a CPAN test
machine causing failures. This one is Perl::Critic::OTRS which isn't even
published on CPAN anymore. This time I'm excluding the entire set of policies
and not adding the module to MANIFEST. perlcritic.t will ignore the policies if
the test machine has installed them, but we will never want to remove the
exclude.
Again, committed to this closed issue because it has not yet been part of a
release.
trunk % svn ci -m "Bug 8002 - Exclude another set of PerlCritic policies found
on a CPAN test machine" t/perlcritic.pl
Sending t/perlcritic.pl
Transmitting file data .done
Committing transaction...
Committed revision 1903454.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 8002] Make perlcritic stricter to CPAN testing standards
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8002
Sidney Markowitz <si...@sidney.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |sidney@sidney.com
--- Comment #1 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Sidney Markowitz from comment #0)
>> The first step will be to change the severity threshold in our perlcritic
> test from 5 to 4, to match CPAN, and in the test disable all policies that
> reveals in our code.
Correction: CPAN does test at severity level 5. The reason they catch bitwise
operator warnings that we don't is that the warnings are in scripts such as
spamd which our test doesn't check. That makes this step unnecessary.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 8002] Make perlcritic stricter to CPAN testing standards
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8002
--- Comment #4 from Sidney Markowitz <si...@sidney.com> ---
Found another module installed on a CPAN test machine, which also issued a
warning about a module that it doesn't install that the test references.
Committed to this closed issue because it has not yet been part of a release.
trunk % svn ci -m "Bug 8002 - Exclude another PerlCritic policy found on a CPAN
test machine, add required modules for test" Makefile.PL t/perlcritic.pl
Sending Makefile.PL
Sending t/perlcritic.pl
Transmitting file data ..done
Committing transaction...
Committed revision 1903087.
--
You are receiving this mail because:
You are the assignee for the bug.