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
Adding Test Reports Feature #1573
Conversation
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, I'd prefer a default but no blockers
@@ -184,6 +190,7 @@ For more detailed documentation, visit: https://kudo.dev/docs/testing`, | |||
testCmd.Flags().StringVar(&kindConfig, "kind-config", "", "Specify the KIND configuration file path (implies --start-kind, cannot be used with --start-control-plane).") | |||
testCmd.Flags().StringVar(&kindContext, "kind-context", "", "Specify the KIND context name to use (default: kind).") | |||
testCmd.Flags().StringVar(&artifactsDir, "artifacts-dir", "", "Directory to output kind logs to (if not specified, the current working directory).") | |||
testCmd.Flags().StringVar(&reportFormat, "report", "", "Specify JSON|XML for report. Report location determined by --artifacts-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.
I think this should have a default, right? Otherwise you'd have to run it first to see if you get json or XML by default?
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.
the default is "" which is no report... by default no report is created.
@@ -150,6 +152,10 @@ For more detailed documentation, visit: https://kudo.dev/docs/testing`, | |||
options.ArtifactsDir = artifactsDir | |||
} | |||
|
|||
if isSet(flags, "report") { | |||
options.ReportFormat = strings.ToLower(reportFormat) |
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.
Will testutils.RunTests
complain if this option isn't "json" or "xml"? I.e. do we need our own validation?
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.
yeah... we should put some validation in kuttl for this... I'll create an issue.
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.
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.
to round out the conversation, there is NO invalid input... The default of "" is no report... but anything else defaults to json and "json" and "xml" are expected but lowercase. We will fix some of this in the kuttl issue listed.
Signed-off-by: Ken Sipe kensipe@gmail.com