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

[RayJob] Improve flexibility to run specified YAML test #1844

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

evalaiyc98
Copy link
Contributor

Why are these changes needed?

The purpose of this PR is the same as the #1812 (comment) , aiming to enhance flexibility in executing sample YAML tests.

With this PR, it is possible to specify one or multiple sample YAML test files. If no files are specified, the test will run on all sample YAML tests related to RayJob.

After the parameter --yaml-files, use the filename of the YAML for testing. Here's an example:

  1. [Default] Run all YAML files.
    python3 tests/test_sample_rayjob_yamls.py

  2. Specified a single YAML file.
    python3 tests/test_sample_rayjob_yamls.py --yaml-files ray_v1alpha1_rayjob.yaml
    You can also specify multiple YAML files:
    python3 tests/test_sample_rayjob_yamls.py --yaml-files ray_v1alpha1_rayjob.yaml ray-job.custom-head-svc.yaml

In summary, this PR can save developers time and provide more flexible testing options.

Related issue number

#1812 (comment)

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

Following is the screenshot of manual test

  1. [Default] Run all YAML files.
    yaml_default
    yaml_default_res

  2. Specified two YAML files.
    yaml_specify
    yaml_specify_res

@kevin85421 kevin85421 self-requested a review January 18, 2024 22:09
@kevin85421 kevin85421 self-assigned this Jan 18, 2024
@kevin85421
Copy link
Member

Would you mind rebasing with the master branch? Thanks!

@kevin85421
Copy link
Member

cc @rueian would you mind reviewing this PR?

@anyscalesam anyscalesam added the enhancement New feature or request label Jan 29, 2024
@rueian
Copy link
Contributor

rueian commented Jan 31, 2024

Sure. The changes align with the latest test_sample_raycluster_yamls.py and look good to me.

So now, we skip untracked rayjob tests as well. I found this behavior is quite implicit and silent which may take some time to debug why a test never be executed. In follow-up PRs, I would recommend we also log a warning message, like the [SKIP] one, to notify developers there are some untracked tests skipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants