Bash functions style cleanup

+ Update rst documentation for bash style
 + Command substitution:
  + Clarify when to use backticks.
  + Recommend avoiding nested command substitution.
  + Do not recommend putting command substitution results into quotes.
 + Function definition content:
  + Move "set -exuo pipefail" after comment only blocks.
  + Other set flags allowed for functions with good reasons.
+ Apply the new recommendations.
 - Blank lines unified in code but no written recommendation in rst.
+ Add missing references to functions called, variables read or set.
 + Add TODOs to where lists would be long.
+ Minor improvements to function descriptions.
+ Make "if" expressions more python-like.
+ Add missing "|| die" (or "|| true") where spotted.
+ Downgrade DEFAULT_NIC to a local variable.
+ Add TODO to list reasons for blacklisted tags.

Change-Id: I05dce030a8c2cb1b3a242d8b977e8fe150d8ee20
Signed-off-by: Vratko Polak <vrpolak@cisco.com>

1 files changed, 24 insertions, 25 deletions

diff --git a/resources/libraries/bash/function/per_patch.sh b/resources/libraries/bash/function/per_patch.sh
index 27fa0cfb06..61de6f7ade 100644
--- a/resources/libraries/bash/function/per_patch.sh
+++ b/resources/libraries/bash/function/per_patch.sh
@@ -15,13 +15,10 @@ set -exuo pipefail
 
 # This library defines functions used mainly by per patch entry scripts.
 # Generally, the functions assume "common.sh" library has been sourced already.
-
 # Keep functions ordered alphabetically, please.
 
 function archive_test_results () {
 
-    set -exuo pipefail
-
     # Arguments:
     # - ${1}: Directory to archive to. Required. Parent has to exist.
     # Variable set:
@@ -34,6 +31,8 @@ function archive_test_results () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory command failed."
     TARGET="$(readlink -f "$1")"
     mkdir -p "${TARGET}" || die "Directory creation failed."
@@ -47,8 +46,6 @@ function archive_test_results () {
 
 function archive_parse_test_results () {
 
-    set -exuo pipefail
-
     # Arguments:
     # - ${1}: Directory to archive to. Required. Parent has to exist.
     # Variables read:
@@ -58,6 +55,8 @@ function archive_parse_test_results () {
     # - archive_test_results - Archiving results.
     # - parse_bmrr_results - See definition in this file.
 
+    set -exuo pipefail
+
     archive_test_results "$1" || die
     parse_bmrr_results "${TARGET}" || {
         die "The function should have died on error."
@@ -67,8 +66,6 @@ function archive_parse_test_results () {
 
 function build_vpp_ubuntu_amd64 () {
 
-    set -exuo pipefail
-
     # This function is using Vagrant script to build VPP with all dependencies
     # that is ARCH/OS aware. VPP repo is SSOT for building mechanics and CSIT
     # is consuming artifacts. This way if VPP will introduce change in building
@@ -82,6 +79,8 @@ function build_vpp_ubuntu_amd64 () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory command failed."
     echo 'Building using "make build-root/vagrant/build.sh"'
     build-root/vagrant/"build.sh" || die "Vagrant VPP build script failed."
@@ -95,8 +94,6 @@ function build_vpp_ubuntu_amd64 () {
 
 function compare_test_results () {
 
-    set -exuo pipefail
-
     # Variables read:
     # - VPP_DIR - Path to directory with VPP git repo (at least built parts).
     # - ARCHIVE_DIR - Path to where robot result files are created in.
@@ -111,6 +108,8 @@ function compare_test_results () {
     # - 0 - If the comparison utility sees no regression (nor data error).
     # - 1 - If the comparison utility sees a regression (or data error).
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory operation failed."
     # Reusing CSIT main virtualenv.
     pip install -r "${PYTHON_SCRIPTS_DIR}/perpatch_requirements.txt" || {
@@ -123,8 +122,6 @@ function compare_test_results () {
 
 function download_builds () {
 
-    set -exuo pipefail
-
     # This is mostly useful only for Sandbox testing, to avoid recompilation.
     #
     # Arguments:
@@ -138,6 +135,8 @@ function download_builds () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory operation failed."
     dirs=("build-root" "build_parent" "build_current" "archive" "csit_current")
     rm -rf ${dirs[@]} || {
@@ -152,8 +151,6 @@ function download_builds () {
 
 function initialize_csit_dirs () {
 
-    set -exuo pipefail
-
     # This could be in prepare_test, but download_builds also needs this.
     #
     # Variables read:
@@ -164,6 +161,8 @@ function initialize_csit_dirs () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory operation failed."
     rm -rf "csit_current" "csit_parent" || {
         die "Directory deletion failed."
@@ -176,8 +175,6 @@ function initialize_csit_dirs () {
 
 function parse_bmrr_results () {
 
-    set -exuo pipefail
-
     # Currently "parsing" is just two greps.
     # TODO: Re-use PAL parsing code, make parsing more general and centralized.
     #
@@ -190,6 +187,8 @@ function parse_bmrr_results () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     rel_dir="$(readlink -e "${1}")" || die "Readlink failed."
     in_file="${rel_dir}/output.xml"
     out_file="${rel_dir}/results.txt"
@@ -206,8 +205,6 @@ function parse_bmrr_results () {
 
 function select_build () {
 
-    set -exuo pipefail
-
     # Arguments:
     # - ${1} - Path to directory to copy VPP artifacts from. Required.
     # Variables read:
@@ -220,6 +217,8 @@ function select_build () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory operation failed."
     source_dir="$(readlink -e "$1")"
     rm -rf "${DOWNLOAD_DIR}"/* || die "Cleanup of download dir failed."
@@ -231,8 +230,6 @@ function select_build () {
 
 function set_aside_commit_build_artifacts () {
 
-    set -exuo pipefail
-
     # Function is copying VPP built artifacts from actual checkout commit for
     # further use and clean git.
     # Variables read:
@@ -245,6 +242,8 @@ function set_aside_commit_build_artifacts () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory operation failed."
     rm -rf "build_current" || die "Remove operation failed."
     mkdir -p "build_current" || die "Directory creation failed."
@@ -262,8 +261,6 @@ function set_aside_commit_build_artifacts () {
 
 function set_aside_parent_build_artifacts () {
 
-    set -exuo pipefail
-
     # Function is copying VPP built artifacts from parent checkout commit for
     # further use. Checkout to parent is not part of this function.
     # Variables read:
@@ -275,6 +272,8 @@ function set_aside_parent_build_artifacts () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     cd "${VPP_DIR}" || die "Change directory operation failed."
     rm -rf "build_parent" || die "Remove failed."
     mkdir -p "build_parent" || die "Directory creation operation failed."
@@ -284,21 +283,19 @@ function set_aside_parent_build_artifacts () {
 
 function set_perpatch_dut () {
 
-    set -exuo pipefail
-
     # Variables set:
     # - DUT - CSIT test/ subdirectory containing suites to execute.
 
     # TODO: Detect DUT from job name, when we have more than just VPP perpatch.
 
+    set -exuo pipefail
+
     DUT="vpp"
 }
 
 
 function set_perpatch_vpp_dir () {
 
-    set -exuo pipefail
-
     # Variables read:
     # - CSIT_DIR - Path to existing root of local CSIT git repository.
     # Variables set:
@@ -306,6 +303,8 @@ function set_perpatch_vpp_dir () {
     # Functions called:
     # - die - Print to stderr and exit, defined in common.sh
 
+    set -exuo pipefail
+
     # In perpatch, CSIT is cloned inside VPP clone.
     VPP_DIR="$(readlink -e "${CSIT_DIR}/..")" || die "Readlink failed."
 }