-
Notifications
You must be signed in to change notification settings - Fork 509
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
Adds support for junit xml output #527
Conversation
Codecov Report
@@ Coverage Diff @@
## master #527 +/- ##
==========================================
+ Coverage 76.33% 76.98% +0.65%
==========================================
Files 97 98 +1
Lines 2303 2364 +61
==========================================
+ Hits 1758 1820 +62
+ Misses 405 404 -1
Partials 140 140
|
+1 interested by this feature |
code review looks good. I want to test a little in the morning before approving. |
pkg/writer/junit_xml.go
Outdated
func JUnitXMLWriter(data interface{}, writer io.Writer) error { | ||
output, ok := data.(policy.EngineOutput) | ||
if !ok { | ||
return fmt.Errorf("incorrect input for JunitXML writer, supportted type is policy.EngineOutput") |
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.
typo => supportted.
Also, the error message seems good for a debug level log but not for an error level log as the user should not be bothered to understand what is policy.EngineOutput
.
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 error is only for the developers, so that developers would know what type to pass
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.
alrighty
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.
can you correct the type though? @patilpankaj212
pkg/writer/junit_xml.go
Outdated
testCase = JUnitTestCase{Failure: new(JUnitFailure)} | ||
} | ||
testCase.Classname = v.File | ||
testCase.Name = "[ERROR] resource: " + fmt.Sprintf(`"%s"`, v.ResourceName) + " at line: " + fmt.Sprintf("%d", v.LineNumber) + " violates: " + v.RuleID |
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.
We can wrap this whole string under fmt.Sprintf
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.
skipMsg := new(JUnitSkipMessage)
skipMsg.Message = v.Comment
testCase = JUnitTestCase{Failure: new(JUnitFailure), SkipMessage: skipMsg}
Summary: results.ScanSummary{ | ||
ResourcePath: "test_resource_path", | ||
IacType: "k8s", | ||
Timestamp: "2020-12-12 11:21:29.902796 +0000 UTC", |
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.
@patilpankaj212 you sure about hardcoding the timestamp?
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.
Is there anything wrong with it?
@@ -55,6 +55,7 @@ type ScanSummary struct { | |||
LowCount int `json:"low" yaml:"low" xml:"low,attr"` | |||
MediumCount int `json:"medium" yaml:"medium" xml:"medium,attr"` | |||
HighCount int `json:"high" yaml:"high" xml:"high,attr"` | |||
TotalTime int64 `json:"-" yaml:"-" xml:"-"` |
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 this be left blank for encoding?
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.
This is done on purpose, it is not required for other output types.
pkg/writer/junit_xml_test.go
Outdated
return | ||
} | ||
if gotWriter := writer.String(); !strings.EqualFold(strings.TrimSpace(gotWriter), strings.TrimSpace(tt.wantWriter)) { | ||
fmt.Println(gotWriter) |
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.
@patilpankaj212 you forgot to remove these console log lines
pkg/writer/junit_xml.go
Outdated
testCase = JUnitTestCase{Failure: new(JUnitFailure)} | ||
} | ||
testCase.Classname = v.File | ||
testCase.Name = "[ERROR] resource: " + fmt.Sprintf(`"%s"`, v.ResourceName) + " at line: " + fmt.Sprintf("%d", v.LineNumber) + " violates: " + v.RuleID |
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.
testCase.Name = "[ERROR] resource: " + fmt.Sprintf(`"%s"`, v.ResourceName) + " at line: " + fmt.Sprintf("%d", v.LineNumber) + " violates: " + v.RuleID | |
testCase.Name = "[ERROR] resource: " + fmt.Sprintf(`"%s,"`, v.ResourceName) + " at line: " + fmt.Sprintf("%d", v.LineNumber) + " violates: rule " + v.RuleID |
35474a8
to
237c04e
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
</testcase> | ||
</testsuite> | ||
</testsuites> | ||
`, version.Get()) |
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.
good one
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
-added support for junit xml output
-unit tests for junit xml output
-for now we will have one test suite as part of output
-added few attributes (File, Severity, Line, Category) as test case attribute which is non standard junitXML
-added violation details as part of test case failure message