Skip to content

Commit

Permalink
Improve test compatibility with Python toolchain flag
Browse files Browse the repository at this point in the history
This rolls-forward the parts of bf66dc7 that made our tests compatible with --incompatible_use_python_toolchains, without actually flipping the flag. The flag is set to default true within our test setup.

Work toward #7899.

TESTED=Confirmed that it doesn't break mac postsubmit: https://buildkite.com/bazel/bazel-bazel/builds/8065
PiperOrigin-RevId: 246404708
  • Loading branch information
brandjon authored and copybara-github committed May 2, 2019
1 parent 8effdb1 commit 29f1932
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ private String getInterpreterPathFromStub(ConfiguredTarget pyExecutableTarget) {
"Failed to find the '%python_binary%' key in the stub script's template expansion action");
}

// TODO(#8169): Delete tests of the legacy --python_top / --python_path behavior.

@Test
public void runtimeSetByPythonTop() throws Exception {
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ public void pythonTop() throws Exception {
scratch.file(
"a/BUILD",
"py_runtime(name='b', files=[], interpreter='c')");
BazelPythonConfiguration config = create("--python_top=//a:b")
.getFragment(BazelPythonConfiguration.class);
BazelPythonConfiguration config =
create("--incompatible_use_python_toolchains=false", "--python_top=//a:b")
.getFragment(BazelPythonConfiguration.class);
assertThat(config.getPythonTop()).isNotNull();
}

Expand All @@ -43,7 +44,7 @@ public void pythonTop_malformedLabel() {
OptionsParsingException expected =
assertThrows(
OptionsParsingException.class,
() -> create("--python_top=//a:!b:"));
() -> create("--incompatible_use_python_toolchains=false", "--python_top=//a:!b:"));
assertThat(expected).hasMessageThat().contains("While parsing option --python_top");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,24 @@ public void setup(MockToolsConfig config) throws IOException {
"toolchain_type(name = 'toolchain_type')",
"constraint_setting(name = 'py2_interpreter_path')",
"constraint_setting(name = 'py3_interpreter_path')",
"py_runtime_pair(name = 'dummy_py_runtime_pair')",
"py_runtime(",
" name = 'py2_interpreter',",
" interpreter_path = '/usr/bin/mockpython2',",
" python_version = 'PY2',",
")",
"py_runtime(",
" name = 'py3_interpreter',",
" interpreter_path = '/usr/bin/mockpython3',",
" python_version = 'PY3',",
")",
"py_runtime_pair(",
" name = 'default_py_runtime_pair',",
" py2_runtime = ':py2_interpreter',",
" py3_runtime = ':py3_interpreter',",
")",
"toolchain(",
" name = 'dummy_toolchain',",
" toolchain = ':dummy_py_runtime_pair',",
" name = 'default_python_toolchain',",
" toolchain = ':default_py_runtime_pair',",
" toolchain_type = ':toolchain_type',",
")",
"exports_files(['precompile.py'])",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib:python-rules",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/test/java/com/google/devtools/build/lib:analysis_testutil",
"//src/test/java/com/google/devtools/build/lib:testutil",
"//third_party:junit4",
"//third_party:truth",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.testutil.TestConstants;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -174,6 +175,10 @@ public void runtimeSandwich() throws Exception {
")");
scratch.file(
"pkg/BUILD",
"load('"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/python:toolchain.bzl', "
+ "'py_runtime_pair')",
"load(':rules.bzl', 'userruntime')",
"py_runtime(",
" name = 'pyruntime',",
Expand All @@ -187,13 +192,22 @@ public void runtimeSandwich() throws Exception {
" interpreter = ':userintr',",
" files = ['userdata.txt'],",
")",
"py_runtime_pair(",
" name = 'userruntime_pair',",
" py2_runtime = 'userruntime',",
")",
"toolchain(",
" name = 'usertoolchain',",
" toolchain = ':userruntime_pair',",
" toolchain_type = '"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/python:toolchain_type',",
")",
"py_binary(",
" name = 'pybin',",
" srcs = ['pybin.py'],",
")");
String pythonTopLabel =
analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:userruntime");
useConfiguration("--python_top=" + pythonTopLabel);
useConfiguration("--extra_toolchains=//pkg:usertoolchain");
ConfiguredTarget target = getConfiguredTarget("//pkg:pybin");
assertThat(collectRunfiles(target))
.containsAtLeast(getSourceArtifact("pkg/data.txt"), getSourceArtifact("pkg/userdata.txt"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ public static void processSkyframeExecutorForTesting(SkyframeExecutor skyframeEx
// TODO(#7903): Remove once our own tests are migrated.
"--incompatible_py3_is_default=false",
"--incompatible_py2_outputs_are_suffixed=false",
// TODO(#7899): Remove once we flip the flag default.
"--incompatible_use_python_toolchains=true",
// TODO(#7849): Remove after flag flip.
"--incompatible_use_toolchain_resolution_for_java_rules");

Expand Down
8 changes: 7 additions & 1 deletion src/test/py/bazel/runfiles_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,14 @@ def _AssertRunfilesLibraryInBazelToolsRepo(self, family, lang_name):
self.AssertExitCode(exit_code, 0, stderr)
bazel_bin = stdout[0]

# TODO(brandjon): (Issue #8169) Make this test compatible with Python
# toolchains. Blocked on the fact that there's no PY3 environment on our Mac
# workers (bazelbuild/continuous-integration#578).
exit_code, _, stderr = self.RunBazel([
"build", "--verbose_failures", "//foo:runfiles-" + family
"build",
"--verbose_failures",
"--incompatible_use_python_toolchains=false",
"//foo:runfiles-" + family
])
self.AssertExitCode(exit_code, 0, stderr)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ fail_if_no_android_sdk
source "${CURRENT_DIR}/../../integration_test_setup.sh" \
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }

# TODO(#8169): Make this test compatible with Python toolchains. Blocked on the
# fact that there's no PY3 environment on our Mac workers
# (bazelbuild/continuous-integration#578).
add_to_bazelrc "build --incompatible_use_python_toolchains=false"

function setup_android_instrumentation_test_env() {
mkdir -p java/com/bin/res/values
mkdir -p javatests/com/bin
Expand Down
69 changes: 21 additions & 48 deletions src/test/shell/bazel/python_version_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ fi
source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }

# TODO(bazelbuild/continuous-integration#578): Enable this test for Mac and
# Windows.

# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux".
# `tr` converts all upper case letters to lower case.
# `case` matches the result if the `uname | tr` expression to string prefixes
Expand All @@ -55,15 +52,15 @@ case "$(uname -s | tr [:upper:] [:lower:])" in
msys*)
# As of 2018-08-14, Bazel on Windows only supports MSYS Bash.
declare -r is_windows=true
# As of 2018-12-17, this test is disabled on windows (via "no_windows" tag),
# so this code shouldn't even have run. See the TODO at
# use_system_python_2_3_runtimes.
# As of 2019-04-26, this test is disabled on Windows (via "no_windows" tag),
# so this code shouldn't even have run.
# TODO(bazelbuild/continuous-integration#578): Enable this test for Windows.
fail "This test does not run on Windows."
;;
darwin*)
# As of 2018-12-17, this test is disabled on mac, but there's no "no_mac" tag
# so we just have to trivially succeed. See the TODO at
# use_system_python_2_3_runtimes.
# As of 2019-04-26, this test is disabled on mac, but there's no "no_mac" tag
# so we just have to trivially succeed.
# TODO(bazelbuild/continuous-integration#578): Enable this test for Mac.
echo "This test does not run on Mac; exiting early." >&2
exit 0
;;
Expand All @@ -79,44 +76,6 @@ if "$is_windows"; then
export MSYS2_ARG_CONV_EXCL="*"
fi

# Use a py_runtime that invokes either the system's Python 2 or Python 3
# interpreter based on the Python mode. On Unix this is a workaround for #4815.
#
# TODO(brandjon): Replace this with the autodetecting Python toolchain.
function use_system_python_2_3_runtimes() {
PYTHON2_BIN=$(which python2 || echo "")
PYTHON3_BIN=$(which python3 || echo "")
# Debug output.
echo "Python 2 interpreter: ${PYTHON2_BIN:-"Not found"}"
echo "Python 3 interpreter: ${PYTHON3_BIN:-"Not found"}"
# Fail if either isn't present.
if [[ -z "${PYTHON2_BIN:-}" || -z "${PYTHON3_BIN:-}" ]]; then
fail "Can't use system interpreter: Could not find one or both of \
'python2', 'python3'"
fi

# Point Python builds at a py_runtime target defined in a //tools package of
# the main repo. This is not related to @bazel_tools//tools/python.
add_to_bazelrc "build --python_top=//tools/python:default_runtime"

mkdir -p tools/python

cat > tools/python/BUILD << EOF
package(default_visibility=["//visibility:public"])
py_runtime(
name = "default_runtime",
files = [],
interpreter_path = select({
"@bazel_tools//tools/python:PY2": "${PYTHON2_BIN}",
"@bazel_tools//tools/python:PY3": "${PYTHON3_BIN}",
}),
)
EOF
}

use_system_python_2_3_runtimes

#### TESTS #############################################################

# Sanity test that our environment setup above works.
Expand Down Expand Up @@ -202,6 +161,8 @@ function test_build_python_zip_works_with_py_runtime() {
mkdir -p test

cat > test/BUILD << EOF
load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair")
py_binary(
name = "pybin",
srcs = ["pybin.py"],
Expand All @@ -212,6 +173,17 @@ py_runtime(
interpreter = ":mockpy.sh",
python_version = "PY3",
)
py_runtime_pair(
name = "mock_runtime_pair",
py3_runtime = ":mock_runtime",
)
toolchain(
name = "mock_toolchain",
toolchain = ":mock_runtime_pair",
toolchain_type = "@bazel_tools//tools/python:toolchain_type",
)
EOF
cat > test/pybin.py << EOF
# This doesn't actually run because we use a mock Python runtime that never
Expand All @@ -224,7 +196,8 @@ echo "I am mockpy!"
EOF
chmod u+x test/mockpy.sh

bazel run //test:pybin --python_top=//test:mock_runtime --build_python_zip \
bazel run //test:pybin \
--extra_toolchains=//test:mock_toolchain --build_python_zip \
&> $TEST_log || fail "bazel run failed"
expect_log "I am mockpy!"
}
Expand Down
8 changes: 6 additions & 2 deletions src/test/shell/integration/runfiles_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,18 @@ msys*|mingw*|cygwin*)
;;
esac

# We disable Python toolchains in EXTRA_BUILD_FLAGS because it throws off the
# counts and manifest checks in test_foo_runfiles.
# TODO(#8169): Update this test and remove the toolchain opt-out.
if "$is_windows"; then
export MSYS_NO_PATHCONV=1
export MSYS2_ARG_CONV_EXCL="*"
export EXT=".exe"
export EXTRA_BUILD_FLAGS="--enable_runfiles --build_python_zip=0"
export EXTRA_BUILD_FLAGS="--incompatible_use_python_toolchains=false \
--enable_runfiles --build_python_zip=0"
else
export EXT=""
export EXTRA_BUILD_FLAGS=""
export EXTRA_BUILD_FLAGS="--incompatible_use_python_toolchains=false"
fi

#### SETUP #############################################################
Expand Down
37 changes: 27 additions & 10 deletions src/test/shell/testenv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,9 @@ common --show_progress_rate_limit=-1
# Disable terminal-specific features.
common --color=no --curses=no
# TODO(#7899): Remove once we flip the flag default.
build --incompatible_use_python_toolchains=true
${EXTRA_BAZELRC:-}
EOF

Expand Down Expand Up @@ -569,11 +572,13 @@ function use_fake_python_runtimes_for_testsuite() {
PYTHON3_FILENAME="python3.sh"
fi

add_to_bazelrc "build --python_top=//tools/python:default_runtime"
add_to_bazelrc "build --extra_toolchains=//tools/python:fake_python_toolchain"

mkdir -p tools/python

cat > tools/python/BUILD << EOF
load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair")
package(default_visibility=["//visibility:public"])
sh_binary(
Expand All @@ -582,15 +587,27 @@ sh_binary(
)
py_runtime(
name = "default_runtime",
files = select({
"@bazel_tools//tools/python:PY3": [":${PYTHON3_FILENAME}"],
"//conditions:default": [":${PYTHON2_FILENAME}"],
}),
interpreter = select({
"@bazel_tools//tools/python:PY3": ":${PYTHON3_FILENAME}",
"//conditions:default": ":${PYTHON2_FILENAME}",
}),
name = "fake_py2_interpreter",
interpreter = ":${PYTHON2_FILENAME}",
python_version = "PY2",
)
py_runtime(
name = "fake_py3_interpreter",
interpreter = ":${PYTHON3_FILENAME}",
python_version = "PY3",
)
py_runtime_pair(
name = "fake_py_runtime_pair",
py2_runtime = ":fake_py2_interpreter",
py3_runtime = ":fake_py3_interpreter",
)
toolchain(
name = "fake_python_toolchain",
toolchain = ":fake_py_runtime_pair",
toolchain_type = "@bazel_tools//tools/python:toolchain_type",
)
EOF

Expand Down

0 comments on commit 29f1932

Please sign in to comment.