[FAB-10849] Improve package dependency checks

This change enables the unit test script to run the linters to enable shared calculation of package dependencies. This change also adds the capability to filter for file types when considering a package to be changed and also includes a package's testdata directory for consideration. Change-Id: I52e2267d129a16e7070bd3a600a8916df1a2f2a4 Signed-off-by: Troy Ronda <[email protected]>
hyperledger · Jun 26, 2018 · 2a53231 · 2a53231
1 parent cb4ad13
commit 2a53231
Show file tree

Hide file tree

Showing 11 changed files with 168 additions and 56 deletions.
diff --git a/Makefile b/Makefile
@@ -197,6 +197,10 @@ license:
 lint: populate
 	@LINT_CHANGED_ONLY=true $(TEST_SCRIPTS_PATH)/check_lint.sh
 
+.PHONY: lint-integration-tests
+lint-integration-tests: populate
+	@LINT_CHANGED_ONLY=true $(TEST_SCRIPTS_PATH)/check_lint_tests.sh
+
 .PHONY: lint-all
 lint-all: populate
 	@$(TEST_SCRIPTS_PATH)/check_lint.sh
@@ -209,8 +213,8 @@ build-softhsm2-image:
 		-f $(FIXTURE_SOFTHSM2_PATH)/Dockerfile .
 
 .PHONY: unit-test
-unit-test: checks depend populate
-	@TEST_CHANGED_ONLY=true FABRIC_SDKGO_CODELEVEL=$(FABRIC_CODELEVEL_UNITTEST_TAG) FABRIC_SDKGO_CODELEVEL_VER=$(FABRIC_CODELEVEL_UNITTEST_VER) $(TEST_SCRIPTS_PATH)/unit.sh
+unit-test: license depend populate lint-integration-tests
+	@TEST_CHANGED_ONLY=true TEST_WITH_LINTER=true FABRIC_SDKGO_CODELEVEL=$(FABRIC_CODELEVEL_UNITTEST_TAG) FABRIC_SDKGO_CODELEVEL_VER=$(FABRIC_CODELEVEL_UNITTEST_VER) $(TEST_SCRIPTS_PATH)/unit.sh
 ifeq ($(FABRIC_SDK_DEPRECATED_UNITTEST),true)
 	@GO_TAGS="$(GO_TAGS) deprecated" TEST_CHANGED_ONLY=true GO_TESTFLAGS="$(GO_TESTFLAGS) -count=1" FABRIC_SDKGO_CODELEVEL=$(FABRIC_CODELEVEL_UNITTEST_TAG) FABRIC_SDKGO_CODELEVEL_VER=$(FABRIC_CODELEVEL_UNITTEST_VER) $(TEST_SCRIPTS_PATH)/unit.sh
 endif
@@ -219,8 +223,8 @@ endif
 unit-tests: unit-test
 
 .PHONY: unit-tests-pkcs11
-unit-tests-pkcs11: checks depend populate
-	@FABRIC_SDKGO_CODELEVEL=$(FABRIC_CODELEVEL_UNITTEST_TAG) FABRIC_SDKGO_CODELEVEL_VER=$(FABRIC_CODELEVEL_UNITTEST_VER) $(TEST_SCRIPTS_PATH)/unit-pkcs11.sh
+unit-tests-pkcs11: license depend populate
+	@TEST_CHANGED_ONLY=true TEST_WITH_LINTER=true FABRIC_SDKGO_CODELEVEL=$(FABRIC_CODELEVEL_UNITTEST_TAG) FABRIC_SDKGO_CODELEVEL_VER=$(FABRIC_CODELEVEL_UNITTEST_VER) $(TEST_SCRIPTS_PATH)/unit-pkcs11.sh
 
 .PHONY: integration-tests-stable
 integration-tests-stable: clean depend populate

diff --git a/test/scripts/check_lint.sh b/test/scripts/check_lint.sh
@@ -9,42 +9,30 @@
 set -e
 LINT_CHANGED_ONLY="${LINT_CHANGED_ONLY:-false}"
 GO_CMD="${GO_CMD:-go}"
-GOMETALINT_CMD="gometalinter"
 SCRIPT_DIR="$(dirname "$0")"
 
 REPO="github.com/hyperledger/fabric-sdk-go"
 
 echo "Running" $(basename "$0")
 
 source ${SCRIPT_DIR}/lib/find_packages.sh
+source ${SCRIPT_DIR}/lib/linter.sh
 
 # Find all packages that should be linted.
 declare -a PKG_SRC=(
-"./pkg"
-"./test"
+    "./pkg"
+    "./test"
 )
 findPackages
 
 # Reduce Linter checks to changed packages.
 if [ "$LINT_CHANGED_ONLY" = true ]; then
-    findChangedFiles
-    findChangedPackages
-    filterExcludedPackages
-    appendDepPackages
-    PKGS=(${DEP_PKGS[@]})
+    findChangedLinterPkgs
 fi
 
-packagesToDirs
-
-if [ ${#DIRS[@]} -eq 0 ]; then
-    echo "Skipping linter since no packages were changed"
+if [ ${#PKGS[@]} -eq 0 ]; then
+    echo "Skipping tests since no packages were changed"
     exit 0
 fi
 
-if [ "$LINT_CHANGED_ONLY" = true ]; then
-    echo "Changed directories to lint: ${DIRS[@]}"
-fi
-
-echo "Running metalinters..."
-$GOMETALINT_CMD --config=./gometalinter.json "${DIRS[@]}"
-echo "Metalinters finished successfully"
+runLinter
diff --git a/test/scripts/check_lint_tests.sh b/test/scripts/check_lint_tests.sh
@@ -0,0 +1,37 @@
+#!/bin/bash
+#
+# Copyright SecureKey Technologies Inc. All Rights Reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+# This script runs Go linting and vetting tools
+
+set -e
+LINT_CHANGED_ONLY="${LINT_CHANGED_ONLY:-false}"
+GO_CMD="${GO_CMD:-go}"
+SCRIPT_DIR="$(dirname "$0")"
+
+REPO="github.com/hyperledger/fabric-sdk-go"
+
+echo "Running" $(basename "$0")
+
+source ${SCRIPT_DIR}/lib/find_packages.sh
+source ${SCRIPT_DIR}/lib/linter.sh
+
+# Find all packages that should be linted.
+declare -a PKG_SRC=(
+    "./test"
+)
+findPackages
+
+# Reduce Linter checks to changed packages.
+if [ "$LINT_CHANGED_ONLY" = true ]; then
+    findChangedLinterPkgs
+fi
+
+if [ ${#PKGS[@]} -eq 0 ]; then
+    echo "Skipping tests since no packages were changed"
+    exit 0
+fi
+
+runLinter
diff --git a/test/scripts/expiredorderer.sh b/test/scripts/expiredorderer.sh
@@ -58,8 +58,8 @@ if [ "$TEST_CHANGED_ONLY" = true ]; then
     findChangedFiles
     cd ${PWD}
 
-    if [[ "${CHANGED_FILES[@]}" =~ "test/fixtures/" ]] || [[ "${CHANGED_FILES[@]}" =~ "test/metadata/" ]]; then
-        echo "Fixture or metadata changed - running all integration tests"
+    if [[ "${CHANGED_FILES[@]}" =~ ( |^)(test/fixtures/|test/metadata/|test/scripts/|Makefile( |$)) ]]; then
+        echo "Test scripts, fixtures or metadata changed - running all tests"
     else
         findChangedPackages
         filterExcludedPackages

diff --git a/test/scripts/expiredpeer.sh b/test/scripts/expiredpeer.sh
@@ -58,8 +58,8 @@ if [ "$TEST_CHANGED_ONLY" = true ]; then
     findChangedFiles
     cd ${PWD}
 
-    if [[ "${CHANGED_FILES[@]}" =~ "test/fixtures/" ]] || [[ "${CHANGED_FILES[@]}" =~ "test/metadata/" ]]; then
-        echo "Fixture or metadata changed - running all integration tests"
+    if [[ "${CHANGED_FILES[@]}" =~ ( |^)(test/fixtures/|test/metadata/|test/scripts/|Makefile( |$)) ]]; then
+        echo "Test scripts, fixtures or metadata changed - running all tests"
     else
         findChangedPackages
         filterExcludedPackages

diff --git a/test/scripts/integration.sh b/test/scripts/integration.sh
@@ -62,8 +62,8 @@ if [ "$TEST_CHANGED_ONLY" = true ]; then
     findChangedFiles
     cd ${PWD}
 
-    if [[ "${CHANGED_FILES[@]}" =~ "test/fixtures/" ]] || [[ "${CHANGED_FILES[@]}" =~ "test/metadata/" ]]; then
-        echo "Fixture or metadata changed - running all integration tests"
+    if [[ "${CHANGED_FILES[@]}" =~ ( |^)(test/fixtures/|test/metadata/|test/scripts/|Makefile( |$)) ]]; then
+        echo "Test scripts, fixtures or metadata changed - running all tests"
     else
         findChangedPackages
         filterExcludedPackages

diff --git a/test/scripts/lib/find_packages.sh b/test/scripts/lib/find_packages.sh
@@ -9,14 +9,22 @@ function findPackages {
     PKGS=()
     for i in "${PKG_SRC[@]}"
     do
-       PKGS+=($(${GO_CMD} list ${i}/... 2> /dev/null | tr '\n' ' '))
+       declare -a FOUND_PKGS=($(${GO_CMD} list ${i}/... 2> /dev/null | tr '\n' ' '))
+       for pkg in "${FOUND_PKGS[@]}"
+       do
+           if [[ ! "${pkg}" =~ $PKG_EXCLUDE ]]; then
+               PKGS+=("${pkg}")
+           fi
+       done
     done
 }
 
 function findChangedFiles {
     CHANGED_FILES=($(git diff --name-only --diff-filter=ACMRTUXB HEAD | tr '\n' ' '))
     declare REMOTE_REF=$(git log -1 --pretty=format:"%d" | grep '[(].*\/' | wc -l)
 
+    # TODO: handle untracked files
+
     # If CHANGED_FILES is empty then there is no working directory changes: fallback to last two commits.
     # Else if REMOTE_REF=0 then working copy commits are even with remote: only use the working copy changes.
     # Otherwise assume that the change is amending the previous commit: use both last two commit and working copy changes.
@@ -38,15 +46,22 @@ function findChangedPackages {
     CHANGED_PKGS=()
     for file in "${CHANGED_FILES[@]}"
     do
-        # TODO filter out non GO/YAML/JSON files
-        # TODO handle vendor
-
-        if [ "$file" != "" ]; then
-            DIR=`dirname $file`
-            if [ "$DIR" = "." ]; then
-                CHANGED_PKG+=("$REPO")
+        declare allowedExtensions="(go|yaml|json|tx|pem|block)"
+        declare fileExt=${file#*.}
+
+        if [ "${file}" != "" ] && [[ ${fileExt} =~ $allowedExtensions ]]; then
+            declare DIR=`dirname ${file}`
+
+            if [ "${DIR}" = "." ]; then
+                CHANGED_PKG+=("${REPO}")
+#            vendor is not currently included in the git repository
+#            also git list currently prints out packages including the vendor/ prefix.
+#            elif [[ "${DIR}" =~ ^vendor/(.*)$ ]]; then
+#                CHANGED_PKGS+=("${BASH_REMATCH[1]}")
+            elif [[ "${DIR}" =~ ^(.*)/testdata(.*)$ ]]; then
+                CHANGED_PKGS+=("${REPO}/${BASH_REMATCH[1]}")
             else
-                CHANGED_PKGS+=("$REPO/$DIR")
+                CHANGED_PKGS+=("${REPO}/${DIR}")
             fi
         fi
     done
@@ -71,23 +86,35 @@ function filterExcludedPackages {
 }
 
 function calcDepPackages {
-    echo "Calculating package dependencies ..."
+    printf "Calculating package dependencies ... (0%%)\r"
+    declare i=0
 
     for pkg in "${PKGS[@]}"
     do
+        declare progress=$((100 * ${i} / ${#PKGS[@]}))
+        i=$((${i} + 1))
+        if [ $((${progress} % 10)) -eq 0 ]; then
+            printf "Calculating package dependencies ... (${progress}%%)\r"
+        fi
+
         declare testImports=$(${GO_CMD} list -f '{{.TestImports}}' ${pkg} | tr -d '[]' | tr ' ' '\n' | \
             grep "^${REPO}" | \
             grep -v "^${REPO}/vendor/" | \
             grep -v "^${REPO}/internal/github.com/" | \
             grep -v "^${REPO}/third_party/github.com/" | \
+            sort -u | \
             tr '\n' ' ')
 
-        declare pkgDeps=$(${GO_CMD} list -f '{{.Deps}}' ${pkg} ${testImports} | tr -d '[]')
-
-        declare val=$(${GO_CMD} list ${testImports} ${pkgDeps} | tr '\n' ' ')
+        declare pkgDeps=$(${GO_CMD} list -f '{{.Deps}}' ${pkg} ${testImports} | tr -d '[]' | tr ' ' '\n' | \
+            grep "^${REPO}" | \
+            grep -v "^${REPO}/vendor/" | \
+            sort -u | \
+            tr '\n' ' ')
 
-        export PKGDEPS__${pkg//[-\.\/]/_}="${val}"
+        declare val=$(${GO_CMD} list ${testImports} ${pkgDeps} | sort -u | tr '\n' ' ')
+        eval "PKGDEPS__${pkg//[-\.\/]/_}=\"${val}\""
     done
+    printf "Calculating package dependencies ... (100%%)\n"
 }
 
 function appendDepPackages {

diff --git a/test/scripts/lib/linter.sh b/test/scripts/lib/linter.sh
@@ -0,0 +1,31 @@
+#!/bin/bash
+#
+# Copyright SecureKey Technologies Inc. All Rights Reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+
+GOMETALINT_CMD="gometalinter"
+
+function runLinter {
+    packagesToDirs
+
+    echo "Directories to lint: ${DIRS[@]}"
+
+    echo "Running metalinters..."
+    ${GOMETALINT_CMD} --config=./gometalinter.json "${DIRS[@]}"
+    echo "Metalinters finished successfully"
+}
+
+function findChangedLinterPkgs {
+    findChangedFiles
+
+    if [[ "${CHANGED_FILES[@]}" =~ ( |^)(test/fixtures/|test/metadata/|test/scripts/|Makefile( |$)) ]]; then
+        echo "Test scripts, fixtures or metadata changed - running all tests"
+    else
+        findChangedPackages
+        filterExcludedPackages
+        appendDepPackages
+        PKGS=(${DEP_PKGS[@]})
+    fi
+}
diff --git a/test/scripts/revoked.sh b/test/scripts/revoked.sh
@@ -58,8 +58,8 @@ if [ "$TEST_CHANGED_ONLY" = true ]; then
     findChangedFiles
     cd ${PWD}
 
-    if [[ "${CHANGED_FILES[@]}" =~ "test/fixtures/" ]] || [[ "${CHANGED_FILES[@]}" =~ "test/metadata/" ]]; then
-        echo "Fixture or metadata changed - running all integration tests"
+    if [[ "${CHANGED_FILES[@]}" =~ ( |^)(test/fixtures/|test/metadata/|test/scripts/|Makefile( |$)) ]]; then
+        echo "Test scripts, fixtures or metadata changed - running all tests"
     else
         findChangedPackages
         filterExcludedPackages

diff --git a/test/scripts/unit-pkcs11.sh b/test/scripts/unit-pkcs11.sh
@@ -9,6 +9,7 @@
 # GO_LDFLAGS: Flags are added to the go test command (example: -s).
 # TEST_CHANGED_ONLY: Boolean on whether to only run tests on changed packages.
 # TEST_RACE_CONDITIONS: Boolean on whether to test for race conditions.
+# TEST_WITH_LINTER: Boolean on whether to run linter prior to unit tests.
 # FABRIC_SDKGO_CODELEVEL_TAG: Go tag that represents the fabric code target
 # FABRIC_SDKGO_CODELEVEL_VER: Version that represents the fabric code target (primarily for fixture lookup)
 # FABRIC_CRYPTOCONFIG_VERSION: Version of cryptoconfig fixture to use
@@ -20,11 +21,13 @@ FABRIC_SDKGO_CODELEVEL_TAG="${FABRIC_SDKGO_CODELEVEL_TAG:-devstable}"
 FABRIC_CRYPTOCONFIG_VERSION="${FABRIC_CRYPTOCONFIG_VERSION:-v1}"
 TEST_CHANGED_ONLY="${TEST_CHANGED_ONLY:-false}"
 TEST_RACE_CONDITIONS="${TEST_RACE_CONDITIONS:-true}"
+TEST_WITH_LINTER="${TEST_WITH_LINTER:-false}"
 SCRIPT_DIR="$(dirname "$0")"
 
 REPO="github.com/hyperledger/fabric-sdk-go"
 
 source ${SCRIPT_DIR}/lib/find_packages.sh
+source ${SCRIPT_DIR}/lib/linter.sh
 
 echo "Running" $(basename "$0")
 
@@ -38,10 +41,14 @@ if [ "$TEST_CHANGED_ONLY" = true ]; then
     findChangedFiles
     cd ${PWD}
 
-    findChangedPackages
-    filterExcludedPackages
-    appendDepPackages
-    PKGS=(${DEP_PKGS[@]})
+    if [[ "${CHANGED_FILES[@]}" =~ ( |^)(test/fixtures/|test/metadata/|test/scripts/|Makefile( |$)) ]]; then
+        echo "Test scripts, fixtures or metadata changed - running all tests"
+    else
+        findChangedPackages
+        filterExcludedPackages
+        appendDepPackages
+        PKGS=(${DEP_PKGS[@]})
+    fi
 fi
 
 RACEFLAG=""
@@ -57,10 +64,14 @@ if [ "$TEST_RACE_CONDITIONS" = true ]; then
 fi
 
 if [ ${#PKGS[@]} -eq 0 ]; then
-    echo "Skipping PKCSS unit tests since no packages were changed"
+    echo "Skipping tests since no packages were changed"
     exit 0
 fi
 
+if [ "${TEST_WITH_LINTER}" = true ]; then
+    runLinter
+fi
+
 echo "Running PKCS11 unit tests (libltdl and softhsm required)..."
 
 # detect softhsm