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][microcheck/11] move get_changed_tests to Test class #45499

Merged
merged 1 commit into from
May 24, 2024
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
30 changes: 2 additions & 28 deletions ci/ray_ci/test_tester.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
_get_flaky_test_targets,
_get_tag_matcher,
_get_new_tests,
_get_changed_files,
_get_changed_tests,
_get_human_specified_tests,
)
from ray_release.test import Test, TestState
Expand Down Expand Up @@ -128,7 +126,7 @@ def test_get_test_targets() -> None:
"ray_release.test.Test.gen_high_impact_tests",
return_value={"step": test_objects},
), mock.patch(
"ci.ray_ci.tester._get_changed_tests",
"ray_release.test.Test.get_changed_tests",
return_value=set(),
), mock.patch(
"ci.ray_ci.tester._get_new_tests",
Expand Down Expand Up @@ -256,7 +254,7 @@ def test_get_high_impact_test_targets() -> None:
"ci.ray_ci.tester._get_new_tests",
return_value=test["new_tests"],
), mock.patch(
"ci.ray_ci.tester._get_changed_tests",
"ray_release.test.Test.get_changed_tests",
return_value=test["changed_tests"],
), mock.patch(
"ci.ray_ci.tester._get_human_specified_tests",
Expand Down Expand Up @@ -285,30 +283,6 @@ def test_get_new_tests(mock_gen_from_s3, mock_run_script_with_output) -> None:
) == {"//new_test"}


@mock.patch.dict(
os.environ,
{"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "base", "BUILDKITE_COMMIT": "commit"},
)
@mock.patch("subprocess.check_call")
@mock.patch("subprocess.check_output")
def test_get_changed_files(mock_check_output, mock_check_call) -> None:
mock_check_output.return_value = b"file1\nfile2\n"
assert _get_changed_files() == {"file1", "file2"}


@mock.patch("ci.ray_ci.tester._get_test_targets_per_file")
@mock.patch("ci.ray_ci.tester._get_changed_files")
def test_get_changed_tests(
mock_get_changed_files, mock_get_test_targets_per_file
) -> None:
mock_get_changed_files.return_value = {"test_src", "build_src"}
mock_get_test_targets_per_file.side_effect = (
lambda x: {"//t1", "//t2"} if x == "test_src" else {}
)

assert _get_changed_tests() == {"//t1", "//t2"}


@mock.patch.dict(
os.environ,
{"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "base", "BUILDKITE_COMMIT": "commit"},
Expand Down
61 changes: 1 addition & 60 deletions ci/ray_ci/tester.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ def _get_high_impact_test_targets(
if test.get_oncall() == team
}
new_tests = _get_new_tests(os_prefix, container)
changed_tests = _get_changed_tests()
changed_tests = Test.get_changed_tests(bazel_workspace_dir)
human_specified_tests = _get_human_specified_tests()

return (
Expand Down Expand Up @@ -464,65 +464,6 @@ def _get_new_tests(prefix: str, container: TesterContainer) -> Set[str]:
return local_test_targets.difference(db_test_targets)


def _get_changed_tests() -> Set[str]:
"""
Get all changed tests in the current PR
"""
changed_files = _get_changed_files()
logger.info(f"Changed files: {changed_files}")
return set(
itertools.chain.from_iterable(
[_get_test_targets_per_file(file) for file in _get_changed_files()]
)
)


def _get_test_targets_per_file(file: str) -> Set[str]:
"""
Get the test target from a file path
"""
try:
package = (
subprocess.check_output(["bazel", "query", file], cwd=bazel_workspace_dir)
.decode()
.strip()
)
if not package:
return set()
targets = subprocess.check_output(
["bazel", "query", f"tests(attr('srcs', {package}, //...))"],
cwd=bazel_workspace_dir,
)
targets = {
target.strip()
for target in targets.decode().splitlines()
if target is not None
}
logger.info(f"Found test targets for file {file}: {targets}")

return targets
except subprocess.CalledProcessError:
logger.info(f"File {file} is not a test target")
return set()


def _get_changed_files() -> Set[str]:
"""
Get all changed files in the current PR
"""
base = os.environ.get("BUILDKITE_PULL_REQUEST_BASE_BRANCH")
head = os.environ.get("BUILDKITE_COMMIT")
if not base or not head:
# if not in a PR, return an empty set
return set()

changes = subprocess.check_output(
["git", "diff", "--name-only", f"origin/{base}...{head}"],
cwd=bazel_workspace_dir,
)
return {file.strip() for file in changes.decode().splitlines() if file is not None}


def _get_flaky_test_targets(
team: str, operating_system: str, yaml_dir: Optional[str] = None
) -> List[str]:
Expand Down
67 changes: 66 additions & 1 deletion release/ray_release/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import enum
import os
import platform
import subprocess
import json
import time
from itertools import chain
from typing import Awaitable, Optional, List, Dict
from typing import Awaitable, Optional, List, Dict, Set
from dataclasses import dataclass

import aioboto3
Expand Down Expand Up @@ -220,6 +221,70 @@ def gen_high_impact_tests(cls, prefix: str) -> Dict[str, List]:

return step_id_to_tests

@classmethod
def get_changed_tests(cls, bazel_workspace_dir: str) -> Set[str]:
"""
Get all changed tests in the current PR
"""
return set(
chain.from_iterable(
[
cls._get_test_targets_per_file(file, bazel_workspace_dir)
for file in cls._get_changed_files(bazel_workspace_dir)
]
)
)

@classmethod
def _get_changed_files(cls, bazel_workspace_dir: str) -> Set[str]:
"""
Get all changed files in the current PR
"""
base = os.environ.get("BUILDKITE_PULL_REQUEST_BASE_BRANCH")
head = os.environ.get("BUILDKITE_COMMIT")
if not base or not head:
# if not in a PR, return an empty set
return set()

changes = subprocess.check_output(
["git", "diff", "--name-only", f"origin/{base}...{head}"],
cwd=bazel_workspace_dir,
)
return {
file.strip() for file in changes.decode().splitlines() if file is not None
}

@classmethod
def _get_test_targets_per_file(
cls, file: str, bazel_workspace_dir: str
) -> Set[str]:
"""
Get the test target from a file path
"""
try:
package = (
subprocess.check_output(
["bazel", "query", file], cwd=bazel_workspace_dir
)
.decode()
.strip()
)
if not package:
return set()
targets = subprocess.check_output(
["bazel", "query", f"tests(attr('srcs', {package}, //...))"],
cwd=bazel_workspace_dir,
)
targets = {
target.strip()
for target in targets.decode().splitlines()
if target is not None
}

return targets
except subprocess.CalledProcessError:
return set()

def is_jailed_with_open_issue(self, ray_github: Repository) -> bool:
"""
Returns whether this test is jailed with open issue.
Expand Down
24 changes: 24 additions & 0 deletions release/ray_release/tests/test_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,5 +384,29 @@ def test_get_test_target():
assert Test({"name": input}).get_target() == output


@mock.patch.dict(
os.environ,
{"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "base", "BUILDKITE_COMMIT": "commit"},
)
@mock.patch("subprocess.check_call")
@mock.patch("subprocess.check_output")
def test_get_changed_files(mock_check_output, mock_check_call) -> None:
mock_check_output.return_value = b"file1\nfile2\n"
assert Test._get_changed_files("") == {"file1", "file2"}


@mock.patch("ray_release.test.Test._get_test_targets_per_file")
@mock.patch("ray_release.test.Test._get_changed_files")
def test_get_changed_tests(
mock_get_changed_files, mock_get_test_targets_per_file
) -> None:
mock_get_changed_files.return_value = {"test_src", "build_src"}
mock_get_test_targets_per_file.side_effect = (
lambda x, _: {"//t1", "//t2"} if x == "test_src" else {}
)

assert Test.get_changed_tests("") == {"//t1", "//t2"}


if __name__ == "__main__":
sys.exit(pytest.main(["-v", __file__]))
Loading