-
-
Notifications
You must be signed in to change notification settings - Fork 650
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
Make generated Junit file compatable with "Maven Surefire" #488
Conversation
- add errors attribute to testsuite, currently I will set 0, but I think Ginkgo must report failures under setup and teardown methods as errors and not as failures - cut testsuite timestamp to three digits after the dot Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
@nodo Can you please take a look? |
Looks like |
@williammartin Thanks, missed this |
reporters/junit_reporter.go
Outdated
@@ -119,8 +121,9 @@ func (reporter *JUnitReporter) SpecDidComplete(specSummary *types.SpecSummary) { | |||
|
|||
func (reporter *JUnitReporter) SpecSuiteDidEnd(summary *types.SuiteSummary) { | |||
reporter.suite.Tests = summary.NumberOfSpecsThatWillBeRun | |||
reporter.suite.Time = summary.RunTime.Seconds() | |||
reporter.suite.Time, _ = strconv.ParseFloat(fmt.Sprintf("%.3f", summary.RunTime.Seconds()), 3) |
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.
Do you think we could use math.Trunc(x * 1000) / 1000
instead?
@williammartin @cynepco3hahue just added a minor comment. It feels a bit easier to read. I know it's not exactly like WDYT? |
@nodo I think it is totally fine, I just afraid to use |
Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
7fdb46e
to
5379cb5
Compare
@cynepco3hahue the change LGTM, thanks! One more thing, please could you add a unit test for the change? |
Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
@nodo Done |
Awesome, merging! Thanks for your contribution @cynepco3hahue ! |
Glad to contribute 😄 |
* Make generated Junit file compatable with "Maven Surefire" - add errors attribute to testsuite, currently I will set 0, but I think Ginkgo must report failures under setup and teardown methods as errors and not as failures - cut testsuite timestamp to three digits after the dot Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
@@ -119,8 +121,9 @@ func (reporter *JUnitReporter) SpecDidComplete(specSummary *types.SpecSummary) { | |||
|
|||
func (reporter *JUnitReporter) SpecSuiteDidEnd(summary *types.SuiteSummary) { | |||
reporter.suite.Tests = summary.NumberOfSpecsThatWillBeRun | |||
reporter.suite.Time = summary.RunTime.Seconds() | |||
reporter.suite.Time = math.Trunc(summary.RunTime.Seconds() * 1000 / 1000) |
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 commit message says "cut testsuite timestamp to three digits after the dot", but does this code really do that? The multiplication + division don't change the value, and then math.Trunc
just throws away all sub-second digits.
Perhaps you meant this:
math.Trunc(summary.RunTime.Seconds() * 1000) / 1000
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.
testsuite
, currently, I will set 0,but I think Ginkgo must report failures under setup and teardown
methods as errors and not as failures
testsuite
timestamp to three digits after the dotFixes #486
Signed-off-by: Artyom Lukianov alukiano@redhat.com