Skip to content

Commit

Permalink
[ci][microcheck] recover the logic to compute new tests (#45507)
Browse files Browse the repository at this point in the history
#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)

This is a redo of #45495 which
got reverted. The difference now is that we run the bazel command in a
container instead of on the current environment. bazel seems to have
issues sharing the cache when calling bazel within bazel
(https://buildkite.com/ray-project/microcheck/builds/444#018fa23a-6e31-435b-a0ea-412ca2d1017b/175-1476)

Test:
- CI
- full microcheck run:
https://buildkite.com/ray-project/microcheck/builds/464

Signed-off-by: can <can@anyscale.com>
  • Loading branch information
can-anyscale committed May 23, 2024
1 parent 1858421 commit 7fa11cf
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 2 deletions.
24 changes: 23 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,19 @@ def test_get_high_impact_test_targets() -> None:
)


@mock.patch("ci.ray_ci.tester_container.TesterContainer.run_script_with_output")
@mock.patch("ray_release.test.Test.gen_from_s3")
def test_get_new_tests(mock_gen_from_s3, mock_run_script_with_output) -> None:
mock_gen_from_s3.return_value = [
_stub_test({"name": "linux://old_test_01"}),
_stub_test({"name": "linux://old_test_02"}),
]
mock_run_script_with_output.return_value = "//old_test_01\n//new_test"
assert _get_new_tests(
"linux", LinuxTesterContainer("test", skip_ray_installation=True)
) == {"//new_test"}


@mock.patch.dict(
os.environ,
{"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "base", "BUILDKITE_COMMIT": "commit"},
Expand Down
21 changes: 20 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, container)
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,20 @@ def _get_human_specified_tests() -> Set[str]:
return tests


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

return 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 7fa11cf

Please sign in to comment.