From 190e4867192bda2cc265f2515579b8c00977afef Mon Sep 17 00:00:00 2001 From: Mehrdad Date: Thu, 18 Jun 2020 03:44:47 -0700 Subject: [PATCH 1/5] Use os.path.join() on Windows --- python/ray/serve/tests/test_failure.py | 3 ++- python/ray/tests/test_autoscaler.py | 2 +- python/ray/tests/test_autoscaler_yaml.py | 7 ++--- python/ray/tests/test_projects.py | 26 +++++++++++-------- python/ray/tune/tests/test_run_experiment.py | 4 +-- python/ray/tune/tests/test_trial_scheduler.py | 2 +- 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/python/ray/serve/tests/test_failure.py b/python/ray/serve/tests/test_failure.py index 4e7eb7f80780c..959384fa22594 100644 --- a/python/ray/serve/tests/test_failure.py +++ b/python/ray/serve/tests/test_failure.py @@ -214,7 +214,8 @@ def __init__(self, path): def __call__(self): pass - temp_path = tempfile.gettempdir() + "/" + serve.utils.get_random_letters() + temp_path = os.path.join(tempfile.gettempdir(), + serve.utils.get_random_letters()) serve.create_backend("replica_failure", Worker, temp_path) serve.update_backend_config("replica_failure", {"num_replicas": 2}) serve.create_endpoint( diff --git a/python/ray/tests/test_autoscaler.py b/python/ray/tests/test_autoscaler.py index 2d5cd76e7d3cb..8d7046d6fcf3b 100644 --- a/python/ray/tests/test_autoscaler.py +++ b/python/ray/tests/test_autoscaler.py @@ -306,7 +306,7 @@ def create_provider(self, config, cluster_name): return self.provider def write_config(self, config): - path = self.tmpdir + "/simple.yaml" + path = os.path.join(self.tmpdir, "simple.yaml") with open(path, "w") as f: f.write(yaml.dump(config)) return path diff --git a/python/ray/tests/test_autoscaler_yaml.py b/python/ray/tests/test_autoscaler_yaml.py index da17e5d2e3324..6c8b76e902781 100644 --- a/python/ray/tests/test_autoscaler_yaml.py +++ b/python/ray/tests/test_autoscaler_yaml.py @@ -8,12 +8,12 @@ from ray.autoscaler.util import prepare_config, validate_config from ray.test_utils import recursive_fnmatch -RAY_PATH = os.path.abspath(os.path.join(__file__, "../../")) +RAY_PATH = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) CONFIG_PATHS = recursive_fnmatch( os.path.join(RAY_PATH, "autoscaler"), "*.yaml") CONFIG_PATHS += recursive_fnmatch( - os.path.join(RAY_PATH, "tune/examples/"), "*.yaml") + os.path.join(RAY_PATH, "tune", "examples"), "*.yaml") class AutoscalingConfigTest(unittest.TestCase): @@ -52,7 +52,8 @@ def _test_invalid_config(self, config_path): pass def testInvalidConfig(self): - self._test_invalid_config("tests/additional_property.yaml") + self._test_invalid_config( + os.path.join("tests", "additional_property.yaml")) if __name__ == "__main__": diff --git a/python/ray/tests/test_projects.py b/python/ray/tests/test_projects.py index b2d6b60daf558..48021fcf123cd 100644 --- a/python/ray/tests/test_projects.py +++ b/python/ray/tests/test_projects.py @@ -94,7 +94,8 @@ def run_test_project(project_dir, command, args): def test_session_start_default_project(): result, mock_calls, test_dir = run_test_project( - "session-tests/project-pass", session_start, ["default"]) + os.path.join("session-tests", "project-pass"), session_start, + ["default"]) loaded_project = ray.projects.ProjectDefinition(test_dir) assert result.exit_code == 0 @@ -138,7 +139,8 @@ def test_session_start_default_project(): def test_session_execute_default_project(): result, mock_calls, test_dir = run_test_project( - "session-tests/project-pass", session_execute, ["default"]) + os.path.join("session-tests", "project-pass"), session_execute, + ["default"]) loaded_project = ray.projects.ProjectDefinition(test_dir) assert result.exit_code == 0 @@ -159,13 +161,14 @@ def test_session_execute_default_project(): assert expected_commands == commands_executed result, mock_calls, test_dir = run_test_project( - "session-tests/project-pass", session_execute, ["--shell", "uptime"]) + os.path.join("session-tests", "project-pass"), session_execute, + ["--shell", "uptime"]) assert result.exit_code == 0 def test_session_start_docker_fail(): - result, _, _ = run_test_project("session-tests/with-docker-fail", - session_start, []) + result, _, _ = run_test_project( + os.path.join("session-tests", "with-docker-fail"), session_start, []) assert result.exit_code == 1 assert ("Docker support in session is currently " @@ -173,8 +176,9 @@ def test_session_start_docker_fail(): def test_session_invalid_config_errored(): - result, _, _ = run_test_project("session-tests/invalid-config-fail", - session_start, []) + result, _, _ = run_test_project( + os.path.join("session-tests", "invalid-config-fail"), session_start, + []) assert result.exit_code == 1 assert "validation failed" in result.output @@ -184,7 +188,7 @@ def test_session_invalid_config_errored(): def test_session_create_command(): result, mock_calls, test_dir = run_test_project( - "session-tests/commands-test", session_start, + os.path.join("session-tests", "commands-test"), session_start, ["first", "--a", "1", "--b", "2"]) # Verify the project can be loaded. @@ -202,7 +206,7 @@ def test_session_create_command(): def test_session_create_multiple(): for args in [{"a": "*", "b": "2"}, {"a": "1", "b": "*"}]: result, mock_calls, test_dir = run_test_project( - "session-tests/commands-test", session_start, + os.path.join("session-tests", "commands-test"), session_start, ["first", "--a", args["a"], "--b", args["b"]]) loaded_project = ray.projects.ProjectDefinition(test_dir) @@ -227,14 +231,14 @@ def test_session_create_multiple(): # Using multiple wildcards shouldn't work result, mock_calls, test_dir = run_test_project( - "session-tests/commands-test", session_start, + os.path.join("session-tests", "commands-test"), session_start, ["first", "--a", "*", "--b", "*"]) assert result.exit_code == 1 def test_session_commands(): result, mock_calls, test_dir = run_test_project( - "session-tests/commands-test", session_commands, []) + os.path.join("session-tests", "commands-test"), session_commands, []) assert "This is the first parameter" in result.output assert "This is the second parameter" in result.output diff --git a/python/ray/tune/tests/test_run_experiment.py b/python/ray/tune/tests/test_run_experiment.py index 98b7b129120c9..53ef55b15f472 100644 --- a/python/ray/tune/tests/test_run_experiment.py +++ b/python/ray/tune/tests/test_run_experiment.py @@ -95,7 +95,7 @@ def _train(self): return {"timesteps_this_iter": 1, "done": True} def _save(self, path): - checkpoint = path + "/checkpoint" + checkpoint = os.path.join(path, "checkpoint") with open(checkpoint, "w") as f: f.write("OK") return checkpoint @@ -116,7 +116,7 @@ def _train(self): return {"timesteps_this_iter": 1, "done": True} def _export_model(self, export_formats, export_dir): - path = export_dir + "/exported" + path = os.path.join(export_dir, "exported") with open(path, "w") as f: f.write("OK") return {export_formats[0]: path} diff --git a/python/ray/tune/tests/test_trial_scheduler.py b/python/ray/tune/tests/test_trial_scheduler.py index ab01d14297938..20bb90632b4ed 100644 --- a/python/ray/tune/tests/test_trial_scheduler.py +++ b/python/ray/tune/tests/test_trial_scheduler.py @@ -1145,7 +1145,7 @@ def _train(self): return {"mean_accuracy": self.training_iteration} def _save(self, path): - checkpoint = path + "/checkpoint" + checkpoint = os.path.join(path, "checkpoint") with open(checkpoint, "w") as f: f.write("OK") return checkpoint From 953804cde880cb57d39f57a4c334fd72007974f6 Mon Sep 17 00:00:00 2001 From: Mehrdad Date: Thu, 18 Jun 2020 15:38:53 -0700 Subject: [PATCH 2/5] Replace subprocess.check_output with subprocess.call --- python/ray/tests/conftest.py | 4 +-- python/ray/tests/test_multi_node.py | 44 ++++++++++++++--------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/python/ray/tests/conftest.py b/python/ray/tests/conftest.py index c8d253175d2e9..3a0ae7724768a 100644 --- a/python/ray/tests/conftest.py +++ b/python/ray/tests/conftest.py @@ -181,13 +181,13 @@ def call_ray_start(request): # Disconnect from the Ray cluster. ray.shutdown() # Kill the Ray cluster. - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) @pytest.fixture def call_ray_stop_only(): yield - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) @pytest.fixture() diff --git a/python/ray/tests/test_multi_node.py b/python/ray/tests/test_multi_node.py index 21ad6f28a9a60..e31c1f574d818 100644 --- a/python/ray/tests/test_multi_node.py +++ b/python/ray/tests/test_multi_node.py @@ -359,66 +359,66 @@ def test_calling_start_ray_head(call_ray_stop_only): # should also test the non-head node code path. # Test starting Ray with no arguments. - subprocess.check_output(["ray", "start", "--head"]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "start", "--head"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with a redis port specified. - subprocess.check_output(["ray", "start", "--head"]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "start", "--head"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with a node IP address specified. - subprocess.check_output( + subprocess.check_call( ["ray", "start", "--head", "--node-ip-address", "127.0.0.1"]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with the object manager and node manager ports # specified. - subprocess.check_output([ + subprocess.check_call([ "ray", "start", "--head", "--object-manager-port", "12345", "--node-manager-port", "54321" ]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with the worker port range specified. - subprocess.check_output([ + subprocess.check_call([ "ray", "start", "--head", "--min-worker-port", "50000", "--max-worker-port", "51000" ]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with the number of CPUs specified. - subprocess.check_output(["ray", "start", "--head", "--num-cpus", "2"]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "start", "--head", "--num-cpus", "2"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with the number of GPUs specified. - subprocess.check_output(["ray", "start", "--head", "--num-gpus", "100"]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "start", "--head", "--num-gpus", "100"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with the max redis clients specified. - subprocess.check_output( + subprocess.check_call( ["ray", "start", "--head", "--redis-max-clients", "100"]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) if "RAY_USE_NEW_GCS" not in os.environ: # Test starting Ray with redis shard ports specified. - subprocess.check_output([ + subprocess.check_call([ "ray", "start", "--head", "--redis-shard-ports", "6380,6381,6382" ]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with all arguments specified. - subprocess.check_output([ + subprocess.check_call([ "ray", "start", "--head", "--redis-shard-ports", "6380,6381,6382", "--object-manager-port", "12345", "--num-cpus", "2", "--num-gpus", "0", "--redis-max-clients", "100", "--resources", "{\"Custom\": 1}" ]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) # Test starting Ray with invalid arguments. with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output( + subprocess.check_call( ["ray", "start", "--head", "--address", "127.0.0.1:6379"]) - subprocess.check_output(["ray", "stop"]) + subprocess.check_call(["ray", "stop"]) # Test --block. Killing a child process should cause the command to exit. blocked = subprocess.Popen(["ray", "start", "--head", "--block"]) From ebcf59bbe447d19f1363ed03114d5d98c7ba9474 Mon Sep 17 00:00:00 2001 From: Mehrdad Date: Thu, 18 Jun 2020 16:24:42 -0700 Subject: [PATCH 3/5] Fix conftest --- python/ray/tests/BUILD | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/ray/tests/BUILD b/python/ray/tests/BUILD index f86445a761462..611920652d0eb 100644 --- a/python/ray/tests/BUILD +++ b/python/ray/tests/BUILD @@ -1,8 +1,8 @@ SRCS = [] + select({ - "@bazel_tools//src/conditions:windows": [ + "@bazel_tools//src/conditions:windows": glob([ # TODO(mehrdadn): This should be added for all platforms once resulting errors are fixed - "conftest.py", - ], + "**/conftest.py", + ]), "//conditions:default": [], }) From d5421e1845199c42c374e8d29a53b517fb3b8933 Mon Sep 17 00:00:00 2001 From: Mehrdad Date: Thu, 18 Jun 2020 20:08:22 -0700 Subject: [PATCH 4/5] Make PlasmaStoreRunner::Stop() check loop_ for NULL --- src/ray/object_manager/plasma/store_runner.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ray/object_manager/plasma/store_runner.cc b/src/ray/object_manager/plasma/store_runner.cc index e8ca85378136c..e67b556f5cac7 100644 --- a/src/ray/object_manager/plasma/store_runner.cc +++ b/src/ray/object_manager/plasma/store_runner.cc @@ -133,7 +133,11 @@ void PlasmaStoreRunner::Start() { ArrowLog::ShutDownArrowLog(); } -void PlasmaStoreRunner::Stop() { loop_->Stop(); } +void PlasmaStoreRunner::Stop() { + if (loop_) { + loop_->Stop(); + } +} void PlasmaStoreRunner::Shutdown() { loop_->Shutdown(); From bd692c69c4970aaf207c552c23a729a14f926030 Mon Sep 17 00:00:00 2001 From: Mehrdad Date: Fri, 12 Jun 2020 23:16:19 -0700 Subject: [PATCH 5/5] Re-enable some tests --- ci/travis/ci.sh | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/ci/travis/ci.sh b/ci/travis/ci.sh index 8e639933a73e7..600311255ab8b 100755 --- a/ci/travis/ci.sh +++ b/ci/travis/ci.sh @@ -125,31 +125,23 @@ test_python() { -python/ray/tests:test_actor_failures -python/ray/tests:test_advanced_2 -python/ray/tests:test_advanced_3 - -python/ray/tests:test_array - -python/ray/tests:test_asyncio + -python/ray/tests:test_array # timeout -python/ray/tests:test_autoscaler_aws -python/ray/tests:test_autoscaler_yaml - -python/ray/tests:test_cancel -python/ray/tests:test_component_failures -python/ray/tests:test_cython - -python/ray/tests:test_dynres -python/ray/tests:test_failure -python/ray/tests:test_global_gc - -python/ray/tests:test_global_state - -python/ray/tests:test_iter - -python/ray/tests:test_memory_scheduling -python/ray/tests:test_memstat -python/ray/tests:test_metrics -python/ray/tests:test_multi_node -python/ray/tests:test_multi_node_2 - -python/ray/tests:test_multinode_failures_2 - -python/ray/tests:test_multiprocessing + -python/ray/tests:test_multiprocessing # flaky -python/ray/tests:test_node_manager -python/ray/tests:test_object_manager -python/ray/tests:test_projects - -python/ray/tests:test_queue - -python/ray/tests:test_ray_init - -python/ray/tests:test_reconstruction + -python/ray/tests:test_queue # timeout + -python/ray/tests:test_reconstruction # UnreconstructableError -python/ray/tests:test_stress -python/ray/tests:test_stress_sharded -python/ray/tests:test_webui