From 8b21109d261a41690641fd19e8d646bc19fb3d1e Mon Sep 17 00:00:00 2001 From: Jon Brandvein Date: Thu, 9 May 2019 16:42:47 -0400 Subject: [PATCH] Fix tests for --incompatible_use_python_toolchains This adds a hook file for run_tests.sh to write toolchain info to before each bazel invocation. This replaces the legacy way of passing an interpreter in via --python_path. It also replaces a config_setting that was used to control whether PY2 or PY3 was used, with a simple constant symbol consumed at loading time. This is needed in order to make the target aware at analysis time of which version it is building for. Fixes #98, fixes #102. --- run_tests.sh | 105 ++++++++++++++++++++++++++++++++++++--- tests/BUILD | 38 ++++++-------- tests/package_f/f_PY3.py | 2 +- toolchain_test_hook.bzl | 4 ++ 4 files changed, 117 insertions(+), 32 deletions(-) create mode 100644 toolchain_test_hook.bzl diff --git a/run_tests.sh b/run_tests.sh index 25ffac1..21a259f 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -21,6 +21,78 @@ function die { exit 1 } +# This sets up the toolchain hook to run tests for the given version of Python +# whose interpreter is located at the given absolute path. +# +# $1 may be either "PY2" or "PY3". If it is PY2, then the Python 2 runtime is +# set to the path in $2, and the Python 3 runtime is set to the default value +# given by $PYTHON3. If $1 is PY3, then $2 is the Python 3 runtime and the +# Python 2 runtime is given by $PYTHON2. +# +# The PYVER constant is also set to $1. It is consumed at loading time by +# //tests:BUILD. +# +# Note that even though tests are only run for one version of Python at a time, +# we still need to provide both runtimes in the toolchain for the sake of +# tools. In particular, the par compiler itself requires PY2, even if the par +# that we are compiling uses PY3. +function set_toolchain_hook { + pyver=$1 + if [ $pyver == "PY3"]; then + py2_path="$PYTHON2" + py3_path="$2" + else + py2_path="$2" + py3_path="$PYTHON3" + fi + + cat > toolchain_test_hook.bzl << EOF +load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair") + +PYVER = "$pyver" + +def define_toolchain_for_testing(): + native.py_runtime( + name = "py2_runtime", + interpreter_path = "$py2_path", + python_version = "PY2", + ) + + native.py_runtime( + name = "py3_runtime", + interpreter_path = "$py3_path", + python_version = "PY3", + ) + + py_runtime_pair( + name = "runtime_pair_for_testing", + py2_runtime = ":py2_runtime", + py3_runtime = ":py3_runtime", + visibility = ["//visibility:public"], + ) + + native.toolchain( + name = "toolchain_for_testing", + toolchain = ":runtime_pair_for_testing", + toolchain_type = "@bazel_tools//tools/python:toolchain_type", + visibility = ["//visibility:public"], + ) +EOF +} + +# Clear the toolchain hook back to its original no-op contents. +# +# If the test exits abnormally and this function isn't run, we may be left with +# a modified version of this file in our source tree. +function clear_toolchain_hook { + cat > toolchain_test_hook.bzl << EOF +PYVER = "PY3" + +def define_toolchain_for_testing(): + pass +EOF +} + # Find various tools in environment PYTHON2=$(which python||true) if [ -z "${PYTHON2}" ]; then @@ -46,9 +118,18 @@ VIRTUALENVDIR=$(dirname $0)/.env # Virtualenv `activate` needs $PS1 set PS1='$ ' -# Must have at least one Python interpreter to test -if [ -z "${PYTHON2}" -a -z "${PYTHON3}" ]; then - die "Could not find Python 2 or 3 interpreter on $PATH" +# Must have both Python interpreters to test. +if [ -z "${PYTHON2}" ]; then + die "Could not find Python 2 on $PATH" +fi +if [ -z "${PYTHON3}" ]; then + die "Could not find Python 3 on $PATH" +fi + +# Must be able to locate toolchain_test_hook.bzl. This will fail if cwd is not +# the root of the subpar workspace. +if [ ! -f "toolchain_test_hook.bzl" ]; then + die "Could not locate toolchain_test_hook.bzl (are we in the workspace root?)" fi # Run test matrix @@ -57,15 +138,19 @@ for PYTHON_INTERPRETER in "${PYTHON2}" "${PYTHON3}"; do continue; fi + BAZEL_TEST="bazel test --test_output=errors \ +--incompatible_use_python_toolchains \ +--extra_toolchains=//tests:toolchain_for_testing" if [ "${PYTHON_INTERPRETER}" = "${PYTHON3}" ]; then - BAZEL_TEST="bazel test --define subpar_test_python_version=3" + PYVER="PY3" else - BAZEL_TEST="bazel test" + PYVER="PY2" fi echo "Testing ${PYTHON_INTERPRETER}" bazel clean - ${BAZEL_TEST} --python_path="${PYTHON_INTERPRETER}" --test_output=errors //... + set_toolchain_hook "$PYVER" "$PYTHON_INTERPRETER" + ${BAZEL_TEST} //... if [ -n "${VIRTUALENV}" ]; then echo "Testing bare virtualenv" @@ -76,7 +161,8 @@ for PYTHON_INTERPRETER in "${PYTHON2}" "${PYTHON3}"; do "${VIRTUALENVDIR}" source "${VIRTUALENVDIR}"/bin/activate bazel clean - ${BAZEL_TEST} --python_path=$(which python) --test_output=errors //... + set_toolchain_hook $PYVER $(which python) + ${BAZEL_TEST} //... deactivate for REQUIREMENTS in tests/requirements-test-*.txt; do @@ -88,8 +174,11 @@ for PYTHON_INTERPRETER in "${PYTHON2}" "${PYTHON3}"; do source "${VIRTUALENVDIR}"/bin/activate pip install -r "${REQUIREMENTS}" bazel clean - ${BAZEL_TEST} --python_path=$(which python) --test_output=errors //... + set_toolchain_hook $PYVER $(which python) + ${BAZEL_TEST} //... deactivate done fi done + +clear_toolchain_hook diff --git a/tests/BUILD b/tests/BUILD index 0e72f9d..f38083b 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -11,14 +11,13 @@ exports_files([ ]) load("//:subpar.bzl", "par_binary") +load("//:toolchain_test_hook.bzl", "PYVER", "define_toolchain_for_testing") -# Configurations -config_setting( - name = "subpar_test_python_version_3", - define_values = { - "subpar_test_python_version": "3", - }, -) +# Creates the target //tests:toolchain_for_testing. +# +# This hook is used by run_tests.sh to inject a Python toolchain into the build +# during tests. When not running tests this hook is a no-op. +define_toolchain_for_testing() # Utility targets sh_library( @@ -65,14 +64,11 @@ par_binary( par_binary( name = "package_f/f", - srcs = select({ - "subpar_test_python_version_3": ["package_f/f_PY3.py"], - "//conditions:default": ["package_f/f_PY2.py"], - }), - main = select({ - "subpar_test_python_version_3": "package_f/f_PY3.py", - "//conditions:default": "package_f/f_PY2.py", - }), + # We need to use the PYVER constant defined by the test hook, rather than + # select(), because python_version is not a configurable attribute. + srcs = ["package_f/f_PY3.py"] if PYVER == "PY3" else ["package_f/f_PY2.py"], + main = "package_f/f_PY3.py" if PYVER == "PY3" else "package_f/f_PY2.py", + python_version = "PY3" if PYVER == "PY3" else "PY2" ) par_binary( @@ -151,16 +147,12 @@ par_binary( args = [ "--par", "%s.par" % path, - ] + select({ - "subpar_test_python_version_3": ["%s_PY3_filelist.txt" % path], - "//conditions:default": ["%s_PY2_filelist.txt" % path], - }), + ("%s_PY3_filelist.txt" % path) if PYVER == "PY3" else ("%s_PY2_filelist.txt" % path) + ], data = [ "%s.par" % label, - ] + select({ - "subpar_test_python_version_3": ["%s_PY3_filelist.txt" % label], - "//conditions:default": ["%s_PY2_filelist.txt" % label], - }), + ("%s_PY3_filelist.txt" % label) if PYVER == "PY3" else ("%s_PY2_filelist.txt" % label) + ], ), ) for name, label, path in [ ("basic", "//tests/package_a:a", "tests/package_a/a"), diff --git a/tests/package_f/f_PY3.py b/tests/package_f/f_PY3.py index c5ac97d..5b42e49 100644 --- a/tests/package_f/f_PY3.py +++ b/tests/package_f/f_PY3.py @@ -22,7 +22,7 @@ def main(): assert sys.version_info.major == 3, sys.version - print('In f_PY2.py main()') + print('In f_PY3.py main()') if __name__ == '__main__': diff --git a/toolchain_test_hook.bzl b/toolchain_test_hook.bzl new file mode 100644 index 0000000..c2e3d7d --- /dev/null +++ b/toolchain_test_hook.bzl @@ -0,0 +1,4 @@ +PYVER = "PY3" + +def define_toolchain_for_testing(): + pass