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] deprecated jail field in release test definition #36614

Merged
merged 1 commit into from
Jun 23, 2023
Merged

Conversation

can-anyscale
Copy link
Collaborator

Why are these changes needed?

Currently the test can be jailed via two ways, either through this 'jailed' field in test definition, or by test state machine. Deprecated this jailed field and let the state machine does it job. This refuses confusion around how to unjail and jail a test.

Also improve the logic of soft fail and append the [jailed] to the buildkite step name. We should not soft fail when the test is jailed.

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • Testing Strategy
    • Unit tests
    • Release tests

@can-anyscale can-anyscale marked this pull request as ready for review June 20, 2023 21:04
@can-anyscale can-anyscale requested a review from a team as a code owner June 20, 2023 21:04
@can-anyscale
Copy link
Collaborator Author

Check that https://buildkite.com/ray-project/release-tests-pr/builds/42882 the jailed test runs correctly and the step name is denoted with [jailed]

Signed-off-by: can <can@anyscale.com>
Copy link
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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

how can one easily tell what is the current state of a test now?

@can-anyscale
Copy link
Collaborator Author

@aslonnie through github issue i guess go/release-test

@can-anyscale
Copy link
Collaborator Author

@rickyyx fyi, this removed the jailed test feature from release test definition, and rely sole on the test state machine to jail test. Should not change the behavior of your existing jail test significantly.

@rickyyx
Copy link
Contributor

rickyyx commented Jun 21, 2023

@rickyyx fyi, this removed the jailed test feature from release test definition, and rely sole on the test state machine to jail test. Should not change the behavior of your existing jail test significantly.

So unstable tests will still be running right?

ray/release/release_tests.yaml

Lines 4757 to 4758 in b9ef5da

stable: false
jailed: true

@can-anyscale
Copy link
Collaborator Author

@rickyyx that's right, they will still be running

@can-anyscale can-anyscale added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Jun 22, 2023
@richardliaw richardliaw merged commit ec979a5 into master Jun 23, 2023
@richardliaw richardliaw deleted the can-no-jail branch June 23, 2023 17:48
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
Signed-off-by: can <can@anyscale.com>
Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants