Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ci][core] Mark python tests flaky on CI #41195

Merged
merged 3 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions python/ray/_private/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1979,3 +1979,17 @@ def reset_autoscaler_v2_enabled_cache():
import ray.autoscaler.v2.utils as u

u.cached_is_autoscaler_v2 = None


def skip_flaky_test() -> bool:
"""
Skip a test if it is flaky (e.g. in premerge)

Default we will skip the flaky test if not specified otherwise in
CI with CI_SKIP_FLAKY_TEST="0"


Returns:
bool: True if the test should be skipped
"""
return os.environ.get("CI_SKIP_FLAKY_TEST", "1") == "1"
4 changes: 4 additions & 0 deletions python/ray/tests/test_client_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
run_string_as_driver,
run_string_as_driver_nonblocking,
wait_for_condition,
skip_flaky_test,
)
from ray.util.state import list_workers

Expand Down Expand Up @@ -327,6 +328,9 @@ def has_client_deprecation_warn(warning: Warning, expected_replacement: str) ->
@pytest.mark.filterwarnings(
"default:Starting a connection through `ray.client` will be deprecated"
)
@pytest.mark.skipif(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make a wrapper like "run_only_in_postmerge()"?

Copy link
Contributor

@rkooo567 rkooo567 Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also @can-anyscale is there some sort of guarantee this env var won't change? Would like to avoid the env var name/behavior is changed, and we stop tracking flaky tests

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it can only be guarantee through code reviews. But unless core change it, I don't think a random engineer will change it though.

Copy link
Contributor Author

@rickyyx rickyyx Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make a wrapper like "run_only_in_postmerge()"?

I thought this would actually bring in additional dependency such as a test_util.py of where we define this run_only_in_postmerge() func

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I don't think we will ever change the env. If there are times that this need to be changed, let us know!

@rickyyx I think that's probably fine? We don't have that many tests anyway. I want to make sure we have corresponding issue (so we can ensure reason is always passed properly) whenever we mark it like this.

Copy link
Contributor Author

@rickyyx rickyyx Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make sure we have corresponding issue (so we can ensure reason is always passed properly) whenever we mark it like this.

that's a good point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually - it's required to pass in reason with skipif using booleans.

skip_flaky_test(), reason="https://github.com/ray-project/ray/issues/38224"
)
def test_client_deprecation_warn():
"""
Tests that calling ray.client directly raises a deprecation warning with
Expand Down
5 changes: 5 additions & 0 deletions python/ray/tests/test_failure_3.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
wait_for_pid_to_exit,
wait_for_condition,
run_string_as_driver_nonblocking,
skip_flaky_test,
)

SIGKILL = signal.SIGKILL if sys.platform != "win32" else signal.SIGTERM
Expand Down Expand Up @@ -240,6 +241,10 @@ def new_task(_):
[{"_system_config": {"timeout_ms_task_wait_for_death_info": 100000000}}],
indirect=True,
)
@pytest.mark.skipif(
skip_flaky_test(),
reason="https://github.com/ray-project/ray/issues/41188",
)
def test_actor_failure_async_3(ray_start_regular):
@ray.remote(max_restarts=1)
class A:
Expand Down
4 changes: 4 additions & 0 deletions python/ray/tests/test_object_assign_owner.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import time
import numpy as np
import os
from ray._private.test_utils import skip_flaky_test


# https://github.com/ray-project/ray/issues/19659
Expand Down Expand Up @@ -155,6 +156,9 @@ def get_objects(self, refs):


# https://github.com/ray-project/ray/issues/30341
@pytest.mark.skipif(
skip_flaky_test(), reason="https://github.com/ray-project/ray/issues/41175"
)
def test_owner_assign_inner_object(shutdown_only):

ray.init()
Expand Down
10 changes: 6 additions & 4 deletions python/ray/tests/test_placement_group.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import sys
import warnings
import os

import pytest

import ray
from ray._private.utils import get_ray_doc_version
import ray.cluster_utils
from ray._private.test_utils import placement_group_assert_no_leak
from ray._private.test_utils import skip_flaky_test
from ray.util.client.ray_client_helpers import connect_to_client_or_not
from ray.util.scheduling_strategies import PlacementGroupSchedulingStrategy

Expand Down Expand Up @@ -338,6 +340,10 @@ def value(self):

@pytest.mark.parametrize("connect_to_client", [False, True])
@pytest.mark.parametrize("gcs_actor_scheduling_enabled", [False, True])
@pytest.mark.skipif(
skip_flaky_test(),
reason="https://github.com/ray-project/ray/issues/38726",
)
def test_placement_group_strict_spread(
ray_start_cluster, connect_to_client, gcs_actor_scheduling_enabled
):
Expand Down Expand Up @@ -646,8 +652,6 @@ def test_omp_num_threads_in_pg(ray_start_cluster):

@ray.remote(num_cpus=3)
def test_omp_num_threads():
import os

omp_threads = os.environ["OMP_NUM_THREADS"]
return int(omp_threads)

Expand All @@ -670,8 +674,6 @@ def test_omp_num_threads():


if __name__ == "__main__":
import os

if os.environ.get("PARALLEL_CI"):
sys.exit(pytest.main(["-n", "auto", "--boxed", "-vs", __file__]))
else:
Expand Down
5 changes: 5 additions & 0 deletions python/ray/tests/test_runtime_env_working_dir_3.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import ray.experimental.internal_kv as kv
from ray._private.ray_constants import RAY_RUNTIME_ENV_URI_PIN_EXPIRATION_S_ENV_VAR
from ray._private.test_utils import chdir, check_local_files_gced, wait_for_condition
from ray._private.test_utils import skip_flaky_test
from ray._private.utils import get_directory_size_bytes

# This test requires you have AWS credentials set up (any AWS credentials will
Expand Down Expand Up @@ -249,6 +250,10 @@ def check(self):
@pytest.mark.parametrize(
"source", [S3_PACKAGE_URI, lazy_fixture("tmp_working_dir")]
)
@pytest.mark.skipif(
skip_flaky_test(),
reason="https://github.com/ray-project/ray/issues/40781",
)
def test_detached_actor_gc(
self,
start_cluster,
Expand Down
5 changes: 5 additions & 0 deletions python/ray/tests/test_state_api_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
format_web_url,
wait_for_condition,
wait_until_server_available,
skip_flaky_test,
)

from ray._private.ray_constants import (
Expand Down Expand Up @@ -1272,6 +1273,10 @@ def verify():
@pytest.mark.skipif(
sys.platform == "win32", reason="Windows has logging race from tasks."
)
@pytest.mark.skipif(
skip_flaky_test(),
reason="https://github.com/ray-project/ray/issues/40959",
)
def test_log_task(shutdown_only):
from ray.runtime_env import RuntimeEnv

Expand Down
Loading