[ci][microcheck/12] move get_new_tests and get_human_specified_tests …

…to Test class (ray-project#45500)

This is part of the series to move some of the function to compute
microcheck test coverage to the Test class. This will allow me to reuse
the logic more easier in other places.

Should be a pure refactoring.

Test:
- CI

---------

Signed-off-by: can <[email protected]>

ci/ray_ci/test_tester.py

@@ -17,7 +17,6 @@
     _get_flaky_test_targets,
     _get_tag_matcher,
     _get_new_tests,
-    _get_human_specified_tests,
 )
 from ray_release.test import Test, TestState
 
@@ -257,7 +256,7 @@ def test_get_high_impact_test_targets() -> None:
             "ray_release.test.Test.get_changed_tests",
             return_value=test["changed_tests"],
         ), mock.patch(
-            "ci.ray_ci.tester._get_human_specified_tests",
+            "ray_release.test.Test.get_human_specified_tests",
             return_value=test["human_tests"],
         ):
             assert (
@@ -283,17 +282,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_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 = [
         {

ci/ray_ci/tester.py

@@ -1,6 +1,5 @@
 import itertools
 import os
-import subprocess
 import sys
 from typing import List, Set, Tuple, Optional
 
@@ -18,7 +17,7 @@
 from ci.ray_ci.linux_tester_container import LinuxTesterContainer
 from ci.ray_ci.windows_tester_container import WindowsTesterContainer
 from ci.ray_ci.tester_container import TesterContainer
-from ci.ray_ci.utils import docker_login, ci_init, logger
+from ci.ray_ci.utils import docker_login, ci_init
 from ray_release.test import Test, TestState
 
 CUDA_COPYRIGHT = """
@@ -418,7 +417,7 @@ def _get_high_impact_test_targets(
     }
     new_tests = _get_new_tests(os_prefix, container)
     changed_tests = Test.get_changed_tests(bazel_workspace_dir)
-    human_specified_tests = _get_human_specified_tests()
+    human_specified_tests = Test.get_human_specified_tests(bazel_workspace_dir)
 
     return (
         high_impact_tests.union(new_tests)
@@ -427,29 +426,6 @@ def _get_high_impact_test_targets(
     )
 
 
-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_new_tests(prefix: str, container: TesterContainer) -> Set[str]:
     """
     Get all local test targets that are not in database

release/ray_release/test.py

@@ -28,6 +28,8 @@
     get_write_state_machine_aws_bucket,
 )
 
+MICROCHECK_COMMAND = "@microcheck"
+
 AWS_TEST_KEY = "ray_tests"
 AWS_TEST_RESULT_KEY = "ray_test_results"
 DEFAULT_PYTHON_VERSION = tuple(
@@ -212,6 +214,29 @@ def gen_high_impact_tests(cls, prefix: str) -> Dict[str, List]:
 
         return step_id_to_tests
 
+    @classmethod
+    def get_human_specified_tests(cls, bazel_workspace_dir: str) -> 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 not message.startswith(MICROCHECK_COMMAND):
+                continue
+            tests = tests.union(message[len(MICROCHECK_COMMAND) :].strip().split(" "))
+
+        return tests
+
     @classmethod
     def get_changed_tests(cls, bazel_workspace_dir: str) -> Set[str]:
         """

release/ray_release/tests/test_test.py

@@ -405,5 +405,16 @@ def test_get_changed_tests(
     assert Test.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 Test.get_human_specified_tests("") == {"//test01", "//test02"}
+
+
 if __name__ == "__main__":
     sys.exit(pytest.main(["-v", __file__]))