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

[Ray] Refine ray tests #2793

Merged

Conversation

chaokunyang
Copy link
Contributor

@chaokunyang chaokunyang commented Mar 7, 2022

What do these changes do?

refine ray tests:

  • print tests setup and execution duration
  • moduling ray tests
  • use shared ray cluster to reduce setup time

Related issue number

Fixes #2792

Check code requirements

  • tests added / passed (if needed)
  • Ensure all linting tests pass, see here for how to run them

@chaokunyang chaokunyang requested review from wjsi and qinxuye as code owners March 7, 2022 11:31
@chaokunyang chaokunyang changed the title [ray] use pytest fork mode for ray tests [ray] refine ray tests Mar 7, 2022
@chaokunyang chaokunyang force-pushed the use_pytest_forked_for_ray_test branch from a2280c9 to b155779 Compare March 7, 2022 14:46
@chaokunyang chaokunyang force-pushed the use_pytest_forked_for_ray_test branch from e81fd84 to 11e1330 Compare March 8, 2022 02:47
@qinxuye qinxuye added mod: ray integration to be backported Indicate that the PR need to be backported to stable branch type: tests labels Mar 8, 2022
@qinxuye qinxuye added this to the v0.9.0b2 milestone Mar 8, 2022
@qinxuye
Copy link
Collaborator

qinxuye commented Mar 8, 2022

Codacy failed, please fix it.

@qinxuye qinxuye modified the milestones: v0.9.0b2, v0.9.0rc1 Mar 8, 2022
@chaokunyang chaokunyang force-pushed the use_pytest_forked_for_ray_test branch from 7821cc8 to df7185f Compare March 9, 2022 03:25
@chaokunyang chaokunyang force-pushed the use_pytest_forked_for_ray_test branch from 4304efd to b38232e Compare March 9, 2022 11:07
ci/reload-env.sh Outdated
@@ -1,5 +1,6 @@
#!/bin/bash

set -e
set -x
Copy link
Member

Choose a reason for hiding this comment

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

Github actions put set -e as a default option. set -x will add unnecessary call records and should be used for debug purpose only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

set -x can help us locate bugs, I prefer enable it. For CI, I think it's OK to have more logs to keep as much info as possible. Is there any privacy issue for enable it?

Copy link
Member

Choose a reason for hiding this comment

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

When using if block in bash, option -e will output EVERY statement executed. Thus it is not appropriate to enable this globally. I'll remove it myself.

Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM

@wjsi wjsi changed the title [ray] refine ray tests [Ray] Refine ray tests Mar 10, 2022
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

@qinxuye qinxuye merged commit a2857bd into mars-project:master Mar 10, 2022
wjsi pushed a commit to wjsi/mars that referenced this pull request Mar 11, 2022
wjsi added a commit that referenced this pull request Mar 11, 2022
@qinxuye qinxuye added backported already PR has been backported and removed to be backported Indicate that the PR need to be backported to stable branch labels Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] AssignerActor band assignment statement maybe incorrect
4 participants