Refactor test_art_target.sh and benchmarks_run_target

Change-Id: I1cf2eb4b52e97cf83ccdb292eecf9e0713eb5fc9

4 files changed, 190 insertions, 219 deletions

diff --git a/benchmarks/benchmarks_run_target.sh b/benchmarks/benchmarks_run_target.sh
index 35479efc..b30690d9 100755
--- a/benchmarks/benchmarks_run_target.sh
+++ b/benchmarks/benchmarks_run_target.sh
@@ -2,7 +2,8 @@
 
 readonly local_path=$(dirname "$0")
 source "${local_path}/../utils/utils.sh"
-source "${local_path}/../utils//utils_android.sh"
+source "${local_path}/../utils/utils_android.sh"
+source "${local_path}/../utils/utils_android_root.sh"
 source "${local_path}/../devices/cpu_freq_utils.sh"
 
 readonly target_timer_name="Target Benchmarks"
@@ -11,7 +12,6 @@ readonly test_time_file="${log_directory}/time_benchmarks.txt"
 
 declare -A options
 readonly default_iterations=10
-tests_summary=
 
 init_options() {
   options["mode"]="all"
@@ -109,92 +109,21 @@ arguments_parser() {
   validate_options
 }
 
-start_section() {
-  log I "Starting section: $1"
-  tests_summary+=$(printf "%-25s " "$1")
-  start_timer "$1"
-  start_logging "${log_directory}/log_build_$1.txt"
-}
-
-end_section() {
-  stop_logging "${log_directory}/log_build_$1.txt"
-  stop_timer "$1"
-  local test_time=$(print_timer "$1")
-  tests_summary+=$(printf "TOOK: %-10s" "${test_time}")
-  tests_summary+="\n"
-  log S "Section $1 FINISHED"
-}
-
-set_common_environment() {
-  # All these should be in sync with:
-  # https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/recipes/art.py
-  log I "Setting-up the test environment"
-
-  # ART test env setting.
-  export ART_BUILD_HOST_DEBUG=false
-  export ART_TEST_ANDROID_ROOT="/data/local/tmp/system"
-  # Java settings.
-  export LEGACY_USE_JAVA7=true
-  # Jack settings.
-  export JACK_SERVER=false
-  export JACK_REPOSITORY="${PWD}/prebuilts/sdk/tools/jacks"
-  # Dex2oat settings.
-  export USE_DEX2OAT_DEBUG=false
-}
-
-build_target() {
-  start_section "build_target_$1"
-
-  if [[ $1 -eq 32 ]]; then
-    setup_android_target "arm_krait-eng"
-    export CUSTOM_TARGET_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker"
-  elif [[ $1 -eq 64 ]]; then
-    setup_android_target "armv8-eng"
-    export CUSTOM_TARGET_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker64"
-    export CUSTOM_TARGET_2ND_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker"
-  else
-    log E "Mode not specified correctly. Make sure your bits are complete ($@)."
-    abort
-  fi
-
-  # FIXME(Stable): We need to build dx for the stable branch.
-  safe_make_build "" "" dx
-  safe ./art/tools/buildbot-build.sh -j"${jcpu_count}" --target
-  end_section "build_target_$1"
-}
-
-device_setup() {
-  start_section "device_setup_$1"
-  # Get adb root - needed for adb shell stop and device_cleanup().
-  safe adb root
-  # Set up buildbot device.
-  safe ./art/tools/setup-buildbot-device.sh
-  # Stop the framework.
-  safe adb shell stop
-  end_section "device_setup_$1"
-}
-
-device_cleanup() {
-  start_section "device_cleanup_$1"
-  # Clean up all vestiges of ART testing on device.
-  safe adb shell rm -rf /data/local/tmp /data/art-test /data/nativetest
-  end_section "device_cleanup_$1"
-}
-
-sync_target() {
-  start_section "sync_target_$1"
+# Note we need adb push for the benchmarks.
+sync_target_adb_push() {
+  start_adb_section "sync_target_$1"
   # Note: We use adb push since this is what lava can use too.
   safe adb push "${OUT}/system" /data/local/tmp/system
   # Push the core.art image.
   safe adb push "${OUT}/data/art-test" /data/art-test
-  end_section "sync_target_$1"
+  end_adb_section "sync_target_$1" "$?"
 }
 
 run_benchmarks() {
-  start_section "run_benchmarks_$2_$1_$3"
+  start_adb_section "run_benchmarks_$2_$1_$3"
   safe ./benchmarks/run.py --target --iterations "${options["iterations"]}" --mode "$1" \
   --android-root "${ART_TEST_ANDROID_ROOT}" --output-json "${log_directory}/$2_$1_$3_aot.json"
-  end_section "run_benchmarks_$2_$1_$3"
+  end_adb_section "run_benchmarks_$2_$1_$3" "$?"
 }
 
 run_all_benchmarks() {
@@ -220,9 +149,6 @@ run_all_benchmarks() {
     safe "${local_path}/../devices/set_cpu_freq.sh" --all --pin_freq
     run_benchmarks "$1" "${target_device}" "${CPUS_NAME}"
   fi
-
-  # Be a good citizen and restore the CPUs configuration to defaults.
-  safe "${local_path}/../devices/set_cpu_freq.sh" --default
 }
 
 main() {
@@ -245,7 +171,7 @@ main() {
     log I "Starting ${bits}bit benchmarks."
 
     # Set environemnt variables.
-    set_common_environment
+    set_environment_common
 
     # Build target.
     build_target "${bits}"
@@ -253,19 +179,26 @@ main() {
     # Setup & sync device.
     device_setup "${bits}"
     device_cleanup "${bits}"
-    sync_target "${bits}"
+    sync_target_adb_push "${bits}"
 
     # Set freq.
     run_all_benchmarks "${bits}"
   done
 
+  # Return the device to its default configuration.
+  device_restore
   stop_timer "${target_timer_name}"
 
   log S "$0 Finished!"
   dump_timer "${target_timer_name}" "${test_time_file}"
-  log S "Test Summary:\n${tests_summary}"
 
-  exit 0
+  local log_level="S"
+  if [[ ${TESTS_RETURN_CODE} -ne 0 ]]; then
+    log_level="E"
+  fi
+  log "${log_level}" "Test Summary:\n${TESTS_SUMMARY}"
+
+  exit "${TESTS_RETURN_CODE}"
 }
 
 main "$@"
diff --git a/tests/test_art_target.sh b/tests/test_art_target.sh
index 49bd65ef..5bf7a26a 100755
--- a/tests/test_art_target.sh
+++ b/tests/test_art_target.sh
@@ -8,14 +8,13 @@
 readonly local_path=$(dirname "$0")
 source "${local_path}/../utils/utils.sh"
 source "${local_path}/../utils/utils_android.sh"
+source "${local_path}/../utils/utils_android_root.sh"
 
 readonly target_timer_name="Target Test"
 readonly log_directory="$(get_workspace)"
 readonly test_time_file="${log_directory}/time_test.txt"
 
 declare -A options
-tests_summary=
-return_code=0
 
 init_options() {
   options["32bit"]="false"
@@ -168,140 +167,23 @@ arguments_parser() {
   validate_options
 }
 
-start_section() {
-  log I "Starting section: $1"
-  tests_summary+=$(printf "%-25s" "$1")
-  start_timer "$1"
-  start_logging "${log_directory}/log_build_$1.txt"
-}
-
-start_adb_section() {
-  start_section "$1"
-  adb_clear_logcat
-}
-
-end_section() {
-  stop_logging "${log_directory}/log_build_$1.txt"
-  stop_timer "$1"
-  local test_time=$(print_timer "$1")
-  tests_summary+=$(printf "TOOK: %-10s" "${test_time}")
-
-  if [[ $2 -ne 0 ]]; then
-    log E "Section: $1 FAILED"
-    tests_summary+="FAILED\n"
-    return_code=$2
-
-    if ! ${options["keep_going"]}; then
-      log E "--keep_going is not set. Stop on the first failed test."
-      exit "$2"
-    fi
-  else
-    log S "Section $1 PASSED"
-    tests_summary+="PASSED\n"
-  fi
-}
-
-end_adb_section() {
-  end_section "$1" "$2"
-  adb_dump_logcat "${log_directory}/log_logcat_$1.txt"
-}
-
-skip_section() {
-  log I "Skipping section $1"
-  stop_logging "${log_directory}/log_build_$1.txt"
-  stop_timer "$1"
-  tests_summary+=$(printf "TOOK: %-10s" "0s")
-  tests_summary+="SKIPPED\n"
-}
-
-set_common_environment() {
-  # All these should be in sync with:
-  # https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/recipes/art.py
-  log I "Setting-up the test environment"
-
-  # ART Tests to be run.
-  export ART_TEST_INTERPRETER=true
-  export ART_TEST_JIT=true
-  export ART_TEST_OPTIMIZING=true
-  export ART_USE_OPTIMIZING_COMPILER=true
-  # Should the script stop after the first failing test?
-  export ART_TEST_KEEP_GOING=true
-  # ART test env setting.
-  export ART_BUILD_HOST_DEBUG=false
-  export ART_TEST_ANDROID_ROOT="/data/local/tmp/system"
-  export ART_TEST_FULL=false
-  export ART_TEST_RUN_TEST_2ND_ARCH=false
-  export ART_TEST_NO_SYNC=true
-  # Java settings.
-  export LEGACY_USE_JAVA7=true
-  # Jack settings.
-  export JACK_SERVER=false
-  export JACK_REPOSITORY="${PWD}/prebuilts/sdk/tools/jacks"
-  # Dex2oat settings.
-  export USE_DEX2OAT_DEBUG=false
+set_environment() {
+  set_environment_common
+  set_environment_tests
 
   if ! ${options["debug"]}; then
-    export ART_TEST_RUN_TEST_DEBUG=false
-    export ART_TEST_RUN_TEST_NDEBUG=true
+    set_environment_debug
   fi
 
   if ${options["concurrent_gc"]}; then
-    export ART_USE_READ_BARRIER=false
-    export ART_HEAP_POISONING=false
+    set_environment_concurrent_gc
   fi
 
   if ${options["keep_failures"]}; then
-    export ART_TEST_RUN_TEST_ALWAYS_CLEAN=false
-    export ART_TEST_QUIET=false
-  fi
-}
-
-build_target() {
-  start_section "build_target_$1"
-
-  if [[ $1 -eq 32 ]]; then
-    setup_android_target "arm_krait-eng"
-    export CUSTOM_TARGET_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker"
-  elif [[ $1 -eq 64 ]]; then
-    setup_android_target "armv8-eng"
-    export CUSTOM_TARGET_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker64"
-    export CUSTOM_TARGET_2ND_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker"
-  else
-    log E "Mode not specified correctly. Make sure your bits are complete ($@)."
-    abort
+    set_environment_keep_test_failures
   fi
-
-  safe ./art/tools/buildbot-build.sh -j"${jcpu_count}" --target
-  end_section "build_target_$1" "$?"
-}
-
-configure_cpus() {
-  safe "${local_path}/../devices/set_cpu_freq.sh" --default
-}
-
-device_setup() {
-  start_section "device_setup_$1"
-  # Get adb root - needed otherwise device_cleanup() would fail.
-  safe adb root
-  # Set up buildbot device.
-  safe ./art/tools/setup-buildbot-device.sh
-  end_section "device_setup_$1" "$?"
-}
-
-device_cleanup() {
-  start_section "device_cleanup_$1"
-  # Clean up all vestiges of ART testing on device.
-  safe adb shell rm -rf /data/local/tmp /data/art-test /data/nativetest
-  end_section "device_cleanup_$1" "$?"
-}
-
-sync_target() {
-  start_adb_section "sync_target_$1"
-  safe make ART_TEST_NO_SYNC=false -j"${jcpu_count}" "test-art-target-sync"
-  end_adb_section "sync_target_$1" "$?"
 }
 
-
 # Build (run) a test target using the Android build system.
 # If a test failed, add it to the list of failed tests.
 # Arguments:
@@ -325,8 +207,15 @@ run_test() {
   disable_error_on_unset_expansion
   # Adding dist will filter out time sensitive tests such as 055-enum-performance.
   make "-j${target_cpu_count}" "$1" "dist"
-  end_adb_section "$2_$3" "$?"
+  local -r return_code=$?
   enable_error_on_unset_expansion
+  end_adb_section "$2_$3" "${return_code}"
+
+  if ! ${options["keep_going"]} && [[ ${return_code} -ne 0 ]]; then
+    log E "--keep_going is not set. Stop on the first failed test."
+    log E "Test Summary:\n${TESTS_SUMMARY}"
+    exit 1
+  fi
 }
 
 test_gtest() {
@@ -406,13 +295,11 @@ main() {
     log I "Starting ${bits}bit tests."
 
     # Set environemnt variables.
-    set_common_environment
+    set_environment
 
     # Build target.
     build_target "${bits}"
 
-    configure_cpus
-
     # Setup & sync device.
     device_setup "${bits}"
     device_cleanup "${bits}"
@@ -426,18 +313,21 @@ main() {
     test_libcore "${bits}"
     test_jdwp "${bits}"
   done
+
+  # Return the device to its default configuration.
+  device_restore
   stop_timer "${target_timer_name}"
 
   log I "$0 Finished!"
   dump_timer "${target_timer_name}" "${test_time_file}"
 
   local log_level="S"
-  if [[ ${return_code} -ne 0 ]]; then
+  if [[ ${TESTS_RETURN_CODE} -ne 0 ]]; then
     log_level="E"
   fi
-  log "${log_level}" "Test Summary:\n${tests_summary}"
+  log "${log_level}" "Test Summary:\n${TESTS_SUMMARY}"
 
-  exit "${return_code}"
+  exit "${TESTS_RETURN_CODE}"
 }
 
 main "$@"
diff --git a/utils/utils_android.sh b/utils/utils_android.sh
index 5900891b..2328a427 100644
--- a/utils/utils_android.sh
+++ b/utils/utils_android.sh
@@ -5,6 +5,53 @@
 readonly jcpu_count=$(nproc)
 readonly build_log="$(get_workspace)/log_build.txt"
 
+# Global cumulative test summary and return code. Used by end/start_<adb>_section().
+TESTS_SUMMARY=
+# shellcheck disable=SC2034
+TESTS_RETURN_CODE=0
+
+start_section() {
+  log I "Starting section: $1"
+  TESTS_SUMMARY+=$(printf "%-25s" "$1")
+  start_timer "$1"
+  start_logging "${log_directory}/log_build_$1.txt"
+}
+
+start_adb_section() {
+  start_section "$1"
+  adb_clear_logcat
+}
+
+end_section() {
+  stop_logging "${log_directory}/log_build_$1.txt"
+  stop_timer "$1"
+  local test_time=$(print_timer "$1")
+  TESTS_SUMMARY+=$(printf "TOOK: %-10s" "${test_time}")
+
+  if [[ $2 -ne 0 ]]; then
+    log E "Section: $1 FAILED"
+    TESTS_SUMMARY+="FAILED\n"
+    # shellcheck disable=SC2034
+    TESTS_RETURN_CODE=$2
+  else
+    log S "Section $1 PASSED"
+    TESTS_SUMMARY+="PASSED\n"
+  fi
+}
+
+end_adb_section() {
+  end_section "$1" "$2"
+  adb_dump_logcat "${log_directory}/log_logcat_$1.txt"
+}
+
+skip_section() {
+  log I "Skipping section $1"
+  stop_logging "${log_directory}/log_build_$1.txt"
+  stop_timer "$1"
+  TESTS_SUMMARY+=$(printf "TOOK: %-10s" "0s")
+  TESTS_SUMMARY+="SKIPPED\n"
+}
+
 env_defines_lunch_target() {
   if [[ -v TARGET_PRODUCT && -v TARGET_BUILD_VARIANT ]]; then
     return 0
@@ -74,7 +121,6 @@ setup_android_target_to_environment_or_default() {
   fi
 }
 
-
 mm_build() {
   disable_error_on_unset_expansion
   start_logging "${build_log}"
diff --git a/utils/utils_android_root.sh b/utils/utils_android_root.sh
new file mode 100644
index 00000000..f2821ac2
--- /dev/null
+++ b/utils/utils_android_root.sh
@@ -0,0 +1,102 @@
+#!/bin/bash
+
+# Common android root methods.
+
+# All these should be in sync with:
+# https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/recipes/art.py
+set_environment_common() {
+  log I "Setting-up the common adb root environment."
+
+  # ART test env setting.
+  export ART_BUILD_HOST_DEBUG=false
+  export ART_TEST_ANDROID_ROOT="/data/local/tmp/system"
+  # Java settings.
+  export LEGACY_USE_JAVA7=true
+  # Jack settings.
+  export JACK_SERVER=false
+  export JACK_REPOSITORY="${PWD}/prebuilts/sdk/tools/jacks"
+  # Dex2oat settings.
+  export USE_DEX2OAT_DEBUG=false
+}
+
+set_environment_tests() {
+  # ART Tests to be run.
+  export ART_TEST_INTERPRETER=true
+  export ART_TEST_JIT=true
+  export ART_TEST_OPTIMIZING=true
+  export ART_USE_OPTIMIZING_COMPILER=true
+  # Should the script stop after the first failing test?
+  export ART_TEST_KEEP_GOING=true
+  export ART_TEST_FULL=false
+  export ART_TEST_RUN_TEST_2ND_ARCH=false
+  export ART_TEST_NO_SYNC=true
+}
+
+set_environment_debug() {
+  export ART_TEST_RUN_TEST_DEBUG=false
+  export ART_TEST_RUN_TEST_NDEBUG=true
+}
+
+set_environment_concurrent_gc() {
+  export ART_USE_READ_BARRIER=false
+  export ART_HEAP_POISONING=false
+}
+
+set_environment_keep_test_failures() {
+  export ART_TEST_RUN_TEST_ALWAYS_CLEAN=false
+  export ART_TEST_QUIET=false
+}
+
+device_setup() {
+  start_adb_section "device_setup_$1"
+  # Get adb root - needed for adb shell stop and device_cleanup().
+  safe adb root
+  # Set up buildbot device.
+  safe ./art/tools/setup-buildbot-device.sh
+  # Stop the framework.
+  safe adb shell stop
+  # Set the CPUs to their default configuration.
+  safe "${local_path}/../devices/set_cpu_freq.sh" --default
+  end_adb_section "device_setup_$1" "$?"
+}
+
+device_restore() {
+  # Start the framework.
+  safe adb shell start
+  # Be a good citizen and restore the CPUs configuration to defaults.
+  safe "${local_path}/../devices/set_cpu_freq.sh" --default
+}
+
+device_cleanup() {
+  start_section "device_cleanup_$1"
+  # Clean up all vestiges of ART testing on device.
+  safe adb shell rm -rf /data/local/tmp /data/art-test /data/nativetest
+  end_section "device_cleanup_$1" "$?"
+}
+
+build_target() {
+  start_section "build_target_$1"
+
+  if [[ $1 -eq 32 ]]; then
+    setup_android_target "arm_krait-eng"
+    export CUSTOM_TARGET_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker"
+  elif [[ $1 -eq 64 ]]; then
+    setup_android_target "armv8-eng"
+    export CUSTOM_TARGET_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker64"
+    export CUSTOM_TARGET_2ND_LINKER="${ART_TEST_ANDROID_ROOT}/bin/linker"
+  else
+    log E "Mode not specified correctly. Make sure your bits are complete ($@)."
+    abort
+  fi
+
+  # FIXME(Stable): We need to build dx for the stable branch.
+  safe_make_build "" "" dx
+  safe ./art/tools/buildbot-build.sh -j"${jcpu_count}" --target
+  end_section "build_target_$1" "$?"
+}
+
+sync_target() {
+  start_adb_section "sync_target_$1"
+  safe make ART_TEST_NO_SYNC=false -j"${jcpu_count}" "test-art-target-sync"
+  end_adb_section "sync_target_$1" "$?"
+}