test: add shellcheck to CI tests (GoogleCloudPlatform#469)

* test: add linter for shell files Also: * fix shellcheck errors in new tools/gcs2bq/run.sh * update contributing guide with local formatter / linter instructions * fix: lint errors with shell scripts * revert shellcheck image in cloudbuild.yaml * fix: format files in examples/ and tools/ subdirectories * fix: exclude node_modules from shellcheck * reduce width of check_format script
Khodaparastan · May 11, 2020 · 704af4c · 704af4c
1 parent e559027
commit 704af4c
Show file tree

Hide file tree

Showing 15 changed files with 256 additions and 179 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -13,16 +13,41 @@ To get started contributing:
 
    ```
    gcloud config set project YOUR-PROJECT
-   cd cloudbuild
-   terraform init
-   terraform apply -var="project_id=$(gcloud config get-value project)" -var='github_owner=GITHUB-USER-ID'
+   export GITHUB_USER=YOUR_GITHUB_USERNAME
+
+   pushd cloudbuild
+   teraform init
+   terraform apply -var="project_id=$(gcloud config get-value project)" -var="github_owner=${GITHUB_USER}"
+   popd
    ```
 
    Builds require a `make` container image in the same project. Build with
-   the following command:
+   the following commands:
 
    ```
+   pushd cloudbuild
    gcloud builds submit --tag gcr.io/$(gcloud config get-value project)/make .
+   popd
+   ```
+
+1. Run the formatter locally.
+
+   From the root of the repository, run the Docker command:
+   ```
+   docker run \
+     --mount type=bind,source="$( pwd )",target=/workspace \
+     --workdir=/workspace \
+     gcr.io/$(gcloud config get-value project)/make fmt
+   ```
+
+1. Run the linter locally.
+
+   From the root of the repository, run the Docker command:
+   ```
+   docker run \
+     --mount type=bind,source="$( pwd )",target=/workspace \
+     --workdir=/workspace \
+     gcr.io/$(gcloud config get-value project)/make test
    ```
 
 1. Develop using the following guidelines to help expedite your review:

diff --git a/cloudbuild.yaml b/cloudbuild.yaml
@@ -1,3 +1,4 @@
 steps:
 - name: 'gcr.io/$PROJECT_ID/make'
-  args: ['test']
+  args: ['test']
+  waitFor: ['-']
diff --git a/cloudbuild/Dockerfile b/cloudbuild/Dockerfile
@@ -31,5 +31,7 @@ RUN npm install gts
 RUN apt-get install -y default-jdk
 RUN wget https://github.com/google/google-java-format/releases/download/google-java-format-1.7/google-java-format-1.7-all-deps.jar --directory-prefix=/usr/share/java/
 
+# install bash linter
+RUN apt-get install -y shellcheck
 
 ENTRYPOINT ["make"]
diff --git a/examples/bigtable-change-key/scripts/copy_schema_to_new_table.sh b/examples/bigtable-change-key/scripts/copy_schema_to_new_table.sh
@@ -36,12 +36,12 @@ fi
 INPUT_TABLE=$1
 OUTPUT_TABLE=$2
 
-echo Creating table $OUTPUT_TABLE
-cbt createtable $OUTPUT_TABLE
+echo Creating table "$OUTPUT_TABLE"
+cbt createtable "$OUTPUT_TABLE"
 
-cbt ls $INPUT_TABLE | tail -n+3 | while read line
+cbt ls "$INPUT_TABLE" | tail -n+3 | while read -r line
 do
-  FAMILY=`echo $line | cut -d " " -f 1`
-  echo Adding column family $FAMILY
-  cbt createfamily $OUTPUT_TABLE $FAMILY
+  FAMILY=$( echo "$line" | cut -d " " -f 1 )
+  echo Adding column family "$FAMILY"
+  cbt createfamily "$OUTPUT_TABLE" "$FAMILY"
 done
diff --git a/examples/bigtable-change-key/scripts/create_sandbox_table.sh b/examples/bigtable-change-key/scripts/create_sandbox_table.sh
@@ -37,19 +37,19 @@ fi
 # Create table
 TABLE_NAME=$1
 
-cbt createtable $TABLE_NAME
-cbt createfamily $TABLE_NAME id
-cbt createfamily $TABLE_NAME loc
+cbt createtable "$TABLE_NAME"
+cbt createfamily "$TABLE_NAME" id
+cbt createfamily "$TABLE_NAME" loc
 
 # Populate table with dummy data (3 records)
-cbt set $TABLE_NAME e57c5e6a-609f-4371-86db-158304c2c1de#189 id:ride_id=e57c5e6a-609f-4371-86db-158304c2c1de
-cbt set $TABLE_NAME e57c5e6a-609f-4371-86db-158304c2c1de#189 id:point_idx=189
-cbt set $TABLE_NAME e57c5e6a-609f-4371-86db-158304c2c1de#189 loc:latitude=40.7854
+cbt set "$TABLE_NAME" e57c5e6a-609f-4371-86db-158304c2c1de#189 id:ride_id=e57c5e6a-609f-4371-86db-158304c2c1de
+cbt set "$TABLE_NAME" e57c5e6a-609f-4371-86db-158304c2c1de#189 id:point_idx=189
+cbt set "$TABLE_NAME" e57c5e6a-609f-4371-86db-158304c2c1de#189 loc:latitude=40.7854
 
-cbt set $TABLE_NAME 33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f#132 id:ride_id=33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f
-cbt set $TABLE_NAME 33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f#132 id:point_idx=132
-cbt set $TABLE_NAME 33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f#132 loc:latitude=41.7854
+cbt set "$TABLE_NAME" 33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f#132 id:ride_id=33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f
+cbt set "$TABLE_NAME" 33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f#132 id:point_idx=132
+cbt set "$TABLE_NAME" 33cb2a42-d9f5-4b64-9e8a-b5aa1d6e142f#132 loc:latitude=41.7854
 
-cbt set $TABLE_NAME 8fa1905e-422c-49f6-8ea3-23c579504d83#1003 id:ride_id=8fa1905e-422c-49f6-8ea3-23c579504d83
-cbt set $TABLE_NAME 8fa1905e-422c-49f6-8ea3-23c579504d83#1003 id:point_idx=1003
-cbt set $TABLE_NAME 8fa1905e-422c-49f6-8ea3-23c579504d83#1003 loc:latitude=11.7854
+cbt set "$TABLE_NAME" 8fa1905e-422c-49f6-8ea3-23c579504d83#1003 id:ride_id=8fa1905e-422c-49f6-8ea3-23c579504d83
+cbt set "$TABLE_NAME" 8fa1905e-422c-49f6-8ea3-23c579504d83#1003 id:point_idx=1003
+cbt set "$TABLE_NAME" 8fa1905e-422c-49f6-8ea3-23c579504d83#1003 loc:latitude=11.7854
diff --git a/examples/dataproc-gcs-connector/build_gcs_connector.sh b/examples/dataproc-gcs-connector/build_gcs_connector.sh
@@ -56,11 +56,11 @@ fi
 cd ..
 
 echo "Running Terraform to build Dataproc cluster"
-cd terraform || exit
-terraform init
-terraform apply -auto-approve
-
-cd ..
+(
+  cd terraform || exit
+  terraform init
+  terraform apply -auto-approve
+)
 
 echo "Running test script on Dataproc cluster"
 chmod u+x test_gcs_connector.sh

diff --git a/examples/dataproc-gcs-connector/connectors.sh b/examples/dataproc-gcs-connector/connectors.sh
@@ -50,7 +50,7 @@ is_worker() {
 }
 
 min_version() {
-  echo -e "$1\n$2" | sort -r -t'.' -n -k1,1 -k2,2 -k3,3 | tail -n1
+  echo -e "$1"'\n'"$2" | sort -r -t'.' -n -k1,1 -k2,2 -k3,3 | tail -n1
 }
 
 get_connector_url() {

diff --git a/helpers/check_format.sh b/helpers/check_format.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # Copyright 2019 Google LLC
 #
@@ -27,28 +27,57 @@
 # Exit with error code 1 - always
 need_formatting() {
     FOLDER=$1
-    FILES_TO_LINT=${@:2}
+    FILES_TO_LINT=${*:2}
     echo "Some files need to be formatted in $FOLDER - FAIL"
     echo "$FILES_TO_LINT"
     exit 1
 }
 
+# validate_bash - takes a folder path as input and shell checks files
+validate_bash() {
+    FOLDER=$1
+    echo "Validating $FOLDER - Checking bash files"
+
+    FILES_TO_CHECK=$(find "$FOLDER" -not \( -name node_modules -prune \) -type f -name "*.sh")
+
+    # Initialize FILES_TO_LINT to empty string
+    FILES_TO_LINT=""
+
+    if [[ ! -z "$FILES_TO_CHECK" ]]
+    then
+        for FILE_TO_CHECK in $FILES_TO_CHECK
+        do
+            if ! shellcheck "$FILE_TO_CHECK";
+            then
+                FILES_TO_LINT+="$FILE_TO_CHECK "
+            fi
+        done
+
+	if [[ ! -z "$FILES_TO_LINT"  ]]
+	then
+            need_formatting "$FOLDER" "$FILES_TO_LINT"
+        fi
+    else
+        echo "No bash files found for $FOLDER - SKIP"
+    fi
+}
+
 # validate_python - takes a folder path as input and validate python files
 # using yapf (supports both python2 and python3)
 # errors out if yapf --diff -r --style google returns a non-0 status
 validate_python() {
     FOLDER=$1
     echo "Validating $FOLDER - Checking python files"
 
-    FILES_TO_CHECK=$(find $FOLDER -type f -name "*.py")
+    FILES_TO_CHECK=$(find "$FOLDER" -type f -name "*.py")
 
     # Initialize FILES_TO_LINT to empty string
     FILES_TO_LINT=""
 
-    (cd $FOLDER && flake8 --exclude=.git,__pycache__,.venv,venv --select=E9,F,C)
-    if [ $? -ne 0 ]
+
+    if ! (cd "$FOLDER" && flake8 --exclude=.git,__pycache__,.venv,venv --select=E9,F,C)
     then
-       need_formatting $FOLDER
+       need_formatting "$FOLDER"
     fi
 
     if [[ ! -z "$FILES_TO_CHECK" ]]
@@ -58,27 +87,31 @@ validate_python() {
         echo "Testing formatting for python2 files in $FOLDER"
 
         # Getting the list of files to lint
-        YAPF_PYTHON2_OUTPUT=$(python2 -m yapf --diff -r --style google $FILES_TO_CHECK 2>&1)
-        YAPF_PYTHON2_STATUS=$(echo $?)
-        FILES_TO_LINT+=$( echo $YAPF_PYTHON2_OUTPUT | egrep '^---.*\(original\)$' | awk '{print $2}')
+        YAPF_PYTHON2_OUTPUT=$(python2 -m yapf --diff -r --style google "$FILES_TO_CHECK" 2>&1)
+        YAPF_PYTHON2_STATUS=$?
+        FILES_TO_LINT+=$( echo "$YAPF_PYTHON2_OUTPUT" | grep -E '^---.*\(original\)$' | awk '{print $2}')
 
         if [[ ! -z "$FILES_TO_LINT" ]]
         then
             # Error out with details
-            need_formatting $FOLDER $FILES_TO_LINT
+            need_formatting "$FOLDER" "$FILES_TO_LINT"
         fi
 
         # Checking python files if python2 failed (i.e not python2 compatible code)
         if [[ "$YAPF_PYTHON2_STATUS" -ne 0 ]]
         then
             # python 3 yapf
             echo "Testing formatting for python3 files in $FOLDER"
-            FILES_TO_LINT+=$(python3 -m yapf --diff -r --style google $FILES_TO_CHECK | egrep '^---.*\(original\)$' | awk '{print $2}')
+            FILES_TO_LINT+=$(
+                python3 -m yapf --diff -r --style google "$FILES_TO_CHECK" |
+                grep -E '^---.*\(original\)$' |
+                awk '{print $2}'
+            )
 
             if [[ ! -z "$FILES_TO_LINT" ]]
             then
                 # Error out with details
-                need_formatting $FOLDER $FILES_TO_LINT
+                need_formatting "$FOLDER" "$FILES_TO_LINT"
             fi
 
             if [[ -z "$FILES_TO_LINT" ]]
@@ -98,12 +131,12 @@ validate_go() {
     FOLDER=$1
     echo "Validating $FOLDER - Checking GO files"
 
-    FILES_TO_LINT=$(gofmt -l $FOLDER)
+    FILES_TO_LINT=$(gofmt -l "$FOLDER")
 
     if [[ ! -z "$FILES_TO_LINT" ]]
     then
         # Error out with details
-        need_formatting $FOLDER $FILES_TO_LINT
+        need_formatting "$FOLDER" "$FILES_TO_LINT"
     else
         echo "No go files need formatting for $FOLDER - SKIP"
     fi
@@ -114,61 +147,62 @@ validate_go() {
 # errors out if gts init or npm audit returns a non-0 status
 validate_typescript(){
     FOLDER=$1
-    if [[ -f "$FOLDER/tsconfig.json" ]]
+
+    FILES_TO_CHECK=$(find "$FOLDER" -type f -name "tsconfig.json")
+
+    if [[ ! -z "$FILES_TO_CHECK" ]]
     then
-        echo "Validating $FOLDER - Checking typescript files"
-        cd $FOLDER
-        npx gts -y init > /dev/null
+        for tsconfig_path in $FILES_TO_CHECK
+        do
+            tsconfig_dir=$(dirname "$tsconfig_path")
+            echo "Validating $tsconfig_dir - Checking typescript files"
+            cd "$tsconfig_dir" || exit 1
 
-        if [[ "$?" -eq 0  ]]
-        then
-            echo "Running npm audit..."
-            npm audit
-            cd -
-            if [[ "$?" -ne 0  ]]
+            if ! npx gts -y init > /dev/null ;
             then
-                echo "$FOLDER npm audit needs fixing - FAIL"
-                exit 1
+                echo "Running npm audit..."
+                if npm audit ;
+                then
+                    echo "$tsconfig_dir npm audit needs fixing - FAIL"
+                    exit 1
+                else
+                    echo "$tsconfig_dir npm audit is clean - PASS"
+                fi
             else
-                echo "$FOLDER npm audit is clean - PASS"
+                echo "gts init returned an error - FAIL"
+                exit 1
             fi
-        else
-            cd -
-            echo "gts init returned an error - FAIL"
-            exit 1
-        fi
+            cd - || exit 1
+        done
     fi
-
 }
 
-# validate_go - takes a folder path as input and validate folder
+# validate_java - takes a folder path as input and validate folder
 # using gts
 # errors out if gts init or npm audit returns a non-0 status
 validate_java(){
     FOLDER=$1
     echo "Validating $FOLDER - Checking java files"
 
-    FILES_TO_CHECK=$(find $FOLDER -type f -name "*.java")
+    FILES_TO_CHECK=$(find "$FOLDER" -type f -name "*.java")
 
     # Initialize FILES_TO_LINT to empty string
     FILES_TO_LINT=""
 
     if [[ ! -z "$FILES_TO_CHECK" ]]
     then
         echo "Testing formatting for java files in $FOLDER"
-        for FILE_TO_CHECK in $FILES_TO_CHECK
-        do
-            java -jar /usr/share/java/google-java-format-1.7-all-deps.jar --set-exit-if-changed $FILE_TO_CHECK > /dev/null
 
-            if [[ "$?" -ne 0 ]]
-            then
-                FILES_TO_LINT+="$FILE_TO_CHECK "
-            fi
-        done
+        # shellcheck disable=SC2086
+        FILES_TO_LINT=$(
+            java -jar "/usr/share/java/google-java-format-1.7-all-deps.jar" \
+                --set-exit-if-changed \
+                -n $FILES_TO_CHECK
+        )
 
         if [[ ! -z "$FILES_TO_LINT" ]]
         then
-            need_formatting $FOLDER $FILES_TO_LINT
+            need_formatting "$FOLDER" "$FILES_TO_LINT"
         fi
 
         if [[ -z "$FILES_TO_LINT" ]]
@@ -182,16 +216,23 @@ validate_java(){
 
 # temporary list of folders to exclude
 EXCLUDE_FOLDERS=$(cat helpers/exclusion_list.txt)
-
-for FOLDER in $(find tools examples -maxdepth 1 -mindepth 1 -type d);
+while IFS= read -r -d '' FOLDER
 do
-    if  [[ ! ${EXCLUDE_FOLDERS[@]} =~ "$FOLDER" ]]
+    if  [[ ! ${EXCLUDE_FOLDERS[*]} =~ $FOLDER ]]
     then
-        validate_python $FOLDER
-        validate_go $FOLDER
-        validate_typescript $FOLDER
-        validate_java $FOLDER
+        validate_bash "$FOLDER"
+        validate_go "$FOLDER"
+        validate_java "$FOLDER"
+        validate_python "$FOLDER"
+        validate_typescript "$FOLDER"
     else
         echo "$FOLDER in exclusion list - SKIP  "
     fi
-done
+# Search all directories and sub-directories except sub-directories of .git
+# https://stackoverflow.com/a/16595367/101923
+done < <(
+    find . -maxdepth 2 -mindepth 1 -type d \
+        -not \( -path ./.git -prune \) \
+        -print0
+)
+echo "finished checking format"