Skip to content

Commit

Permalink
[ci][microcheck] recover the logic to compute new tests (ray-project#…
Browse files Browse the repository at this point in the history
…45495)

ray-project#45462 adds a new tests by
changing bazel rule instead of adding a new test file; this case can
only be covered by our previous logic of computing new tests; recover
this logic (in addition to the logic of computing new tests by looking
at changed test files)

Test:
- CI

---------

Signed-off-by: can <can@anyscale.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
  • Loading branch information
can-anyscale authored and ryanaoleary committed Jun 6, 2024
1 parent a74c45f commit 18f7fad
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
22 changes: 21 additions & 1 deletion ci/ray_ci/test_tester.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
_get_high_impact_test_targets,
_get_flaky_test_targets,
_get_tag_matcher,
_get_new_tests,
_get_changed_files,
_get_changed_tests,
_get_human_specified_tests,
Expand Down Expand Up @@ -129,6 +130,9 @@ def test_get_test_targets() -> None:
), mock.patch(
"ci.ray_ci.tester._get_changed_tests",
return_value=set(),
), mock.patch(
"ci.ray_ci.tester._get_new_tests",
return_value=set(),
):
assert set(
_get_test_targets(
Expand Down Expand Up @@ -215,6 +219,7 @@ def test_get_high_impact_test_targets() -> None:
{
"input": [],
"new_tests": set(),
"changed_tests": set(),
"human_tests": set(),
"output": set(),
},
Expand All @@ -234,6 +239,7 @@ def test_get_high_impact_test_targets() -> None:
),
],
"new_tests": {"//core_new"},
"changed_tests": {"//core_new"},
"human_tests": {"//human_test"},
"output": {
"//core_good",
Expand All @@ -247,8 +253,11 @@ def test_get_high_impact_test_targets() -> None:
"ray_release.test.Test.gen_high_impact_tests",
return_value={"step": test["input"]},
), mock.patch(
"ci.ray_ci.tester._get_changed_tests",
"ci.ray_ci.tester._get_new_tests",
return_value=test["new_tests"],
), mock.patch(
"ci.ray_ci.tester._get_changed_tests",
return_value=test["changed_tests"],
), mock.patch(
"ci.ray_ci.tester._get_human_specified_tests",
return_value=test["human_tests"],
Expand All @@ -263,6 +272,17 @@ def test_get_high_impact_test_targets() -> None:
)


@mock.patch("subprocess.check_output")
@mock.patch("ray_release.test.Test.gen_from_s3")
def test_get_new_tests(mock_gen_from_s3, mock_check_output) -> None:
mock_gen_from_s3.return_value = [
_stub_test({"name": "linux://old_test_01"}),
_stub_test({"name": "linux://old_test_02"}),
]
mock_check_output.return_value = b"//old_test_01\n//new_test"
assert _get_new_tests("linux") == {"//new_test"}


@mock.patch.dict(
os.environ,
{"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "base", "BUILDKITE_COMMIT": "commit"},
Expand Down
25 changes: 24 additions & 1 deletion ci/ray_ci/tester.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,15 @@ def _get_high_impact_test_targets(
for test in itertools.chain.from_iterable(step_id_to_tests.values())
if test.get_oncall() == team
}
new_tests = _get_new_tests(os_prefix)
changed_tests = _get_changed_tests()
human_specified_tests = _get_human_specified_tests()

return high_impact_tests.union(changed_tests).union(human_specified_tests)
return (
high_impact_tests.union(new_tests)
.union(changed_tests)
.union(human_specified_tests)
)


def _get_human_specified_tests() -> Set[str]:
Expand All @@ -445,6 +450,24 @@ def _get_human_specified_tests() -> Set[str]:
return tests


def _get_new_tests(prefix: str) -> Set[str]:
"""
Get all local test targets that are not in database
"""
local_test_targets = (
subprocess.check_output(
["bazel", "query", "tests(//...)"],
cwd=bazel_workspace_dir,
)
.decode()
.strip()
.split(os.linesep)
)
db_test_targets = {test.get_target() for test in Test.gen_from_s3(prefix=prefix)}

return set(local_test_targets).difference(db_test_targets)


def _get_changed_tests() -> Set[str]:
"""
Get all changed tests in the current PR
Expand Down

0 comments on commit 18f7fad

Please sign in to comment.