From 566dd5e8175ba38f61ccba670fae53eaeaab6a15 Mon Sep 17 00:00:00 2001 From: Cuong Nguyen <128072568+can-anyscale@users.noreply.github.com> Date: Fri, 17 May 2024 13:48:25 -0700 Subject: [PATCH] [ci][microcheck] manually add tests to microcheck (#45400) Allow PR owners to add tests to microcheck manually by adding a line with syntax `@microcheck //test_target_01 //test_target_02` to the git commit message. Thanks @stephanie-wang for the great idea. Note that it's ok to provide an invalid target because `tester` will re-validate them with `bazel query`. Test: - //release:test_config is manually added and run https://buildkite.com/ray-project/microcheck/builds/169#018f84d5-1024-4285-a3fa-59a96e925563/179-1057 --------- Signed-off-by: can --- ci/ray_ci/test_tester.py | 23 ++++++++++++++++++++++- ci/ray_ci/tester.py | 28 ++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/ci/ray_ci/test_tester.py b/ci/ray_ci/test_tester.py index c1d6078ef67db..c2083222df955 100644 --- a/ci/ray_ci/test_tester.py +++ b/ci/ray_ci/test_tester.py @@ -18,6 +18,7 @@ _get_tag_matcher, _get_changed_files, _get_changed_tests, + _get_human_specified_tests, ) from ray_release.test import Test, TestState @@ -214,6 +215,7 @@ def test_get_high_impact_test_targets() -> None: { "input": [], "new_tests": set(), + "human_tests": set(), "output": set(), }, { @@ -232,9 +234,11 @@ def test_get_high_impact_test_targets() -> None: ), ], "new_tests": {"//core_new"}, + "human_tests": {"//human_test"}, "output": { "//core_good", "//core_new", + "//human_test", }, }, ] @@ -245,6 +249,9 @@ def test_get_high_impact_test_targets() -> None: ), mock.patch( "ci.ray_ci.tester._get_changed_tests", return_value=test["new_tests"], + ), mock.patch( + "ci.ray_ci.tester._get_human_specified_tests", + return_value=test["human_tests"], ): assert ( _get_high_impact_test_targets( @@ -256,7 +263,10 @@ def test_get_high_impact_test_targets() -> None: ) -@mock.patch.dict(os.environ, {"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "base"}) +@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: @@ -277,6 +287,17 @@ def test_get_changed_tests( assert _get_changed_tests() == {"//t1", "//t2"} +@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_human_specified_tests(mock_check_output, mock_check_call) -> None: + mock_check_output.return_value = b"hi\n@microcheck //test01 //test02\nthere" + assert _get_human_specified_tests() == {"//test01", "//test02"} + + def test_get_flaky_test_targets() -> None: test_harness = [ { diff --git a/ci/ray_ci/tester.py b/ci/ray_ci/tester.py index b3c5b7486c89d..cb1cd40a7977a 100644 --- a/ci/ray_ci/tester.py +++ b/ci/ray_ci/tester.py @@ -38,6 +38,7 @@ """ # noqa: E501 DEFAULT_EXCEPT_TAGS = {"manual"} +MICROCHECK_COMMAND = "@microcheck" # Gets the path of product/tools/docker (i.e. the parent of 'common') bazel_workspace_dir = os.environ.get("BUILD_WORKSPACE_DIRECTORY", "") @@ -416,8 +417,32 @@ def _get_high_impact_test_targets( if test.get_oncall() == team } changed_tests = _get_changed_tests() + human_specified_tests = _get_human_specified_tests() - return high_impact_tests.union(changed_tests) + return high_impact_tests.union(changed_tests).union(human_specified_tests) + + +def _get_human_specified_tests() -> Set[str]: + """ + Get all test targets that are specified by humans + """ + 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() + + tests = set() + messages = subprocess.check_output( + ["git", "rev-list", "--format=%b", f"origin/{base}...{head}"], + cwd=bazel_workspace_dir, + ) + for message in messages.decode().splitlines(): + if message.startswith(MICROCHECK_COMMAND): + tests = tests.union(message[len(MICROCHECK_COMMAND) :].strip().split(" ")) + logger.info(f"Human specified tests: {tests}") + + return tests def _get_changed_tests() -> Set[str]: @@ -472,7 +497,6 @@ def _get_changed_files() -> Set[str]: # if not in a PR, return an empty set return set() - subprocess.check_call(["git", "fetch", "origin", base], cwd=bazel_workspace_dir) changes = subprocess.check_output( ["git", "diff", "--name-only", f"origin/{base}...{head}"], cwd=bazel_workspace_dir,