Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI] [Minimal install] Check python version in minimal install #36887

Merged
merged 7 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ci/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -743,10 +743,13 @@ build() {
}

run_minimal_test() {
EXPECTED_PYTHON_VERSION=$1
BAZEL_EXPORT_OPTIONS="$(./ci/run/bazel_export_options)"
# Ignoring shellcheck is necessary because if ${BAZEL_EXPORT_OPTIONS} is wrapped by the double quotation,
# bazel test cannot recognize the option.
# shellcheck disable=SC2086
bazel test --test_output=streamed --config=ci --test_env=RAY_MINIMAL=1 --test_env=EXPECTED_PYTHON_VERSION=$EXPECTED_PYTHON_VERSION ${BAZEL_EXPORT_OPTIONS} python/ray/tests/test_minimal_install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of an "easier" approach where we just asserted the python version in shell here like check the python --version output.

What's the motif for running another pytest and assert in there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how bazel works, so wanted to verify that bazel picks up right version. If someone confident in bazel behavior says we can trust it, I can delete the test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion here - I' m ok with the current approach.

# shellcheck disable=SC2086
bazel test --test_output=streamed --config=ci --test_env=RAY_MINIMAL=1 ${BAZEL_EXPORT_OPTIONS} python/ray/tests/test_basic
# shellcheck disable=SC2086
bazel test --test_output=streamed --config=ci --test_env=RAY_MINIMAL=1 --test_env=TEST_EXTERNAL_REDIS=1 ${BAZEL_EXPORT_OPTIONS} python/ray/tests/test_basic
Expand Down Expand Up @@ -787,7 +790,7 @@ test_minimal() {
./ci/env/install-minimal.sh "$1"
echo "Installed minimal dependencies."
./ci/env/env_info.sh
python ./ci/env/check_minimal_install.py
python ./ci/env/check_minimal_install.py --expected-python-version "$1"
run_minimal_test "$1"
}

Expand Down
27 changes: 27 additions & 0 deletions ci/env/check_minimal_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@

This is to ensure that tests with minimal dependencies are not tainted
by too many installed packages.

It also ensures the correct Python version.
"""

from typing import List
import argparse
import sys

# These are taken from `setup.py` for ray[default]
DEFAULT_BLACKLIST = [
Expand Down Expand Up @@ -47,5 +51,28 @@ def assert_packages_not_installed(blacklist: List[str]):
)


def assert_python_version(expected_python_version: str) -> None:
actual_major, actual_minor = sys.version_info[:2]
actual_version = f"{actual_major}.{actual_minor}"
expected_version = expected_python_version.strip()
assert expected_version == actual_version, (
f"Expected Python version expected_version={expected_version}, "
f"actual_version={actual_version}"
)


if __name__ == "__main__":

parser = argparse.ArgumentParser()
parser.add_argument(
"--expected-python-version",
type=str,
help="Expected Python version in MAJOR.MINOR format, e.g. 3.11",
default=None,
)
args = parser.parse_args()

assert_packages_not_installed(DEFAULT_BLACKLIST)

if args.expected_python_version is not None:
assert_python_version(args.expected_python_version)
9 changes: 2 additions & 7 deletions ci/env/install-minimal.sh
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
#!/usr/bin/env bash

if [ "$1" == "3.11" ]; then
# TODO: fix build wheels unsupported tags in the future
echo "'set -xe' not working for Python 3.11"
else
set -xe
fi
set -xe

# Python version can be specified as 3.7, 3.8, 3.9, etc..
if [ -z "$1" ]; then
Expand All @@ -32,7 +27,7 @@ echo "Python version is ${PYTHON_VERSION}"
ROOT_DIR=$(cd "$(dirname "$0")/$(dirname "$(test -L "$0" && readlink "$0" || echo "/")")" || exit; pwd)
WORKSPACE_DIR="${ROOT_DIR}/../.."

# Installs conda and python 3.7
# Installs conda and the specified python version
MINIMAL_INSTALL=1 PYTHON=${PYTHON_VERSION} "${WORKSPACE_DIR}/ci/env/install-dependencies.sh"

# Re-install Ray wheels
Expand Down
1 change: 1 addition & 0 deletions python/ray/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ py_test_module_list(
"test_metrics_agent_2.py",
"test_microbenchmarks.py",
"test_mini.py",
"test_minimal_install.py",
"test_numba.py",
"test_redis_tls.py",
"test_raylet_output.py",
Expand Down
36 changes: 36 additions & 0 deletions python/ray/tests/test_minimal_install.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# coding: utf-8
"""
Tests that are specific to minimal installations.
"""

import pytest
import os
import sys


@pytest.mark.skipif(
os.environ.get("RAY_MINIMAL", "0") != "1",
reason="Skip unless running in a minimal install.",
)
def test_correct_python_version():
"""
Validate that Bazel uses the correct Python version in our minimal tests.
"""
expected_python_version = os.environ.get("EXPECTED_PYTHON_VERSION", "").strip()
assert (
expected_python_version
), f"EXPECTED_PYTHON_VERSION is {expected_python_version}"

actual_major, actual_minor = sys.version_info[:2]
actual_version = f"{actual_major}.{actual_minor}"
assert actual_version == expected_python_version, (
f"expected_python_version={expected_python_version}"
f"actual_version={actual_version}"
)


if __name__ == "__main__":
if os.environ.get("PARALLEL_CI"):
sys.exit(pytest.main(["-n", "auto", "--boxed", "-vs", __file__]))
else:
sys.exit(pytest.main(["-sv", __file__]))