From 7136f817feaa378cab9c9c7f28c87740897605e8 Mon Sep 17 00:00:00 2001
From: can <can@anyscale.com>
Date: Mon, 12 Jun 2023 23:09:39 +0000
Subject: [PATCH] Avoid modify test object before running tests

Signed-off-by: can <can@anyscale.com>
---
 release/ray_release/buildkite/filter.py     |  6 ++-
 release/ray_release/tests/test_buildkite.py | 41 +++++++++++++--------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/release/ray_release/buildkite/filter.py b/release/ray_release/buildkite/filter.py
index ad80eb61fddb..820a837becba 100644
--- a/release/ray_release/buildkite/filter.py
+++ b/release/ray_release/buildkite/filter.py
@@ -1,4 +1,5 @@
 import re
+import copy
 from collections import defaultdict
 from typing import List, Optional, Tuple, Dict, Any
 
@@ -40,8 +41,9 @@ def filter_tests(
         if not run_jailed_tests:
             if test.get("jailed", False):
                 continue
-            test.update_from_s3()
-            if test.is_jailed_with_open_issue(TestStateMachine.get_ray_repo()):
+            clone_test = copy.deepcopy(test)
+            clone_test.update_from_s3()
+            if clone_test.is_jailed_with_open_issue(TestStateMachine.get_ray_repo()):
                 continue
 
         test_frequency = get_frequency(test["frequency"])
diff --git a/release/ray_release/tests/test_buildkite.py b/release/ray_release/tests/test_buildkite.py
index 0570db20e45c..c65f4b8769be 100644
--- a/release/ray_release/tests/test_buildkite.py
+++ b/release/ray_release/tests/test_buildkite.py
@@ -6,6 +6,7 @@
 from unittest.mock import patch
 
 import yaml
+from github import Repository
 
 from ray_release.buildkite.concurrency import (
     get_test_resources_from_cluster_compute,
@@ -64,6 +65,14 @@ def artifacts(self):
         return self
 
 
+class MockTest(Test):
+    def update_from_s3(self) -> None:
+        self["update_from_s3"] = True
+
+    def is_jailed_with_open_issue(self, ray_github: Repository) -> bool:
+        return False
+
+
 class BuildkiteSettingsTest(unittest.TestCase):
     def setUp(self) -> None:
         self.buildkite = {}
@@ -374,24 +383,23 @@ def _filter_names_smoke(self, *args, **kwargs):
         filtered = filter_tests(*args, **kwargs)
         return [(t[0]["name"], t[1]) for t in filtered]
 
-    @patch("ray_release.test.Test.update_from_s3", return_value=None)
-    @patch("ray_release.test.Test.is_jailed_with_open_issue", return_value=False)
     @patch(
         "ray_release.test_automation.state_machine.TestStateMachine.get_ray_repo",
         return_value=None,
     )
     def testFilterTests(self, *args):
+        test = MockTest(
+            {
+                "name": "test_1",
+                "frequency": "nightly",
+                "smoke_test": {"frequency": "nightly"},
+                "team": "team_1",
+                "run": {"type": "job"},
+            }
+        )
         tests = [
-            Test(
-                {
-                    "name": "test_1",
-                    "frequency": "nightly",
-                    "smoke_test": {"frequency": "nightly"},
-                    "team": "team_1",
-                    "run": {"type": "job"},
-                }
-            ),
-            Test(
+            test,
+            MockTest(
                 {
                     "name": "test_2",
                     "frequency": "weekly",
@@ -400,8 +408,8 @@ def testFilterTests(self, *args):
                     "run": {"type": "client"},
                 }
             ),
-            Test({"name": "other_1", "frequency": "weekly", "team": "team_2"}),
-            Test(
+            MockTest({"name": "other_1", "frequency": "weekly", "team": "team_2"}),
+            MockTest(
                 {
                     "name": "other_2",
                     "frequency": "nightly",
@@ -410,8 +418,8 @@ def testFilterTests(self, *args):
                     "run": {"type": "job"},
                 }
             ),
-            Test({"name": "other_3", "frequency": "manual", "team": "team_2"}),
-            Test({"name": "test_3", "frequency": "nightly", "team": "team_2"}),
+            MockTest({"name": "other_3", "frequency": "manual", "team": "team_2"}),
+            MockTest({"name": "test_3", "frequency": "nightly", "team": "team_2"}),
         ]
 
         filtered = self._filter_names_smoke(tests, frequency=Frequency.ANY)
@@ -426,6 +434,7 @@ def testFilterTests(self, *args):
                 ("test_3", False),
             ],
         )
+        assert not test.get("update_from_s3")
 
         filtered = self._filter_names_smoke(
             tests,