You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2019/04/09 15:48:58 UTC

[impala] 06/06: IMPALA-8371: Return appropriate error code for unified backend tests

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

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 45364a47c12a71eee9ee5592e05a8fc2295951d5
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Thu Mar 28 14:23:22 2019 -0700

    IMPALA-8371: Return appropriate error code for unified backend tests
    
    Unified backend tests rely on generating bash scripts for each test
    that call the unified executable with a filter to run the appropriate
    subset of the tests. The generated script currently does not return
    the return code from the test execution.
    
    This changes the test execution scripts to return the appropriate
    return code. To do this, the script generator is changed from
    a bash implementation in bin/gen-backend-test-script.sh to
    a python implementation in bin/gen-backend-test-script.py.
    This makes it easier to handle shell variables in the script
    template correctly.
    
    Testing:
     - Ran backend tests on centos 6, centos 7
     - Manually tested with a failing test and verified return value
    
    Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
    Reviewed-on: http://gerrit.cloudera.org:8080/12885
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/CMakeLists.txt              |  6 ++--
 bin/gen-backend-test-script.py | 77 ++++++++++++++++++++++++++++++++++++++++++
 bin/gen-backend-test-script.sh | 48 --------------------------
 3 files changed, 80 insertions(+), 51 deletions(-)

diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index 7fe2db0..c671daf 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -601,9 +601,9 @@ ENDFUNCTION()
 FUNCTION(ADD_UNIFIED_BE_TEST TEST_NAME TEST_FILTER_PATTERN)
   # This gets the directory where the test is from (e.g. 'exprs' or 'runtime')
   file(RELATIVE_PATH DIR_NAME "${CMAKE_SOURCE_DIR}/be/src/" ${CMAKE_CURRENT_SOURCE_DIR})
-  add_custom_target(${TEST_NAME} "${CMAKE_SOURCE_DIR}/bin/gen-backend-test-script.sh"
-    "${BUILD_OUTPUT_ROOT_DIRECTORY}/${DIR_NAME}/${TEST_NAME}"
-    ${TEST_FILTER_PATTERN})
+  add_custom_target(${TEST_NAME} "${CMAKE_SOURCE_DIR}/bin/gen-backend-test-script.py"
+    "--test_script_output" "${BUILD_OUTPUT_ROOT_DIRECTORY}/${DIR_NAME}/${TEST_NAME}"
+    "--gtest_filter" ${TEST_FILTER_PATTERN})
   # Incorporate this TEST_FILTER_PATTERN into the aggregate list of filter patterns
   get_property(tmp GLOBAL PROPERTY AGG_UNIFIED_FILTER_PATTERN)
   set(tmp "${TEST_FILTER_PATTERN}:${tmp}")
diff --git a/bin/gen-backend-test-script.py b/bin/gen-backend-test-script.py
new file mode 100755
index 0000000..3abf306
--- /dev/null
+++ b/bin/gen-backend-test-script.py
@@ -0,0 +1,77 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+# This script generates a bash script to run a certain set of backend tests. It is
+# used by the build system (see be/CMakeLists.txt for usage) and is not intended to be
+# run directly.
+
+import argparse
+import os
+import stat
+import textwrap
+
+# The script header template requires substitutions to set the variables used by the body.
+script_header_template = textwrap.dedent("""\
+    #!/bin/bash
+    GTEST_FILTER="{gtest_filter}"
+    TEST_EXEC_NAME="{test_exec_name}"
+""")
+
+# The script body uses shell variables for all of its key components. It requires
+# no substitutions.
+script_body = textwrap.dedent(r"""
+    ${IMPALA_HOME}/be/build/latest/service/unifiedbetests \
+      --gtest_filter=${GTEST_FILTER} \
+      --gtest_output=xml:${IMPALA_BE_TEST_LOGS_DIR}/${TEST_EXEC_NAME}.xml \
+      -log_filename="${TEST_EXEC_NAME}" \
+      "$@"
+    RETURN_CODE=$?
+    # Running a gtest executable using the -gtest_filter flag produces a JUnitXML that
+    # contains all the tests whether they ran or not. The tests that ran are populated
+    # normally. The tests that didn't run have status="notrun". This is a problem for the
+    # unified binary, because every JUnitXML would contain every test, with a large
+    # portion of them marked as notrun. Various consumers of JUnitXML (such as Jenkins)
+    # may not understand this distinction, so it is useful to prune the notrun entries.
+    # The junitxml_prune_notrun.py walks through the JUnitXML file and drops the entries
+    # that did not run, producing a new JUnitXML file of the same name.
+    ${IMPALA_HOME}/bin/junitxml_prune_notrun.py  \
+      -f "${IMPALA_BE_TEST_LOGS_DIR}/${TEST_EXEC_NAME}.xml"
+    exit ${RETURN_CODE}
+""")
+
+
+def main():
+  parser = argparse.ArgumentParser()
+  parser.add_argument("--gtest_filter", required=True,
+    help="The location to write the generated script")
+  parser.add_argument("--test_script_output", required=True,
+    help="The test filter pattern to pass to gtest to limit the set of tests run")
+  args = parser.parse_args()
+  with open(args.test_script_output, "w") as f:
+    test_exec_name = os.path.basename(args.test_script_output)
+    script_header = script_header_template.format(gtest_filter=args.gtest_filter,
+      test_exec_name=test_exec_name)
+    f.write(script_header)
+    f.write(script_body)
+  # Make the script executable by user and group.
+  st = os.stat(args.test_script_output)
+  os.chmod(args.test_script_output, st.st_mode | stat.S_IXUSR | stat.S_IXGRP)
+
+
+if __name__ == "__main__":
+  main()
diff --git a/bin/gen-backend-test-script.sh b/bin/gen-backend-test-script.sh
deleted file mode 100755
index 32322b7..0000000
--- a/bin/gen-backend-test-script.sh
+++ /dev/null
@@ -1,48 +0,0 @@
-#!/bin/bash
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you 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.
-
-# This script generates a bash script to run a certain set of backend tests
-# using the unified backend test executable. It take two positional arguments:
-# 1: The file location to write the generated script
-# 2: The test filter pattern that describes which tests to run (following gtest's
-#    --gtest_filter)
-# This script is used by the build system and is not intended to be run directly.
-TEST_EXEC_LOCATION=${1}
-GTEST_FILTER=${2}
-TEST_EXEC_NAME=$(basename "${TEST_EXEC_LOCATION}")
-
-cat > "${TEST_EXEC_LOCATION}" << EOF
-#!/bin/bash
-${IMPALA_HOME}/be/build/latest/service/unifiedbetests \
-  --gtest_filter=${GTEST_FILTER} \
-  --gtest_output=xml:${IMPALA_BE_TEST_LOGS_DIR}/${TEST_EXEC_NAME}.xml \
-  -log_filename="${TEST_EXEC_NAME}" \
-  "\$@"
-# Running a gtest executable using the -gtest_filter flag produces a JUnitXML that
-# contains all the tests whether they ran or not. The tests that ran are populated
-# normally. The tests that didn't run have status="notrun". This is a problem for the
-# unified binary, because every JUnitXML would contain every test, with a large portion
-# of them marked as notrun. Various consumers of JUnitXML (such as Jenkins) may not
-# understand this distinction, so it is useful to prune the notrun entries. The
-# junitxml_prune_notrun.py walks through the JUnitXML file and drops the entries that
-# did not run, producing a new JUnitXML file of the same name.
-${IMPALA_HOME}/bin/junitxml_prune_notrun.py \
-  -f "${IMPALA_BE_TEST_LOGS_DIR}/${TEST_EXEC_NAME}.xml"
-EOF
-
-chmod +x "${TEST_EXEC_LOCATION}"