feat: issue#647 export log to file #938
Conversation
|
Hey @alonzyl :) on validation should i try to write in the directory to check if there are righting rights? maybe not limit the file saving only to json, allow all supported output types. Thanks! |
|
Hi, I updated the code with the comments above. Would like to hear you opinion :) |
|
@alonzyl Thank you for submitting this PR! 🚀 We will review it in the upcoming days :) |
cmd/test/main.go
Outdated
| err = evaluation.PrintResults(evaluationData) | ||
|
|
||
| if testCommandData.SaveResults != "" { | ||
| if evaluation.IsWritableDirectory(testCommandData.SaveResults) { |
There was a problem hiding this comment.
This function looks a little redundant to me, I mean why not just try to writeFile and if an error is returned then just return the error?
The error returned from this should look like this, which is understandable enough, WDYT?
@alonzyl
cmd/test/main.go
Outdated
|
|
||
| err = ioutil.WriteFile(testCommandData.SaveResults, []byte(resultsText), 0666) | ||
| if err != nil { | ||
| fmt.Println(err) |
There was a problem hiding this comment.
This print seems a bit redundant as well, since the error returned will be printed out
pkg/evaluation/save_results_flag.go
Outdated
| "golang.org/x/sys/unix" | ||
| ) | ||
|
|
||
| func IsWritableDirectory(filePath string) bool { |
There was a problem hiding this comment.
If you indeed don't use this function, This whole file can be deleted as well, less code makes it simpler
| func test_testCommand_save_results_flag(t *testing.T, ctx *TestCommandContext) { | ||
| flags := TestCommandFlags{SaveResults: "./"} | ||
| err := flags.Validate() | ||
| assert.NoError(t, err) |
There was a problem hiding this comment.
Optional - You could add a test case for when there is a wrong path and an error is thrown
shalev007
changed the title
shalev007
changed the title
|
Hey @alonzyl thanks for the contribution! 🙏 |
|
Hi, I totally agree with your remarks and have made the necessary changes accordingly. If there are more things that I can improve, I would be happy to make further changes :) |
|
@alonzyl Thanks you so much for the changes, it looks great. |
I would appreciate your feedback on whether there are any areas that I could enhance this cr. Any suggestions for improvement would be greatly appreciated.
while writing the code i came across a few questions about the flow, I would be happy to change anything if you think it would be better.