Add a 'preflight check' stage to CI (#923)

Motivation:

Often contributors miss out license checks, have formatting issues or
haven't generated linux tests. Let's bring these checks up front and
make sure they're done before we run any tests.

Modifications:

- Add a "Preflight Checks" stage to CI
- Add a sanity script which currently just checks license headers; we
  can add other checks later. Note that in CI this is called via
  .travis-script.sh as it sets up the environment (setting up PATH etc.t)
- Add building and caching SwiftFormat to the install step, it's not
  used yet but will be later.
- Fix a bunch of shellcheck issues
- Replaces some travis environment variables with flags

Result:

- We have a way to check non-code related things before checking
  functional changes

.travis-install.sh

@@ -38,16 +38,12 @@ PROTOBUF_VERSION=3.9.1
 BAZEL_VERSION=0.28.1
 GRPC_VERSION=1.23.0
 
-GREEN='\033[0;32m'
-BLUE='\033[0;34m'
-NO_COLOR='\033[0m'
-
 info() {
-  printf "${BLUE}$1${NO_COLOR}\n"
+  printf '\033[0;34m%s\033[0m\n' "$1"
 }
 
 success() {
-  printf "${GREEN}$1${NO_COLOR}\n"
+  printf '\033[0;32m%s\033[0m\n' "$1"
 }
 
 # Install the protoc compiler.
@@ -182,24 +178,74 @@ build_grpc_cpp_server() {
   echo -en 'travis_fold:end:install.grpc_cpp_server\\r'
 }
 
-main() {
-  cd
-  mkdir -p local "$BIN_CACHE"
-  mkdir -p local "$ZIP_CACHE"
+function install_swiftformat() {
+  echo -en 'travis_fold:start:install.swiftformat\\r'
+  info "Installing swiftformat"
 
-  install_protoc
-  install_swift
 
-  if [ "$RUN_INTEROP_TESTS" = "true" ]; then
-    install_bazel
-    build_grpc_cpp_server
+  if [ ! -f "$BIN_CACHE/swiftformat" ]; then
+    git clone --depth 1 "https://github.com/nicklockwood/SwiftFormat"
+
+    info "Building swiftformat"
+    cd SwiftFormat
+    swift build --product swiftformat
+
+    cp "$(swift build --show-bin-path)/swiftformat" "$BIN_CACHE/swiftformat"
+  else
+    info "Skipping download and build of SwiftFormat, using cached binaries"
   fi
 
-  # Verify installation
-  info "Contents of $HOME/local:"
-  find local
-  success "Install script completed"
+  # We should have cached swiftformat now, copy it to $HOME/local/bin
+  cp "$BIN_CACHE/swiftformat" "$HOME"/local/bin/swiftformat
+
+  success "Installed swiftformat"
+  echo -en 'travis_fold:end:install.swiftformat\\r'
 }
 
-# Run the installation.
-main
+cd
+mkdir -p local/bin "$BIN_CACHE" "$ZIP_CACHE"
+
+should_install_protoc=false
+should_install_interop_server=false
+should_install_swiftformat=false
+
+while getopts "pif" optname; do
+  case $optname in
+    p)
+      should_install_protoc=true
+      ;;
+    i)
+      should_install_interop_server=true
+      ;;
+    f)
+      should_install_swiftformat=true
+      ;;
+    \?)
+      echo "Uknown option $optname"
+      exit 2
+      ;;
+  esac
+done
+
+# Always install Swift.
+install_swift
+
+if $should_install_protoc; then
+  install_protoc
+fi
+
+if $should_install_interop_server; then
+  install_bazel
+  build_grpc_cpp_server
+fi
+
+if $should_install_swiftformat; then
+  # If we're building SwiftFormat we need to know where Swift lives.
+  export PATH=$HOME/local/bin:$PATH
+  install_swiftformat
+fi
+
+# Verify installation
+info "Contents of $HOME/local:"
+find "$HOME"/local
+success "Install script completed"

.travis-script.sh

@@ -17,16 +17,12 @@
 # See: Makefile
 BUILD_OUTPUT=./.build/debug
 
-GREEN='\033[0;32m'
-BLUE='\033[0;34m'
-NO_COLOR='\033[0m'
-
 info() {
-  printf "${BLUE}$1${NO_COLOR}\n"
+  printf '\033[0;34m%s\033[0m\n' "$1"
 }
 
 success() {
-  printf "${GREEN}$1${NO_COLOR}\n"
+  printf '\033[0;32m%s\033[0m\n' "$1"
 }
 
 setup_environment() {
@@ -45,14 +41,15 @@ make_all() {
 }
 
 make_test() {
+  local tsan=$1
   echo -en 'travis_fold:start:make.test\\r'
 
-  if [[ ${NO_TSAN} == "true" ]]; then
-    info "Running Swift tests"
-    make test
-  else
+  if $tsan; then
     info "Running Swift tests with TSAN"
     make test-tsan
+  else
+    info "Running Swift tests"
+    make test
   fi
 
   success "Swift tests passed"
@@ -168,21 +165,39 @@ run_interop_reconnect_test() {
   echo -en 'travis_fold:end:test.interop_reconnect\\r'
 }
 
-main() {
-  setup_environment
-
-  # If we're running interop tests don't bother with the other stuff.
-  if [ "$RUN_INTEROP_TESTS" = "true" ]; then
-    run_interop_tests
-    run_interop_reconnect_test
-  else
-    make_all
-    make_check_linuxmain
-    make_test
-    make_test_plugin
-    make_project
-  fi
-}
-
-# Run the thing!
-main
+just_sanity=false
+just_interop_tests=false
+tsan=false
+
+while getopts "sit" optname; do
+  case $optname in
+    s)
+      just_sanity=true
+      ;;
+    i)
+      just_interop_tests=true
+      ;;
+    t)
+      tsan=true
+      ;;
+    \?)
+      echo "Uknown option $optname"
+      exit 2
+      ;;
+  esac
+done
+
+setup_environment
+
+if $just_sanity; then
+  ./scripts/sanity.sh
+elif $just_interop_tests; then
+  run_interop_tests
+  run_interop_reconnect_test
+else
+  make_all
+  make_check_linuxmain
+  make_test $tsan
+  make_test_plugin
+  make_project
+fi

.travis.yml

@@ -18,20 +18,32 @@
 language: minimal
 jobs:
   include:
+    # Sanity checks for PRs.
+    - &sanity
+      stage: "Preflight Checks"
+      name: "Sanity (license headers and formatting)"
+      os: linux
+      dist: bionic
+      install: ./.travis-install.sh -f  # install swiftformat
+      script: ./.travis-script.sh -s  # just sanity
+      env: SWIFT_VERSION=5.2
     # Tests for each PR.
     - &tests
       stage: "Test"
       name: "Unit Tests: Ubuntu 18.04 (Swift 5.2)"
       os: linux
       dist: bionic
-      script: ./.travis-script.sh
-      env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.2
+      install: ./.travis-install.sh -p  # install protoc
+      script: ./.travis-script.sh -t  # with tsan
+      env: SWIFT_VERSION=5.2
     - <<: *tests
       name: "Unit Tests: Ubuntu 18.04 (Swift 5.1)"
-      env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.1.5 NO_TSAN=true
+      script: ./.travis-script.sh
+      env: SWIFT_VERSION=5.1.5
     - <<: *tests
       name: "Unit Tests: Ubuntu 18.04 (Swift 5.0)"
-      env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.0.3 NO_TSAN=true
+      script: ./.travis-script.sh
+      env: SWIFT_VERSION=5.0.3
     - <<: *tests
       name: "Unit Tests: Xcode 11.5"
       os: osx
@@ -47,14 +59,15 @@ jobs:
       name: "Interoperability Tests: Ubuntu 18.04 (Swift 5.2)"
       os: linux
       dist: bionic
-      script: ./.travis-script.sh
-      env: RUN_INTEROP_TESTS=true SWIFT_VERSION=5.2
+      install: ./.travis-install.sh -p -i # install protoc and interop server
+      script: ./.travis-script.sh -i  # interop tests
+      env: SWIFT_VERSION=5.2
     - <<: *interop_tests
       name: "Interoperability Tests: Ubuntu 18.04 (Swift 5.1)"
-      env: RUN_INTEROP_TESTS=true SWIFT_VERSION=5.1.5
+      env: SWIFT_VERSION=5.1.5
     - <<: *interop_tests
       name: "Interoperability Tests: Ubuntu 18.04 (Swift 5.0)"
-      env: RUN_INTEROP_TESTS=true SWIFT_VERSION=5.0.3
+      env: SWIFT_VERSION=5.0.3
     - <<: *interop_tests
       name: "Interoperability Tests: Xcode 11.5"
       os: osx
@@ -68,6 +81,7 @@ jobs:
     - <<: *development
 
 stages:
+  - name: "Preflight Checks"
   # Always run this stage.
   - name: "Test"
   # Only run when pushing (or merging) to main
@@ -91,7 +105,3 @@ addons:
       - g++
       - zlib1g-dev
       - python3
-
-before_install: ./scripts/license-check.sh
-install: ./.travis-install.sh
-script: ./.travis-script.sh

scripts/license-check.sh

@@ -140,11 +140,4 @@ check_copyright_headers() {
 
 errors=0
 check_copyright_headers
-
-if [[ "$errors" == 0 ]]; then
-  echo "License headers: OK"
-else
-  echo "License headers: found $errors issue(s)."
-fi
-
 exit $errors

scripts/sanity.sh

@@ -0,0 +1,51 @@
+#!/bin/bash
+
+# Copyright 2020, gRPC Authors All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eu
+
+HERE="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+function run_logged() {
+  local message=$1
+  local command=$2
+
+  log=$(mktemp)
+
+  printf '==> %s ... ' "$message"
+
+  if $command > "$log" 2>&1; then
+    printf "\033[0;32mOK\033[0m\n"
+  else
+    printf "\033[0;31mFAILED\033[0m\n"
+    echo "=== Captured output:"
+    cat "$log"
+    echo "==="
+  fi
+}
+
+function check_license_headers() {
+  run_logged "Checking license headers" "$HERE/license-check.sh"
+}
+
+function check_formatting() {
+  hash swiftformat 2> /dev/null || { printf "\033[0;31mERROR\033[0m swiftformat must be installed (see: https://github.com/nicklockwood/SwiftFormat)\n"; exit 1; }
+  run_logged "Checking formatting" "swiftformat --lint $HERE/.."
+}
+
+check_license_headers
+
+# We won't run this just yet.
+# check_formatting