Skip to content

Commit

Permalink
Bump pytest from 5.4.3 to 7.0.1 (ray-project#26334)
Browse files Browse the repository at this point in the history
See ray-project#23676 for context. This is another attempt at that as I figured out what's going wrong in `bazel test`. Supersedes ray-project#24828.

Now that there are Python 3.10 wheels for Ray 1.13 and this is no longer a blocker for supporting Python 3.10, I still want to make `bazel test //python/ray/tests/...` work for developing in a 3.10 env, and make it easier to add Python 3.10 tests to CI in future.

The change contains three commits with rather descriptive commit message, which I repeat here:

Pass deps to py_test in py_test_module_list

    Bazel macro py_test_module_list takes a `deps` argument, but completely
    ignores it instead of passes it to `native.py_test`. Fixing that as we
    are going to use deps of py_test_module_list in BUILD in later changes.

    cpp/BUILD.bazel depends on the broken behaviour: it deps-on a cc_library
    from a py_test, which isn't working, see upstream issue:
    bazelbuild/bazel#701.
    This is fixed by simply removing the (non-working) deps.

Depend on conftest and data files in Python tests BUILD files

    Bazel requires that all the files used in a test run should be
    represented in the transitive dependencies specified for the test
    target. For py_test, it means srcs, deps and data.

    Bazel enforces this constraint by creating a "runfiles" directory,
    symbolic links files in the dependency closure and run the test in the
    "runfiles" directory, so that the test shouldn't see files not in the
    dependency graph.

    Unfortunately, the constraint does not apply for a large number of
    Python tests, due to pytest (>=3.9.0, <6.0) resolving these symbolic
    links during test collection and effectively "breaks out" of the
    runfiles tree.

    pytest >= 6.0 introduces a breaking change and removed the symbolic link
    resolving behaviour, see pytest pull request
    pytest-dev/pytest#6523 for more context.

    Currently, we are underspecifying dependencies in a lot of BUILD files
    and thus blocking us from updating to newer pytest (for Python 3.10
    support). This change hopefully fixes all of them, and at least those in
    CI, by adding data or source dependencies (mostly for conftest.py-s)
    where needed.

Bump pytest version from 5.4.3 to 7.0.1

    We want at least pytest 6.2.5 for Python 3.10 support, but not past
    7.1.0 since it drops Python 3.6 support (which Ray still supports), thus
    the version constraint is set to <7.1.

    Updating pytest, combined with earlier BUILD fixes, changed the ground
    truth of a few error message based unit test, these tests are updated to
    reflect the change.

    There are also two small drive-by changes for making test_traceback and
    test_cli pass under Python 3.10. These are discovered while debugging CI
    failures (on earlier Python) with a Python 3.10 install locally.  Expect
    more such issues when adding Python 3.10 to CI.

Signed-off-by: Stefan van der Kleij <[email protected]>
  • Loading branch information
Riatre authored and Stefan van der Kleij committed Aug 18, 2022
1 parent d0d6a9f commit 3e8b259
Show file tree
Hide file tree
Showing 23 changed files with 215 additions and 119 deletions.
3 changes: 2 additions & 1 deletion bazel/python.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ load("@bazel_skylib//lib:paths.bzl", "paths")
# py_test_module_list creates a py_test target for each
# Python file in `files`

def py_test_module_list(files, size, deps, extra_srcs, name_suffix="", **kwargs):
def py_test_module_list(files, size, deps, extra_srcs=[], name_suffix="", **kwargs):
for file in files:
# remove .py
name = paths.split_extension(file)[0] + name_suffix
Expand All @@ -14,6 +14,7 @@ def py_test_module_list(files, size, deps, extra_srcs, name_suffix="", **kwargs)
size = size,
main = file,
srcs = extra_srcs + [file],
deps = deps,
**kwargs
)

Expand Down
4 changes: 2 additions & 2 deletions ci/build/test-wheels.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ if [[ "$platform" == "linux" ]]; then
"$PYTHON_EXE" -u -c "import ray; print(ray.__commit__)" | grep "$TRAVIS_COMMIT" || (echo "ray.__commit__ not set properly!" && exit 1)

# Install the dependencies to run the tests.
"$PIP_CMD" install -q aiohttp aiosignal frozenlist grpcio pytest==5.4.3 requests proxy.py
"$PIP_CMD" install -q aiohttp aiosignal frozenlist grpcio 'pytest==7.0.1' requests proxy.py

# Run a simple test script to make sure that the wheel works.
for SCRIPT in "${TEST_SCRIPTS[@]}"; do
Expand Down Expand Up @@ -117,7 +117,7 @@ elif [[ "$platform" == "macosx" ]]; then
"$PIP_CMD" install -q "$PYTHON_WHEEL"

# Install the dependencies to run the tests.
"$PIP_CMD" install -q aiohttp aiosignal frozenlist grpcio pytest==5.4.3 requests proxy.py
"$PIP_CMD" install -q aiohttp aiosignal frozenlist grpcio 'pytest==7.0.1' requests proxy.py

# Run a simple test script to make sure that the wheel works.
for SCRIPT in "${TEST_SCRIPTS[@]}"; do
Expand Down
4 changes: 2 additions & 2 deletions ci/env/install-minimal.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ eval "${WORKSPACE_DIR}/ci/ci.sh build"

# Install test requirements
python -m pip install -U \
pytest==5.4.3 \
pytest==7.0.1 \
numpy

# Train requirements.
# TODO: make this dynamic
if [ "${TRAIN_MINIMAL_INSTALL-}" = 1 ]; then
python -m pip install -U "ray[tune]"
fi
fi
4 changes: 1 addition & 3 deletions cpp/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,5 @@ py_test_module_list(
"small_size_python_tests",
"team:core",
],
deps = [
":ray_api",
],
deps = [],
)
17 changes: 17 additions & 0 deletions dashboard/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,41 @@ py_library(
),
)

py_library(
name = "conftest",
srcs = ["tests/conftest.py"],
deps = ["//python/ray/tests:conftest"],
)

py_test_run_all_subdirectory(
size = "medium",
include = ["**/test*.py"],
exclude = ["modules/test/**", "modules/node/tests/test_node.py", "tests/test_dashboard.py"],
extra_srcs = [],
data = [
"modules/job/tests/backwards_compatibility_scripts/test_backwards_compatibility.sh",
"modules/job/tests/pip_install_test-0.5-py3-none-any.whl",
"modules/snapshot/snapshot_schema.json",
"modules/tests/test_config_files/basic_runtime_env.yaml",
] + glob([
"modules/job/tests/subprocess_driver_scripts/*.py",
]),
deps = [":conftest"],
tags = ["exclusive", "team:serve"],
)

py_test(
name = "test_node",
size = "medium",
srcs = ["modules/node/tests/test_node.py"],
deps = [":conftest"],
tags = ["exclusive", "team:serve"],
)

py_test(
name = "test_dashboard",
size = "medium",
srcs = ["tests/test_dashboard.py"],
deps = [":conftest"],
tags = ["exclusive", "team:serve"],
)
5 changes: 5 additions & 0 deletions python/ray/autoscaler/aws/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
filegroup(
name = "example",
data = glob(["example-*.yaml"]),
visibility = ["//python/ray/tests:__pkg__"],
)
5 changes: 5 additions & 0 deletions python/ray/autoscaler/azure/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
filegroup(
name = "example",
data = glob(["example-*.yaml"]),
visibility = ["//python/ray/tests:__pkg__"],
)
5 changes: 5 additions & 0 deletions python/ray/autoscaler/gcp/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
filegroup(
name = "example",
data = glob(["example-*.yaml"]),
visibility = ["//python/ray/tests:__pkg__"],
)
5 changes: 5 additions & 0 deletions python/ray/autoscaler/local/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
filegroup(
name = "example",
data = glob(["example-*.yaml"]),
visibility = ["//python/ray/tests:__pkg__"],
)
16 changes: 7 additions & 9 deletions python/ray/data/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,18 @@
# --------------------------------------------------------------------
load("//bazel:python.bzl", "py_test_module_list")

SRCS = [] + select({
"@bazel_tools//src/conditions:windows": glob([
"**/conftest.py",
]),
"//conditions:default": [],
})
py_library(
name = "conftest",
srcs = ["tests/conftest.py"],
deps = ["//python/ray/tests:conftest"],
)

py_test(
name = "test_preprocessors",
size = "small",
srcs = ["tests/test_preprocessors.py"],
tags = ["team:ml", "exclusive", "ray_air"],
deps = ["//:ray_lib"],
deps = ["//:ray_lib", ":conftest"],
)

py_test_module_list(
Expand All @@ -26,7 +25,6 @@ py_test_module_list(
exclude=["tests/test_preprocessors.py"]
),
size = "large",
extra_srcs = SRCS,
tags = ["team:core", "exclusive"],
deps = ["//:ray_lib"],
deps = ["//:ray_lib", ":conftest"],
)
9 changes: 9 additions & 0 deletions python/ray/experimental/packaging/example_pkg/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
filegroup(
name = "example_pkg",
data = [
"ray_pkg.yaml",
] + glob([
"my_pkg/**/*.py",
]),
visibility = ["//python/ray/tests:__pkg__"],
)
6 changes: 6 additions & 0 deletions python/ray/serve/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ py_library(

serve_tests_srcs = glob(["tests/**/*.py"])

filegroup(
name = "test_config_files",
data = glob(["tests/test_config_files/**/*"]),
)

py_test(
name = "test_api",
size = "medium",
Expand Down Expand Up @@ -313,6 +318,7 @@ py_test(
srcs = serve_tests_srcs,
tags = ["exclusive", "team:serve"],
deps = [":serve_lib"],
data = [":test_config_files"],
)

py_test(
Expand Down
Loading

0 comments on commit 3e8b259

Please sign in to comment.