Skip to content

Commit

Permalink
Use ./pants.pex, not ./pants, to run internal integration tests (p…
Browse files Browse the repository at this point in the history
…antsbuild#8183)

## Changes to get `--chroot` working
As prework to running integration tests with `--chroot` and then [remoting](pantsbuild#8113), we first must make two major changes to how we handle integration tests:

1) Having integration tests depend on `//:build_root`. This ensures that `get_buildroot()` properly resolves to the chrooted folder in `.pants.d`, rather than traversing all the way up to the original build root.
2) Using `./pants.pex` rather than `./pants` in integration tests. Because V1 strips the prefix of source roots, whereby `src/python/pants` becomes `pants`, we cannot also have a file named `pants` in the same build root. Instead, we can safely depend on `./pants.pex`.

## Autogeneration of `pants.pex`
Now that we depend on `pants.pex`, it is very easy for this file to fall out-of-date and to unintentionally be testing old code.

We work around this via the design in pantsbuild#8209. First, we start caching built PEXes in `~/.cache/pants`, with the ID derived from all the source files used for Pants. This allows us to quickly switch between branches without having to regenerate the PEX every time.

Then, we teach `./pants` to run `bootstrap_pants_pex.sh` whenever `test` is one of the goals and it's not CI. This will ensure that the file is always up-to-date.
  • Loading branch information
Eric-Arellano authored Sep 2, 2019
1 parent 21d0011 commit 684e043
Show file tree
Hide file tree
Showing 12 changed files with 131 additions and 14 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ out/
/logs/
arc.sh
BUILD.release*
/pants.pex
pants.ini.sitegen
target
GPATH
Expand Down
9 changes: 8 additions & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,12 @@ files(

files(
name = 'pants_ini',
source = 'pants.ini'
source = 'pants.ini',
)

# NB: This is used for integration tests. This is generated automatically via `./pants` and
# `build-support/bin/bootstrap_pants_pex.sh`.
files(
name = 'pants_pex',
source = 'pants.pex',
)
57 changes: 57 additions & 0 deletions build-support/bin/bootstrap_pants_pex.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/usr/bin/env bash
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd ../.. && pwd -P)"

# This script is used to generate pants.pex and particularly to allow us to maintain multiple versions,
# each mapped to a particular snapshot of the source code. The different versions are maintained in
# the CACHE_ROOT, with the current one symlinked into the build root. This allows us to quickly
# change the specific pants.pex being used. This mechanism is similar to bootstrap_code.sh.

export PY="${PY:-python3}"

# shellcheck source=build-support/common.sh
source "${REPO_ROOT}/build-support/common.sh"
# shellcheck source=build-support/pants_venv
source "${REPO_ROOT}/build-support/pants_venv"
# shellcheck source=build-support/bin/native/bootstrap_code.sh
source "${REPO_ROOT}/build-support/bin/native/bootstrap_code.sh"

readonly PANTS_PEX_CACHE_DIR="${CACHE_ROOT}/bin/pants-pex"

function bootstrap_pants_pex() {
local pants_pex_version
pants_pex_version="$(calculate_pants_pex_current_hash)"
local target_binary="${PANTS_PEX_CACHE_DIR}/pants.${pants_pex_version}.pex"

if [[ ! -f "${target_binary}" ]]; then
log "pants.pex is outdated or does not yet exist. Bootstrapping..."
./pants --quiet binary src/python/pants/bin:pants_local_binary || exit 1

mkdir -p "$(dirname "${target_binary}")"
cp dist/pants_local_binary.pex "${target_binary}"
fi

# Ensure that `pants.pex` uses the correct version.
# NB: the V2 engine does not work if this is a symlink, so we must physically copy the file.
cp "${target_binary}" pants.pex
}

function calculate_pants_pex_current_hash() {
# NB: These folder names were found by getting all the dependencies for `pants.pex` by running
# `./pants dependencies --transitive src/python/pants/bin:pants_local_binary | sort`.
(
cd "${REPO_ROOT}" || exit 1
(uname
python --version 2>&1
git ls-files --cached --others --exclude-standard \
"${REPO_ROOT}/3rdparty/python" \
"${REPO_ROOT}/contrib/python/src/python/pants/contrib/python/checks" \
"${REPO_ROOT}/src/python" \
"${REPO_ROOT}/pants-plugins" \
| git hash-object --stdin-paths) | fingerprint_data
)
}

# Redirect to ensure that we don't interfere with stdout.
activate_pants_venv 1>&2
bootstrap_native_code 1>&2
bootstrap_pants_pex 1>&2
12 changes: 5 additions & 7 deletions build-support/bin/ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ def setup_python_interpreter(version: PythonVersion) -> None:


def set_run_from_pex() -> None:
# We want all invocations of ./pants (apart from the bootstrapping one above) to delegate
# to ./pants.pex, and not themselves attempt to bootstrap.
# In this file we invoke ./pants.pex directly anyway, but some of those invocations will run
# integration tests that shell out to `./pants`, so we set this env var for those cases.
# Even though our Python integration tests and commands in this file directly invoke `pants.pex`,
# some places like the JVM tests may still directly call the script `./pants`. When this happens,
# we want to ensure that the script immediately breaks out to `./pants.pex` to avoid
# re-bootstrapping Pants in CI.
os.environ["RUN_PANTS_FROM_PEX"] = "1"


Expand Down Expand Up @@ -213,9 +213,7 @@ def bootstrap(*, clean: bool, python_version: PythonVersion) -> None:
die("Failed to clean before bootstrapping Pants.")

try:
subprocess.run(["./pants", "binary", "src/python/pants/bin:pants_local_binary"], check=True)
Path("dist/pants_local_binary.pex").rename("pants.pex")
subprocess.run(["./pants.pex", "--version"], check=True)
subprocess.run(["./build-support/bin/bootstrap_pants_pex.sh"], check=True)
except subprocess.CalledProcessError:
die("Failed to bootstrap Pants.")

Expand Down
4 changes: 2 additions & 2 deletions build-support/bin/native/bootstrap_code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ function calculate_current_hash() {
echo "${RUST_TOOLCHAIN}"
uname
python --version 2>&1
git ls-files -c -o --exclude-standard \
git ls-files --cached --others --exclude-standard \
"${NATIVE_ROOT}" \
"${REPO_ROOT}/rust-toolchain" \
"${REPO_ROOT}/src/python/pants/engine/native.py" \
"${REPO_ROOT}/build-support/bin/native" \
"${REPO_ROOT}/3rdparty/python/requirements.txt" \
| grep -v -E -e "/BUILD$" -e "/[^/]*\.md$" \
| git hash-object -t blob --stdin-paths) | fingerprint_data
| git hash-object --stdin-paths) | fingerprint_data
)
}

Expand Down
2 changes: 1 addition & 1 deletion build-support/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function end_travis_section() {
}

function fingerprint_data() {
git hash-object -t blob --stdin
git hash-object --stdin
}

function git_merge_base() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ python_tests(
'3rdparty/python:parameterized',
'3rdparty/python:pex',
'3rdparty/python:wheel',
'//:build_root',
'//:pants_pex',
'build-support/regexes',
'contrib/python/src/python/pants/contrib/python/checks/tasks/checkstyle',
'src/python/pants/backend/python/subsystems',
Expand All @@ -17,5 +19,4 @@ python_tests(
'src/python/pants/util:dirutil',
'tests/python/pants_test/backend/python/tasks:python_task_test_base',
],
timeout=100,
)
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CheckstyleTest(PythonTaskTestBase):
def build_checker_wheel(root_dir: str) -> str:
target = Checkstyle._CHECKER_ADDRESS_SPEC
command = [
os.path.join(get_buildroot(), 'pants'),
os.path.join(get_buildroot(), 'pants.pex'),
f'--pants-distdir={root_dir}',
'setup-py',
'--run=bdist_wheel --universal',
Expand Down
20 changes: 20 additions & 0 deletions pants
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,24 @@ else
export PANTS_DEV=1
fi

# Integration tests depend on an up-to-date `pants.pex`. Here, we check if we're running tests
# and ensure the `pants.pex` is generated if so. We do this here, rather than via `pants-plugins`,
# because we want the file to be generated _before_ any tests run, not during that Pants invocation,
# as the dependency is marked via BUILD files so should not be constructed at runtime.
#
# Note that we have no way to distinguish between integration tests vs unit tests here, so we
# unfortunately rebuild the pants.pex for unit tests too.
#
# We do not do this when in CI because CI downloads the PEX from AWS and we never want a worker
# shard to try bootstrapping the PEX itself.
test_goal_used=false
for arg in "$@"; do
if [[ "${arg}" == 'test' || "${arg}" == 'test.*' ]]; then
test_goal_used=true
fi
done
if [[ "${test_goal_used}" == 'true' && "${TRAVIS}" != 'true' ]]; then
./build-support/bin/bootstrap_pants_pex.sh
fi

exec_pants_bare "$@"
2 changes: 2 additions & 0 deletions tests/java/org/pantsbuild/tools/runner/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ junit_tests(
'3rdparty:guava',
'3rdparty:jsr305',
'3rdparty:junit',
'//:build_root',
'//:pants_pex',
'src/java/org/pantsbuild/tools/runner:runner-library',
],
# This test needs to invoke ./pants at the buildroot.
Expand Down
2 changes: 2 additions & 0 deletions tests/python/pants_test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ python_library(
dependencies = [
'3rdparty/python:ansicolors',
'3rdparty/python:dataclasses',
'//:build_root',
'//:pants_pex',
'build-support/regexes',
'src/python/pants/base:build_environment',
'src/python/pants/base:build_file',
Expand Down
31 changes: 30 additions & 1 deletion tests/python/pants_test/pants_run_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from contextlib import contextmanager
from dataclasses import dataclass
from operator import eq, ne
from pathlib import Path
from threading import Lock
from typing import List, Optional, Union

Expand Down Expand Up @@ -172,7 +173,35 @@ def _read_log(filename):
class PantsRunIntegrationTest(unittest.TestCase):
"""A base class useful for integration tests for targets in the same repo."""

PANTS_SCRIPT_NAME = 'pants'
class InvalidTestEnvironmentError(Exception):
"""Raised when the external environment is not set up properly to run integration tests."""

@property
def PANTS_SCRIPT_NAME(self) -> str:
# We first try to depend on `pants.pex`, instead of `pants`. We do this to allow tests to work
# with --chroot and the V2 test runner / remoting. For both use cases, all dependencies must be
# explicitly declared via BUILD files, including any use of the file `./pants.pex` or `./pants.`
# However, we cannot explicitly depend on `./pants` via BUILD files! We strip the prefix of
# source roots, so `src/python/pants` becomes the directory `pants`
# (https://github.com/pantsbuild/pants/pull/8185). We cannot have a directory named `pants` and
# a file named `pants`. So, we can only safely depend explicitly on `pants.pex`.
#
# Nevertheless, we still use `./pants` as a fallback. Tests that use the fallback won't work
# with --chroot or the V2 test runner, but they will at least work with the default V1 test
# runner.
if Path('pants.pex').exists():
# NB: this is a generated file that gets automatically generated by `./pants` and
# `build-support/bin/bootstrap_pants_pex.sh`.
return 'pants.pex'
if Path('pants').is_file():
return 'pants'
raise self.InvalidTestEnvironmentError(
"Missing the file `pants.pex`. Be sure that this file exists at the build root, as "
"integration tests depend on it to run properly.\nAlternatively, integration tests will fall "
"back to using the script `pants` at the build root. However, using this fallback instead of "
"`pants.pex` is discouraged because your tests will not work with `--chroot` or the V2 test "
"runner."
)

@classmethod
def use_pantsd_env_var(cls):
Expand Down

0 comments on commit 684e043

Please sign in to comment.