Skip to content

Commit

Permalink
Avoid modify test object before running tests
Browse files Browse the repository at this point in the history
Signed-off-by: can <can@anyscale.com>
  • Loading branch information
can-anyscale committed Jun 13, 2023
1 parent 5c0201f commit 7136f81
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 18 deletions.
6 changes: 4 additions & 2 deletions release/ray_release/buildkite/filter.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import re
import copy
from collections import defaultdict
from typing import List, Optional, Tuple, Dict, Any

Expand Down Expand Up @@ -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"])
Expand Down
41 changes: 25 additions & 16 deletions release/ray_release/tests/test_buildkite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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)
Expand All @@ -426,6 +434,7 @@ def testFilterTests(self, *args):
("test_3", False),
],
)
assert not test.get("update_from_s3")

filtered = self._filter_names_smoke(
tests,
Expand Down

0 comments on commit 7136f81

Please sign in to comment.