You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by md...@apache.org on 2022/04/04 17:30:52 UTC

[solr] 03/03: SOLR-16134: Failing integration tests should fail the build (#785)

This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git

commit 570d9a6782198a835914c46642edc7ab2e8dfd42
Author: Mike Drob <md...@apache.org>
AuthorDate: Mon Apr 4 11:38:32 2022 -0500

    SOLR-16134: Failing integration tests should fail the build (#785)
    
    Address a few pipe-fail cases in bin/solr
    
    (cherry picked from commit 3ed17c2737db038c2908a1475099647fb4ef5b38)
---
 solr/bin/solr                                      | 51 +++++++++++-----------
 solr/packaging/build.gradle                        |  1 +
 solr/packaging/test/README.md                      |  2 +
 solr/packaging/test/bats_helper.bash               |  1 +
 .../test/{test_start_solr.bats => test_bats.bats}  | 26 +++++++----
 solr/packaging/test/test_create_collection.bats    |  4 +-
 solr/packaging/test/test_delete_collection.bats    |  4 +-
 solr/packaging/test/test_start_solr.bats           |  4 +-
 8 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/solr/bin/solr b/solr/bin/solr
index e0f0dc26ceb..c1ef2af1e00 100755
--- a/solr/bin/solr
+++ b/solr/bin/solr
@@ -64,6 +64,7 @@ fi
 
 # This helps with debugging when running bats tests but not the whole script is compliant yet
 # set -u
+# set -o pipefail
 
 # Resolve symlinks to this script
 while [ -h "$SOLR_SCRIPT" ] ; do
@@ -848,8 +849,8 @@ function stop_solr() {
         # Check if a process is running with the specified PID.
         # -o stat will output the STAT, where Z indicates a zombie
         # stat='' removes the header (--no-headers isn't supported on all platforms)
-        STAT=`ps -o stat='' $SOLR_PID | tr -d ' '`
-        if [[ "$STAT" != "" && "$STAT" != "Z" ]]; then
+        STAT=`(ps -o stat='' $SOLR_PID || :) | tr -d ' '`
+        if [[ "${STAT:-Z}" != "Z" ]]; then
           slept=$((loops * 2))
           if [ $slept -lt $SOLR_STOP_WAIT ]; then
             sleep 2
@@ -868,9 +869,9 @@ function stop_solr() {
     exit 0
   fi
 
-  STAT=`ps -o stat='' $SOLR_PID | tr -d ' '`
-  if [[ "$STAT" != "" && "$STAT" != "Z" ]]; then
-    if [ "$JSTACK" != "" ]; then
+  STAT=`(ps -o stat='' $SOLR_PID || :) | tr -d ' '`
+  if [[ "${STAT:-Z}" != "Z" ]]; then
+    if [ -n "{$JSTACK:-}" ]; then
       echo -e "Solr process $SOLR_PID is still running; jstacking it now."
       $JSTACK $SOLR_PID
     elif [ "$JATTACH" != "" ]; then
@@ -884,13 +885,13 @@ function stop_solr() {
     sleep 10
   fi
 
-  STAT=`ps -o stat='' $SOLR_PID | tr -d ' '`
-  if [ "$STAT" == "Z" ]; then
+  STAT=`(ps -o stat='' $SOLR_PID || :) | tr -d ' '`
+  if [ "${STAT:-}" == "Z" ]; then
     # This can happen if, for example, you are running Solr inside a docker container with multiple processes
     # rather than running it is as the only service. The --init flag on docker avoids that particular problem.
     echo -e "Solr process $SOLR_PID has terminated abnormally. Solr has exited but a zombie process entry remains."
     exit 1
-  elif [ "$STAT" != "" ]; then
+  elif [ -n "${STAT:-}" ]; then
     echo "ERROR: Failed to kill previous Solr Java process $SOLR_PID ... script fails."
     exit 1
   fi
@@ -1905,12 +1906,12 @@ if [[ "$SCRIPT_CMD" == "start" ]]; then
   # see if Solr is already running
   SOLR_PID=`solr_pid_by_port "$SOLR_PORT"`
 
-  if [ -z "$SOLR_PID" ]; then
+  if [ -z "${SOLR_PID:-}" ]; then
     # not found using the pid file ... but use ps to ensure not found
-    SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+    SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
   fi
 
-  if [ "$SOLR_PID" != "" ]; then
+  if [ -n "${SOLR_PID:-}" ]; then
     echo -e "\nPort $SOLR_PORT is already being used by another process (pid: $SOLR_PID)\nPlease choose a different port using the -p option.\n"
     exit 1
   fi
@@ -1920,7 +1921,7 @@ else
   SOLR_PID=`solr_pid_by_port "$SOLR_PORT"`
   if [ -z "$SOLR_PID" ]; then
     # not found using the pid file ... but use ps to ensure not found
-    SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+    SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
   fi
   if [ "$SOLR_PID" != "" ]; then
     stop_solr "$SOLR_SERVER_DIR" "$SOLR_PORT" "$STOP_KEY" "$SOLR_PID"
@@ -2189,7 +2190,7 @@ function start_solr() {
     echo -e "    JAVA            = $JAVA"
     echo -e "    SOLR_SERVER_DIR = $SOLR_SERVER_DIR"
     echo -e "    SOLR_HOME       = $SOLR_HOME"
-    echo -e "    SOLR_HOST       = $SOLR_HOST"
+    echo -e "    SOLR_HOST       = ${SOLR_HOST:-}"
     echo -e "    SOLR_PORT       = $SOLR_PORT"
     echo -e "    STOP_PORT       = $STOP_PORT"
     echo -e "    JAVA_MEM_OPTS   = ${JAVA_MEM_OPTS[@]}"
@@ -2201,27 +2202,27 @@ function start_solr() {
       echo -e "    CLOUD_MODE_OPTS = ${CLOUD_MODE_OPTS[@]}"
     fi
 
-    if [ "$SOLR_OPTS" != "" ]; then
+    if [ -n "${SOLR_OPTS:-}" ]; then
       echo -e "    SOLR_OPTS       = ${SOLR_OPTS[@]}"
     fi
 
-    if [ "$SOLR_ADDL_ARGS" != "" ]; then
+    if [ -n "${SOLR_ADDL_ARGS:-}" ]; then
       echo -e "    SOLR_ADDL_ARGS  = $SOLR_ADDL_ARGS"
     fi
 
-    if [ "$ENABLE_REMOTE_JMX_OPTS" == "true" ]; then
-      echo -e "    RMI_PORT        = $RMI_PORT"
+    if [ "${ENABLE_REMOTE_JMX_OPTS:-false}" == "true" ]; then
+      echo -e "    RMI_PORT        = ${RMI_PORT:-}"
       echo -e "    REMOTE_JMX_OPTS = ${REMOTE_JMX_OPTS[@]}"
     fi
 
-    if [ "$SOLR_LOG_LEVEL" != "" ]; then
+    if [ -n "${SOLR_LOG_LEVEL:-}" ]; then
       echo -e "    SOLR_LOG_LEVEL  = $SOLR_LOG_LEVEL"
     fi
 
-    if [ "$SOLR_DATA_HOME" != "" ]; then
+    if [ -n "${SOLR_DATA_HOME:-}" ]; then
       echo -e "    SOLR_DATA_HOME  = $SOLR_DATA_HOME"
     fi
-    echo -e "\n"
+    echo
   fi
 
   # need to launch solr from the server dir
@@ -2274,9 +2275,9 @@ function start_solr() {
       (loops=0
       while true
       do
-        running=$(lsof -t -PniTCP:$SOLR_PORT -sTCP:LISTEN)
-        if [ -z "$running" ]; then
-	  slept=$((loops * 2))
+        running=$(lsof -t -PniTCP:$SOLR_PORT -sTCP:LISTEN || :)
+        if [ -z "${running:-}" ]; then
+          slept=$((loops * 2))
           if [ $slept -lt $SOLR_START_WAIT ]; then
             sleep 2
             loops=$[$loops+1]
@@ -2286,7 +2287,7 @@ function start_solr() {
             exit # subshell!
           fi
         else
-          SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+          SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
           echo -e "\nStarted Solr server on port $SOLR_PORT (pid=$SOLR_PID). Happy searching!\n"
           exit # subshell!
         fi
@@ -2295,7 +2296,7 @@ function start_solr() {
     else
       echo -e "NOTE: Please install lsof as this script needs it to determine if Solr is listening on port $SOLR_PORT."
       sleep 10
-      SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+      SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
       echo -e "\nStarted Solr server on port $SOLR_PORT (pid=$SOLR_PID). Happy searching!\n"
       return;
     fi
diff --git a/solr/packaging/build.gradle b/solr/packaging/build.gradle
index 67cdf561556..04279e56f75 100644
--- a/solr/packaging/build.gradle
+++ b/solr/packaging/build.gradle
@@ -218,6 +218,7 @@ class BatsTask extends Exec {
     executable "$project.ext.nodeProjectDir/node_modules/bats/bin/bats"
 
     // Note: tests to run must be listed after all other arguments
+    // Additional debugging output: -x, --verbose-run
     setArgs(['--print-output-on-failure'] + (testFiles.empty ? testDir : testFiles))
 
     super.exec()
diff --git a/solr/packaging/test/README.md b/solr/packaging/test/README.md
index 91fdac223a7..93599e3b3fe 100644
--- a/solr/packaging/test/README.md
+++ b/solr/packaging/test/README.md
@@ -49,6 +49,8 @@ Test are defined as `@test "description of the test" { ... }`
 Some tests will start clusters or create collections,
  please take care to delete any resources that you create.
  They will not be cleaned for you automatically.
+ The example `test_bats.bats` shows how to properly do setup and teardown,
+ along with debug advice and explanations of some subtle traps to avoid.
 
 It is recommended that you install and run `shellcheck` to verify your test scripts and catch common mistakes before committing your changes.
 
diff --git a/solr/packaging/test/bats_helper.bash b/solr/packaging/test/bats_helper.bash
index 88a6cc90c1a..da52b973fc7 100644
--- a/solr/packaging/test/bats_helper.bash
+++ b/solr/packaging/test/bats_helper.bash
@@ -27,6 +27,7 @@ common_setup() {
     load "${BATS_LIB_PREFIX}/bats-assert/load.bash"
 
     PATH="${SOLR_TIP:-.}/bin:$PATH"
+    export SOLR_ULIMIT_CHECKS=false
 }
 
 delete_all_collections() {
diff --git a/solr/packaging/test/test_start_solr.bats b/solr/packaging/test/test_bats.bats
similarity index 61%
copy from solr/packaging/test/test_start_solr.bats
copy to solr/packaging/test/test_bats.bats
index ff0ba356a6d..53c09544c19 100644
--- a/solr/packaging/test/test_start_solr.bats
+++ b/solr/packaging/test/test_bats.bats
@@ -15,19 +15,29 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# This file has stubs for setup and teardown of a cluster and debugging hints
+
 load bats_helper
 
-setup() {
+setup_file() {
+  # set up paths and helpers
   common_setup
+
+  solr start -c -V
+  # echo $output >&3
 }
 
-teardown() {
-  solr stop -all >/dev/null 2>&1
+teardown_file() {
+  # set up paths, not preserved from setup
+  common_setup
+
+  # Conversely, on shutdown, we do need this to execute strictly
+  # because using "run" will eat filing test exit codes
+  solr stop -all
+  # DEBUG : (echo -n "# " ; solr stop -V -all) >&3
 }
 
-@test "SOLR11740 check f" {
-  run -0 solr start
-  run -0 solr start -p 7574
-  run bash -c 'solr stop -all 2>&1'
-  refute_output --partial 'forcefully killing'
+@test "nothing" {
+  # hint: if we need to demonstrate a failing test, change this line to 'false'
+  true
 }
diff --git a/solr/packaging/test/test_create_collection.bats b/solr/packaging/test/test_create_collection.bats
index 7d01ba45f56..8430992af43 100644
--- a/solr/packaging/test/test_create_collection.bats
+++ b/solr/packaging/test/test_create_collection.bats
@@ -19,7 +19,7 @@ load bats_helper
 
 setup_file() {
   common_setup
-  run solr start -c
+  solr start -c
 
   local source_configset_dir="$SOLR_TIP/server/solr/configsets/sample_techproducts_configs"
   test -d $source_configset_dir
@@ -28,7 +28,7 @@ setup_file() {
 
 teardown_file() {
   common_setup
-  run solr stop -all
+  solr stop -all
 }
 
 setup() {
diff --git a/solr/packaging/test/test_delete_collection.bats b/solr/packaging/test/test_delete_collection.bats
index 1951a9812ba..5d3afd9cd84 100644
--- a/solr/packaging/test/test_delete_collection.bats
+++ b/solr/packaging/test/test_delete_collection.bats
@@ -19,12 +19,12 @@ load bats_helper
 
 setup_file() {
   common_setup
-  run solr start -c
+  solr start -c
 }
 
 teardown_file() {
   common_setup
-  run solr stop -all
+  solr stop -all
 }
 
 setup() {
diff --git a/solr/packaging/test/test_start_solr.bats b/solr/packaging/test/test_start_solr.bats
index ff0ba356a6d..048f99fc9e1 100644
--- a/solr/packaging/test/test_start_solr.bats
+++ b/solr/packaging/test/test_start_solr.bats
@@ -26,8 +26,8 @@ teardown() {
 }
 
 @test "SOLR11740 check f" {
-  run -0 solr start
-  run -0 solr start -p 7574
+  solr start
+  solr start -p 7574
   run bash -c 'solr stop -all 2>&1'
   refute_output --partial 'forcefully killing'
 }