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

[Serve][Part2] Migrate the tests to use deployment graph api #26507

Merged
merged 5 commits into from
Jul 15, 2022

Conversation

sihanwang41
Copy link
Contributor

@sihanwang41 sihanwang41 commented Jul 13, 2022

Why are these changes needed?

Second pr for the test migration in the current session.

  • ignore test_deployment_graph file since it is for testing internal deployment graph function, which need to use deploy() or list_deployment() etc to verify the correcness.
  • ignore the test_fastapi and multi routes test with multi deployment which is not supported yet in the deployment graph.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@edoakes
Copy link
Contributor

edoakes commented Jul 14, 2022

@sihanwang41 there is a windows test failure in serve:test_gcs_failure. I don't see it failing on flaky test tracker. Re-ran it to see if it passes, but take a look?

@edoakes edoakes added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 14, 2022
@simon-mo
Copy link
Contributor

@edoakes test_gcs_failure turns out hasn't been enabled in previous master builds. This PRs enables it. @sihanwang41 I think it's fine to skip it on windows if it is too hard to debug: https://github.com/ray-project/ray/blob/master/ci/ci.sh#L172

@sihanwang41
Copy link
Contributor Author

In the test, we are using forked to isolate the environment. Looks like it is not supported in the windows.
code:

if __name__ == "__main__":
    # When GCS is down, right now some core worker members are not cleared
    # properly in ray.shutdown. Given that this is not hi-pri issue,
    # using --forked for isolation.
    sys.exit(pytest.main(["-v", "-s", "--forked", __file__]))
Exception ignored in: <function ForkedFunc.__del__ at 0x000001FE7FE99940>
Traceback (most recent call last):
  File "C:\Miniconda3\lib\site-packages\py\_process\forkedfunc.py", line 110, in __del__
    if self.pid is not None:  # only clean up in main process
AttributeError: 'ForkedFunc' object has no attribute 'pid'

I will skip the test on windows for now. @edoakes @simon-mo

@simon-mo
Copy link
Contributor



//python/ray/serve:test_gcs_failure                                      FAILED in 3 out of 3 in 4.5s
--
  | Stats over 3 runs: max = 4.5s, min = 4.3s, avg = 4.4s, dev = 0.1s
  | C:/tmp/4lhdprva/execroot/com_github_ray_project_ray/bazel-out/x64_windows-opt/testlogs/python/ray/serve/test_gcs_failure/test.log
  | C:/tmp/4lhdprva/execroot/com_github_ray_project_ray/bazel-out/x64_windows-opt/testlogs/python/ray/serve/test_gcs_failure/test_attempts/attempt_1.log
  | C:/tmp/4lhdprva/execroot/com_github_ray_project_ray/bazel-out/x64_windows-opt/testlogs/python/ray/serve/test_gcs_failure/test_attempts/attempt_2.log


it still shows on windows. I think you have skip it on bazel target level. (see my previous comment)

@simon-mo simon-mo merged commit 09a6e53 into ray-project:master Jul 15, 2022
truelegion47 pushed a commit to truelegion47/ray that referenced this pull request Jul 16, 2022
* master: (35 commits)
  [data] Refactor all to all op implementations into a separate file (ray-project#26585)
  [Datasets] Explicitly define Dataset-like APIs in DatasetPipeline class (ray-project#26394)
  [Serve][Part2] Migrate the tests to use deployment graph api (ray-project#26507)
  [Serve] Default to EveryNode when starting Serve from REST API (ray-project#26588)
  Revert "[KubeRay][Autoscaler][Core] Add a flag to disable ray status version check (ray-project#26584)" (ray-project#26597)
  [air] Add initial benchmark section (ray-project#26608)
  [Workflow] Remove workflow execution module (ray-project#26504)
  [air] Add xgboost release test for silver tier(10-node case). (ray-project#26460)
  Revert "Revert "[serve] Use soft constraint for pinning controller on head node (ray-project#25091)" (ray-project#25857)" (ray-project#25858)
  [RLlib] Fixes MARWIL release tests (ray-project#26586)
  [Datasets] Improve read_xxx experience of HTTP file (ray-project#26454)
  Cleanup ActorContext due to multi actor instances got removed. (ray-project#26497)
  Print newest_ckpt_path when resuming trial. (ray-project#26561)
  Fix test_serialization_error_message for pytest 6.x (ray-project#26591)
  [RLlib] Make DQN update_target use only trainable variables. (ray-project#25226)
  [RLlib] In env check, step only expected agents. (ray-project#26425)
  [RLlib] `restart_failed_sub_environments` now works for MA cases and crashes during `reset()`; +more tests and logging; add eval worker sub-env fault tolerance test. (ray-project#26276)
  [runtime env] plugin refactor[4/n]: remove runtime env protobuf (ray-project#26522)
  Improve streaming read performance for default configuration. (ray-project#26587)
  [Dashboard] Fix test dashboard flaky by catch an expected exception (ray-project#26555)
  ...
truelegion47 pushed a commit to truelegion47/ray that referenced this pull request Jul 16, 2022
xwjiang2010 pushed a commit to xwjiang2010/ray that referenced this pull request Jul 19, 2022
avnishn pushed a commit to smorad/ray that referenced this pull request Jul 20, 2022
klwuibm pushed a commit to yuanchi2807/ray that referenced this pull request Jul 27, 2022
franklsf95 pushed a commit to franklsf95/ray that referenced this pull request Aug 2, 2022
gramhagen pushed a commit to gramhagen/ray that referenced this pull request Aug 15, 2022
Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants