Project

General

Profile

Feature #23235

Feature #20901: Run a shell linter on bash scripts and add unit tests

Run shellcheck linter on bash scripts automatically

Added by Leonardo Lai about 2 months ago. Updated 18 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
09/05/2019
Due date:
% Done:

90%

Estimated time:
Stakeholders:
Duration:

Description

Shellcheck is a GPLv3 tool that gives warnings and suggestions for bash/sh shell scripts.
It points out and clarifies typical syntax and semantic issues that could lead to errors, and also informs the developer about corner-cases and pitfalls that may cause failures under some circumstances.

https://github.com/koalaman/shellcheck

It would be nice to have this tool periodically running on GlideinWMS bash scripts, both for error checking and code quality improvements.
The plan is to add shellcheck to Jenkins CI, at least to monitor the number of warnings over time and get a rough estimate of the code quality.

About shellcheck

In shellcheck, every kind of warning is identified by a code (eg. SC1000). Most of them are relatively easy to resolve manually, however there are some tricky ones.
For instance, when a variable is sourced from another file, shellcheck wants to either know the filepath or at least be informed, otherwise a warning is thrown.
On the other hand, variables that are used by sourced scripts may still be flagged as unused.

The easiest way to bypass this pedantic behavior, which is kind of annoying, is to explicitly ignore warnings with certain codes using the '-e' option, at the cost of few potential false negatives.

For instance, some error codes that are troublesome are:
-SC1090 -> SC can't include sourced files from paths determined at runtime
-SC1091 -> SC can't access the sourced file
-SC2154 -> variable referenced but not assigned

In case of false positives, which rarely occur, the developer can add a shellcheck directive to the code, which applies to the next block and is something like:

# shellcheck disable=SC2086

History

#1 Updated by Leonardo Lai about 2 months ago

  • Status changed from New to Work in progress

I have added a new script run_shellcheck.sh in build/jenkins.
It runs shellcheck on all files in the current branch, producing a JSON report of the issues for each file. It also counts the total number of warnings.

Integration with Jenkins is yet to be done.

#2 Updated by Leonardo Lai 25 days ago

  • % Done changed from 0 to 90

ShellCheck is almost fully integrated with Jenkins.
It is run right after PyLint, and produces a primary log listing all the inspected files for each branch, along with the numbers of found issues per severity level.
Each file is accompanied by its own JSON log, which shows in detail the reason of the issues (if any) and their location.
The return status of each test (branch) depends on the nature of the issues:
- FAILED > at least one issue of level 'error'
- WARNING > at least one issue of level 'warning', but no errors
- SUCCESS > no errors or warnings. 'info' and 'style' are acceptable (still notified in the logs).

Things left to do:
  • publish the secondary logs: they are already moved to the desired folder (<test_id>/shellcheck/<branch_name>/), but for some reason they're not shown when clicking on 'shellcheck' in the jenkins web report.
  • Add a column to the HTML table, so to include shellcheck reports to daily emails.

#3 Updated by Leonardo Lai 25 days ago

The additions are only in the test platform at the moment.
The script run_shellcheck.sh is available in glideinwms branch v34/23235.
The CI platform workflow modifications are in test_workflow.cfg

#4 Updated by Marco Mambelli 18 days ago

  • Status changed from Work in progress to Resolved

Tested and merged

#5 Updated by Marco Mambelli 18 days ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF