-
Notifications
You must be signed in to change notification settings - Fork 410
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
[Feature][kubectl-plugin] e2e test for 'kubectl ray log' #2486
base: master
Are you sure you want to change the base?
Conversation
Expect(strings.TrimSpace(string(output))).Should(MatchRegexp(expectedOutputStringFormat)) | ||
|
||
// Check that the log directory exists | ||
logDirInfo, err := os.Stat(expectedDirPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewsykim are there files that you think we should check that is for sure retrieved? Eg stdout.log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to check the contents of some log files that exist in every standard Ray cluster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It checks for "stdout.log" and "raylet.out" and a specific log content. PTAL!
3d02506
to
9fc0313
Compare
1c1d121
to
fb97861
Compare
|
||
var _ = Describe("Calling ray plugin `log` command on Ray Cluster", Ordered, func() { | ||
It("succeed in retrieving all ray cluster logs", func() { | ||
expectedDirPath := "./raycluster-sample" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we first change current directory into a temp folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new directory is created for the raycluster and all the necessary logs should stay in there so I don't think there's a need to create a temp folder. Would creating a temp folder be safer/easier for cleanup? WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be eaiser for cleanup. You only need to delete one folder at the end of the tests instead of using an AfterEach
block.
fb97861
to
a135c0f
Compare
Why are these changes needed?
This adds e2e test for
kubectl ray log
.Related issue number
Checks