[Feature][kubectl-plugin] e2e test for 'kubectl ray log' #2486
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.
@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.
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.
It checks for "stdout.log" and "raylet.out" and a specific log content. PTAL!
chiayi
force-pushed
the
kubectl-plugin
branch
3 times, most recently
from
3d02506
to
9fc0313
Compare
chiayi
force-pushed
the
kubectl-plugin
branch
3 times, most recently
from
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.
Should we first change current directory into a temp folder?
There was a problem hiding this comment.
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.
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.
There was a problem hiding this comment.
We would still need to do a cleanup after each 'It' block right? Since we want to make sure previous downloaded log files don't effect proceeding calls.
There was a problem hiding this comment.
Hmm. Actually I originally thought that some It didn't create folders, but you still executed os.RemoveAll.
chiayi
force-pushed
the
kubectl-plugin
branch
from
fb97861
to
a135c0f
Compare
Why are these changes needed?
This adds e2e test for
kubectl ray log.Related issue number
Checks