From 79ec8fd7c151947fbcab246215c89c1bb84e32a2 Mon Sep 17 00:00:00 2001 From: Eric Lunderberg Date: Tue, 5 Jul 2022 15:35:36 -0500 Subject: [PATCH 1/4] [CI] Use command-line argument or TVM_BUILD_PATH for C++ unittests Previously, the `ci.py` script would execute all C++ unit tests in the `"build"` directory, regardless of the docker image being used. This change allows a caller to specify the build directory to be used by `task_cpp_unittest.sh`, either by the command line or by using the same `TVM_BUILD_PATH environment variable as used by the top-level Makefile, and passes this argument from `ci.py`. To preserve the existing behavior for the pre-commit CI, if no argument is passed and if the `TVM_BUILD_PATH` is undefined, `task_cpp_unittest.sh` defaults to the `"build"` directory. Python unit tests executed through `ci.py` used the `TVM_LIBRARY_PATH` environment variable, and were not similarly affected. --- tests/scripts/ci.py | 17 ++++++++++++----- tests/scripts/task_cpp_unittest.sh | 17 ++++++++++++++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/tests/scripts/ci.py b/tests/scripts/ci.py index 1ffd2d20e7ae..598e29342e49 100755 --- a/tests/scripts/ci.py +++ b/tests/scripts/ci.py @@ -372,12 +372,14 @@ def fn( if precheck is not None: precheck() + build_dir = get_build_dir(name) + if skip_build: scripts = [] else: scripts = [ - f"./tests/scripts/task_config_build_{name}.sh {get_build_dir(name)}", - f"./tests/scripts/task_build.py --build-dir {get_build_dir(name)}", + f"./tests/scripts/task_config_build_{name}.sh {build_dir}", + f"./tests/scripts/task_build.py --build-dir {build_dir}", ] if post_build is not None: @@ -394,7 +396,9 @@ def fn( # Add named test suites for option_name, (_, extra_scripts) in options.items(): if kwargs.get(option_name, False): - scripts += extra_scripts + scripts.extend( + script.format(name=name, build_dir=build_dir) for script in extra_scripts + ) docker( name=gen_name(f"ci-{name}"), @@ -553,7 +557,7 @@ def add_subparser( return subparser -CPP_UNITTEST = ("run c++ unitests", ["./tests/scripts/task_cpp_unittest.sh"]) +CPP_UNITTEST = ("run c++ unitests", ["./tests/scripts/task_cpp_unittest.sh {build_dir}"]) generated = [ generate_command( @@ -610,7 +614,10 @@ def add_subparser( generate_command( name="wasm", help="Run WASM build and test(s)", - options={"test": ("run WASM tests", ["./tests/scripts/task_web_wasm.sh"])}, + options={ + "cpp": CPP_UNITTEST, + "test": ("run WASM tests", ["./tests/scripts/task_web_wasm.sh"]), + }, ), generate_command( name="qemu", diff --git a/tests/scripts/task_cpp_unittest.sh b/tests/scripts/task_cpp_unittest.sh index a28efb0328ec..5dd278f8807a 100755 --- a/tests/scripts/task_cpp_unittest.sh +++ b/tests/scripts/task_cpp_unittest.sh @@ -18,6 +18,16 @@ set -euxo pipefail +if [ $# -gt 0 ]; then + BUILD_DIR="$1" +elif [ -n "${TVM_BUILD_PATH}" ]; then + # TVM_BUILD_PATH may contain multiple space-separated paths. If + # so, use the first one. + BUILD_DIR=$(IFS=" "; set -- $TVM_BUILD_PATH; echo $1) +else + BUILD_DIR=build +fi + # Python is required by apps/bundle_deploy source tests/scripts/setup-pytest-env.sh @@ -32,16 +42,17 @@ export OMP_NUM_THREADS=1 # Build cpptest suite python3 tests/scripts/task_build.py \ --sccache-bucket tvm-sccache-prod \ - --cmake-target cpptest + --cmake-target cpptest \ + --build-dir "${BUILD_DIR}" # crttest requires USE_MICRO to be enabled, which is currently the case # with all CI configs -pushd build +pushd "${BUILD_DIR}" ninja crttest popd -pushd build +pushd "${BUILD_DIR}" ctest --gtest_death_test_style=threadsafe popd From fb35c460e14ec3ad93c7a2317f39d8920efff336 Mon Sep 17 00:00:00 2001 From: Eric Lunderberg Date: Tue, 5 Jul 2022 16:18:07 -0500 Subject: [PATCH 2/4] Remove `name=name` in format script Co-authored-by: driazati <9407960+driazati@users.noreply.github.com> --- tests/scripts/ci.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/ci.py b/tests/scripts/ci.py index 598e29342e49..ea8288f42f04 100755 --- a/tests/scripts/ci.py +++ b/tests/scripts/ci.py @@ -397,7 +397,7 @@ def fn( for option_name, (_, extra_scripts) in options.items(): if kwargs.get(option_name, False): scripts.extend( - script.format(name=name, build_dir=build_dir) for script in extra_scripts + script.format(build_dir=build_dir) for script in extra_scripts ) docker( From 5154b4a8549985e286164d9ee9f6a0dc9c500a6a Mon Sep 17 00:00:00 2001 From: Eric Lunderberg Date: Tue, 5 Jul 2022 16:34:47 -0500 Subject: [PATCH 3/4] Fix lint error --- tests/scripts/ci.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/scripts/ci.py b/tests/scripts/ci.py index ea8288f42f04..022d19200232 100755 --- a/tests/scripts/ci.py +++ b/tests/scripts/ci.py @@ -396,9 +396,7 @@ def fn( # Add named test suites for option_name, (_, extra_scripts) in options.items(): if kwargs.get(option_name, False): - scripts.extend( - script.format(build_dir=build_dir) for script in extra_scripts - ) + scripts.extend(script.format(build_dir=build_dir) for script in extra_scripts) docker( name=gen_name(f"ci-{name}"), From 3e65c2c7eca093fd3376bf7b3afb715f212c5f81 Mon Sep 17 00:00:00 2001 From: Eric Lunderberg Date: Tue, 5 Jul 2022 19:55:25 -0500 Subject: [PATCH 4/4] Use default expansion of TVM_BUILD_PATH Otherwise, `set -u` rightly errors out for it being undefined. --- tests/scripts/task_cpp_unittest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/task_cpp_unittest.sh b/tests/scripts/task_cpp_unittest.sh index 5dd278f8807a..8ae2e9b1109f 100755 --- a/tests/scripts/task_cpp_unittest.sh +++ b/tests/scripts/task_cpp_unittest.sh @@ -20,7 +20,7 @@ set -euxo pipefail if [ $# -gt 0 ]; then BUILD_DIR="$1" -elif [ -n "${TVM_BUILD_PATH}" ]; then +elif [ -n "${TVM_BUILD_PATH:-}" ]; then # TVM_BUILD_PATH may contain multiple space-separated paths. If # so, use the first one. BUILD_DIR=$(IFS=" "; set -- $TVM_BUILD_PATH; echo $1)