Project

General

Profile

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:
Category:
Enhancement to existing tests
Target version:
SUSE QA - Milestone 21
Start date:
2018-11-21
Due date:
2018-12-04
% Done:

0%

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

Actions #1

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 #2

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 #3

Updated by okurz over 5 years ago

  • Parent task set to #44075
Actions #4

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 #5

Updated by okurz over 5 years ago

  • Description updated (diff)
Actions #6

Updated by JERiveraMoya over 5 years ago

  • Assignee set to JERiveraMoya
Actions #7

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 #8

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 #9

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 #11

Updated by JERiveraMoya over 5 years ago

  • Status changed from In Progress to Feedback
Actions #12

Updated by JERiveraMoya over 5 years ago

See also future possible additions in https://progress.opensuse.org/issues/44159#note-7

Actions #13

Updated by okurz over 5 years ago

it's great, just minor comments in the PR.

Actions #14

Updated by JERiveraMoya over 5 years ago

  • Status changed from Feedback to In Progress

Waiting until the end of the sprint to resolve it in case some user feedback.

Actions #15

Updated by JERiveraMoya over 5 years ago

  • Status changed from In Progress to Resolved

I was checking travis, and AFIK, no issues with PRs regarding this change.

Actions

Also available in: Atom PDF