action #44165: [functional][y] Use more static code style checks - try to use https://metacpan.org/pod/Perl::Critic::Policy::ControlStructures::ProhibitDeepNests - openQA Tests - openSUSE Project Management Tool

Custom queries

All open Feature tests
openQA Infrastructure Project
openqa-review - Closed tickets last updated by openqa-review, last 30 days
QA roadmap long-term
QA SLE functional
QA SLE Functional - closed in last 14 days
QA SLE Functional - High, need to be refined
QA SLE Functional - over cycle time median
QA SLE u
QA SLE y
QA tools (tag not necessary in openQA and subprojects)
QA tools tag (tag not necessary in openQA and subprojects; excluding tickets in "Ready" version as they are already on the backlog)
QAC - Backlog
QAM
QE tools team - backlog (ready issues)
QE tools team - backlog (w/o infra)
QE tools team - backlog SLA high
QE tools team - backlog SLA immediate
QE tools team - backlog SLA no immediate/urgent in feedback/blocked
QE tools team - backlog SLA normal
QE tools team - backlog SLA urgent
QE tools team - backlog SLO high
QE tools team - backlog SLO normal
QE tools team - backlog SLO urgent
QE tools team - backlog, high-level view (epics and higher)
QE tools team - backlog, non-reactive work, needs parent
QE tools team - backlog, top-level view (all sagas)
QE tools team - closed within last 14 days
QE tools team - closed within last 60 days
QE tools team - closed yesterday
QE Tools Team - Collaborative Session
QE tools team - due date forecast
QE tools team - exceeding due-date
QE tools team - infrastructure backlog
QE tools team - next - sorted by update time
QE tools team - next issues
QE tools team - non-estimated (unblocked) issues (w/o infra)
QE tools team - ready issues - Workable
QE tools team - ready, not assigned/blocked/low
QE tools team - update forecast
QE tools team - updated by priority
QE tools team - what members of the team are working on - Feedback (not-low)
QE Tools Team Backlog By Assignee
SLE15 Migration Open Tickets
SLE15 SP1 Migration Open Tickets
SLE15SP3 Migration open ticket
SLE15SP3 Security open ticket
Tools Team Retrospective
Tools Team Retrospective (not estimated or assigned)

Actions

action #44165

closed

coordination #44075: [functional][y][epic] Use more static code style checks to make life for test developers such a pain that nobody dares to add new tests which we would not be able to maintain anyway ;)

[functional][y] Use more static code style checks - try to use https://metacpan.org/pod/Perl::Critic::Policy::ControlStructures::ProhibitDeepNests

Added by okurz over 5 years ago. Updated over 5 years ago.

Status:

Resolved

Priority:

Low

Assignee:

JERiveraMoya

Category:

Enhancement to existing tests

Target version:

SUSE QA - Milestone 21

Start date:

2018-11-21

Due date:

2018-12-04

% Done:

Estimated time:

5.00 h

Difficulty:

Description

Acceptance criteria¶

AC1: The nesting level is tested as part of the normal tests
AC2: Tests accept the current state of the test code and ensure no further degradation of quality

Suggestions¶

Add the policy to https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/Makefile#L89
Call it, try it out and see where it fails
Tweak the limit to whatever currently works
Optional: Fix the low-hanging fruits

History
Notes
Property changes

Actions

Copy link

Updated by okurz over 5 years ago

Copied from action #44159: [functional][y][timeboxed:4h] Use more static code style checks - evaluate what perl critic policies make sense added

Actions

Copy link

Updated by okurz over 5 years ago

Copied from deleted (action #44159: [functional][y][timeboxed:4h] Use more static code style checks - evaluate what perl critic policies make sense)

Actions

Copy link

Updated by okurz over 5 years ago

Parent task set to #44075

Actions

Copy link

Updated by riafarov over 5 years ago

Status changed from New to Workable
Priority changed from Normal to Low
Estimated time set to 5.00 h

Actions

Copy link

Updated by okurz over 5 years ago

Description updated (diff)

Actions

Copy link

Updated by JERiveraMoya over 5 years ago

Assignee set to JERiveraMoya

Actions

Copy link

Updated by JERiveraMoya over 5 years ago

In my local system was required to install:
perl-Selenium-Remote-Driver (to run succesfully make test)
perl-Perl-Critic (to run succesfully make perlcritic)
Las command returned:

PERL5LIB=tools/lib/perlcritic:$PERL5LIB perlcritic --quiet --gentle --include Perl::Critic::Policy::HashKeyQuote --include Perl::Critic::Policy::ConsistentQuoteLikeWords .
lib/apachetest.pm: Variable declared in conditional statement at line 67, column 9.  Declare variables outside of the condition.  (Severity: 5)
lib/hacluster.pm: Variable declared in conditional statement at line 217, column 5.  Declare variables outside of the condition.  (Severity: 5)
lib/ipmi_backend_utils.pm: qw should be used as function at line 20, column 19.  use qw(A B).  (Severity: 4)
lib/kdump_utils.pm: qw should be used as function at line 16, column 16.  use MODULE 'func' for single imports.  (Severity: 4)
lib/main_common.pm: Expression form of "eval" at line 262, column 5.  See page 161 of PBP.  (Severity: 5)
lib/main_common.pm: Expression form of "eval" at line 265, column 9.  See page 161 of PBP.  (Severity: 5)
lib/main_common.pm: "return" statement with explicit "undef" at line 333, column 5.  See page 199 of PBP.  (Severity: 5)
lib/main_ltp.pm: qw should be used as function at line 23, column 19.  use MODULE 'func' for single imports.  (Severity: 4)
lib/main_ltp.pm: Two-argument "open" used at line 43, column 5.  See page 207 of PBP.  (Severity: 5)
lib/main_ltp.pm: Two-argument "open" used at line 57, column 5.  See page 207 of PBP.  (Severity: 5)
lib/migration.pm: qw should be used as function at line 26, column 9.  use qw(A B).  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 545, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 546, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 547, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 548, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 549, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 550, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 551, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 552, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 553, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 554, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 555, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 556, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 557, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 558, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 559, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 560, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 561, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 562, column 9.  Avoid useless quotes.  (Severity: 4)
lib/registration.pm: Hash key with quotes at line 563, column 9.  Avoid useless quotes.  (Severity: 4)
lib/repo_tools.pm: qw should be used as function at line 24, column 15.  use qw(A B).  (Severity: 4)
lib/selenium.pm: qw should be used as function at line 38, column 22.  use qw(A B).  (Severity: 4)
lib/selenium.pm: Hash key with quotes at line 194, column 49.  Avoid useless quotes.  (Severity: 4)
lib/selenium.pm: Hash key with quotes at line 195, column 49.  Avoid useless quotes.  (Severity: 4)
lib/suseconnect_register.pm: qw should be used as function at line 24, column 15.  use qw(A B).  (Severity: 4)
lib/suseconnect_register.pm: Variable declared in conditional statement at line 55, column 9.  Declare variables outside of the condition.  (Severity: 5)
lib/suseconnect_register.pm: Variable declared in conditional statement at line 56, column 9.  Declare variables outside of the condition.  (Severity: 5)
lib/susedistribution.pm: Subroutine prototypes used at line 291, column 1.  See page 194 of PBP.  (Severity: 5)
lib/wickedbase.pm: Code before strictures are enabled at line 34, column 1.  See page 429 of PBP.  (Severity: 5)
lib/y2logsstep.pm: Hash key with quotes at line 284, column 23.  Avoid useless quotes.  (Severity: 4)
lib/y2logsstep.pm: Hash key with quotes at line 284, column 42.  Avoid useless quotes.  (Severity: 4)
lib/y2logsstep.pm: Hash key with quotes at line 284, column 59.  Avoid useless quotes.  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 18, column 71.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 19, column 72.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 22, column 69.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 25, column 10.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 30, column 10.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 55, column 37.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 56, column 37.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 57, column 44.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 60, column 36.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 74, column 36.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 75, column 36.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 76, column 43.  use qw(A B).  (Severity: 4)
t/01_version_utils.t: qw should be used as function at line 79, column 35.  use qw(A B).  (Severity: 4)
lib/publiccloud/basetest.pm: Code before strictures are enabled at line 21, column 1.  See page 429 of PBP.  (Severity: 5)
products/sle/main.pm: Hash key with quotes at line 226, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 227, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 228, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 232, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 233, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 234, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 238, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 239, column 9.  Avoid useless quotes.  (Severity: 4)
products/sle/main.pm: Hash key with quotes at line 240, column 9.  Avoid useless quotes.  (Severity: 4)
[Makefile:93: perlcritic] Error 2

I cannot see in the log that we are running in our ci (Travis) this part of perlcritic and there are still error without adding any new checking or excluding two current policies:

PERL5LIB=tools/lib/perlcritic:$PERL5LIB perlcritic --quiet --gentle .
lib/apachetest.pm: Variable declared in conditional statement at line 67, column 9.  Declare variables outside of the condition.  (Severity: 5)
lib/hacluster.pm: Variable declared in conditional statement at line 217, column 5.  Declare variables outside of the condition.  (Severity: 5)
lib/main_common.pm: Expression form of "eval" at line 262, column 5.  See page 161 of PBP.  (Severity: 5)
lib/main_common.pm: Expression form of "eval" at line 265, column 9.  See page 161 of PBP.  (Severity: 5)
lib/main_common.pm: "return" statement with explicit "undef" at line 333, column 5.  See page 199 of PBP.  (Severity: 5)
lib/main_ltp.pm: Two-argument "open" used at line 43, column 5.  See page 207 of PBP.  (Severity: 5)
lib/main_ltp.pm: Two-argument "open" used at line 57, column 5.  See page 207 of PBP.  (Severity: 5)
lib/suseconnect_register.pm: Variable declared in conditional statement at line 55, column 9.  Declare variables outside of the condition.  (Severity: 5)
lib/suseconnect_register.pm: Variable declared in conditional statement at line 56, column 9.  Declare variables outside of the condition.  (Severity: 5)
lib/susedistribution.pm: Subroutine prototypes used at line 291, column 1.  See page 194 of PBP.  (Severity: 5)
lib/wickedbase.pm: Code before strictures are enabled at line 34, column 1.  See page 429 of PBP.  (Severity: 5)
lib/publiccloud/basetest.pm: Code before strictures are enabled at line 21, column 1.  See page 429 of PBP.  (Severity: 5)
make: *** [Makefile:93: perlcritic] Error 2

Actions

Copy link

Updated by JERiveraMoya over 5 years ago

Status changed from Workable to In Progress

I will try to address the previous one and see if we can remove those 5 (as the policy is already at the minimum).

Actions

Copy link

Updated by okurz over 5 years ago

Yes, this is a bit strange. Maybe you are using a different version or the perl critic changes in our tests actually do not work at all as expected or there is an additional config file which isn not read.

Actions

Copy link

#10