[ci] bump rayciversion, test_rules.txt updates default #59744
Conversation
Update to raycicmd has conditional testing built in, alongside some changes to test_rules.txt ray-project/rayci#331 This update bumps the rayciversion, alongside required changes to create defaults Signed-off-by: andrew <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps rayciversion and updates ci/pipeline/test_rules.txt to use new \fallthrough and \default features. While this is a good refactoring to make CI logic more declarative, it introduces a critical issue. The script that appears to be responsible for parsing this file, ci/pipeline/determine_tests_to_run.py, does not support the new syntax. This will cause the rules to be misinterpreted and lead to a severe drop in test coverage. This dependency on an un-updated script makes the current change unsafe to merge.
andrew-anyscale
force-pushed
the
andrewpollack/bump-test-rules
branch
from
15452d4 to
720e22c
Compare
Signed-off-by: Andrew Pollack-Gray <[email protected]>
| \fallthrough | ||
| @ always lint |
There was a problem hiding this comment.
reading this now, this feels a bit unnatural to read.
I feel that it should be something like:
* # matching anything
@ always lint
\fallthrough # can only be right before a `;` line.
;
then this file is still going through a roughly speaking sequential logic flow
otherwise, maybe it is still more natural to write:
\always
@ always lint
;
There was a problem hiding this comment.
I'll make the change to update to the first proposal
There was a problem hiding this comment.
|
closing in favor of #59987 |
Update to raycicmd has conditional testing built in, alongside some changes to test_rules.txt
ray-project/rayci#331
This update bumps the rayciversion, alongside required changes to create defaults