-
Notifications
You must be signed in to change notification settings - Fork 86
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
Test Report Dir Bug Fix #364
Conversation
Signed-off-by: Ken Sipe <kensipe@gmail.com>
@@ -241,6 +236,19 @@ func (ts *Testsuites) Report(dir, name string, ftype Type) error { | |||
} | |||
} | |||
|
|||
func ensureDir(dir string) error { |
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.
Wouldn't it be easier if this func just had an early exit if dir == ""
? To avoid the nesting in the calling func.
Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com>
pkg/report/report.go
Outdated
@@ -241,6 +234,20 @@ func (ts *Testsuites) Report(dir, name string, ftype Type) error { | |||
} | |||
} | |||
|
|||
func ensureDir(dir string) error { | |||
if dir != "" { |
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.
if dir != "" { | |
if dir == "" { |
Signed-off-by: Ken Sipe <kensipe@gmail.com>
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.
LGTM
A newly added feature was to create the directory for test reports if it didn't exist. The default if not specified location of the test report was "", which equated to the current directory. The check on "" existence was false which lead to the mkdir "" which is obvious going to fail. The solution is to verify that the artifactsDir isn't "" prior to that logic. Signed-off-by: Ken Sipe <kensipe@gmail.com> Signed-off-by: Israel Blancas <iblancasa@gmail.com>
A newly added feature was to create the directory for test reports if it didn't exist. The default if not specified location of the test report was "", which equated to the current directory. The check on "" existence was false which lead to the mkdir "" which is obvious going to fail. The solution is to verify that the artifactsDir isn't "" prior to that logic. Signed-off-by: Ken Sipe <kensipe@gmail.com> Signed-off-by: Israel Blancas <iblancasa@gmail.com>
A newly added feature was to create the directory for test reports if it didn't exist. The default if not specified location of the test report was "", which equated to the current directory. The check on "" existence was false which lead to the
mkdir ""
which is obvious going to fail. The solution is to verify that the artifactsDir isn't "" prior to that logic.Thanks to the community Erik and @iblancasa for helping discover and track down so quickly. We will be getting a patch release out today.
Signed-off-by: Ken Sipe kensipe@gmail.com