Option to print output of passed tests in Reporters #586

manosnoam · 2019-06-25T14:51:19Z

This is PR for issue #583

A new CLI option: -ginkgo.reportPassed :
It will print output for each passed test in the generated report,
including JUnit, Teamcity, and Default reporters.

For example, in JUnit (XML), the test output will be added under:
<testcase> <passed>

The default behavior (without this option), prints test output
only if the test case (spec) has failed.

manosnoam · 2019-06-27T08:09:20Z

Hi @williammartin,
I saw you've reviewed #571 (Print Skip reason in JUnit reporter if one was provided),
so it'll be great if you could review this PR as well.
I've added this feature test into reporters/default_reporter_test.go

Please see issue #583 for more details.
Thanks!

williammartin · 2019-06-27T08:49:37Z

Hi @manosnoam,

I think the general idea of enabling passed to be included in the output makes sense, thanks.

I think there's probably some missing tests in junit_reporter_test.go that covers the additional logic in junit_reporter.go. As far as I can see here the only test is that the flag gets passed through.

That said, I don't love the idea of introducing a reporter specific flag. I wonder whether any of the following options are better:

Make use of the existing -v verbose flag (has slightly different semantics but similar outcome)
Provide a new flag -passed which would work for all reporters (would have to implement for all reporters)
Make this a compile time configuration (would not be toggleable and wouldn't work with old test suites) e.g.

junitReporter := reporters.NewJUnitReporterWithConfig("junit.xml", { passed: true })

I think there are advantages and disadvantages to all of these options. Let me know your thoughts.

manosnoam · 2019-06-27T09:29:49Z

@williammartin, thanks for super fast reply!
I think -v (verbose) is usually for detailed console output, but not for the generated reports.
-passed is what I've implemented, but I called it -junitPassed.

I guess there's a way to add test of this flag, can you suggest how to test this CLI flag ?

williammartin · 2019-06-27T09:43:43Z

I think -v (verbose) is usually for detailed console output, but not for the generated reports.

I think these are maybe just two sides of the same coin. The console output is the default reporter that you can combine with other reporters. What I'm suggesting is maybe -v should be used to signify the same thing (but I'm not convinced they are exactly equivalent which is why I might favour -passed).

-passed is what I've implemented, but I called it -junitPassed

yeh I think this what bothers me though, for example when I look at https://github.com/onsi/ginkgo/pull/586/files#diff-245d401c0f93bef3b7927b60d3418905L303 there's no reason that a reporter abstraction should leak into this thing that knows how to run specs.

Similarly, this is behaviour that should probably not be solely useful for the junit reporter but also for default or teamcity (or any other custom reporter).

I guess there's a way to add test of this flag, can you suggest how to test this CLI flag ?

Ideally we would have an integration test that actually exercises a test suite using this feature, but at the very least I would expect to see something in https://github.com/onsi/ginkgo/blob/master/reporters/junit_reporter_test.go#L79-L92 that ensures a PassedMessage is set.

Hope this helps.

manosnoam · 2019-06-27T14:29:59Z

What about renaming CLI flag to -reportPassed, and in config/config.go to ReportPassed bool ?
In specrunner/spec_runner.go you're right, we don't really need to interact with DefaultReporterConfig.
The functionality works without it, so all we need is to append output to the summary:
if len(summary.CapturedOutput) == 0 { summary.CapturedOutput = string(runner.writer.Bytes()) }
Actually, the flag is already evaluated in juinit_reporter.go: https://github.com/onsi/ginkgo/pull/586/files#diff-87479b8a1ad5040d4d2558e52c89cf07R115
I'll add same condition check to teamcity_reporter.go and default_reporter.go
I will add integration tests to these as well.

manosnoam · 2019-06-27T22:59:08Z

Hi @williammartin, @joestringer
I've implemented the requirements from my last comment, and added relevant tests under:

internal/specrunner/spec_runner_test.go
reporters/default_reporter_test.go
reporters/junit_reporter_test.go
reporters/teamcity_reporter_test.go

All Travis-CI tests are now passing.

I don't see the "Request" button to ask for review, not sure what's the process here...
Can you please review ?

williammartin · 2019-07-04T15:52:27Z

I had a play around and it seems to make sense to me. Thanks!

williammartin · 2019-07-04T15:53:29Z

I think there's something funny going on with the commits in this PR though, can you rebase to just one commit describing this change?

A new CLI option: -ginkgo.reportPassed It will print output for each passed test in the generated report, including JUnit, Teamcity, and Default reporters. For example, in JUnit (XML), the test output will be added under: `<testcase> <passed>` The default behavior (without this option), prints test output only if the test case (spec) has failed. [Fixes onsi#583]

manosnoam · 2019-07-07T08:55:42Z

I think there's something funny going on with the commits in this PR though, can you rebase to just one commit describing this change?

Thanks @williammartin, got this fixed and rebased.
CI passed.

Can it be merged now ?

In onsi#586, the JUnit reporter was extended to capture the output of passing tests when the -reportPassing flag is used. However, the passed element is not in the JUnit XML schema. While that alone might a theoretical problem, it is also a practical problem, because some parsers, e.g., the TeamCity XML Reporting Plugin JUnit parser, conform to that schema, ignore the element, and show no output for passing tests. The fix seems to be to use the system-out element instead. I have confirmed that, when using the system-out attribute, the TeamCity XML Reporting Plugin Junit parser shows output for a passing test.

In #586, the JUnit reporter was extended to capture the output of passing tests when the -reportPassing flag is used. However, the passed element is not in the JUnit XML schema. While that alone might a theoretical problem, it is also a practical problem, because some parsers, e.g., the TeamCity XML Reporting Plugin JUnit parser, conform to that schema, ignore the element, and show no output for passing tests. The fix seems to be to use the system-out element instead. I have confirmed that, when using the system-out attribute, the TeamCity XML Reporting Plugin Junit parser shows output for a passing test.

manosnoam force-pushed the junit_passed branch from d2cf5f6 to 26f0456 Compare June 25, 2019 15:35

manosnoam force-pushed the junit_passed branch 5 times, most recently from 9378a22 to 07137e3 Compare June 27, 2019 22:45

manosnoam force-pushed the junit_passed branch 2 times, most recently from 82dcb6b to 5860cdd Compare June 30, 2019 12:36

manosnoam changed the title ~~Option to print output of passed tests in Junit XML result~~ Option to print output of passed tests in Reportes Jun 30, 2019

manosnoam mentioned this pull request Jun 30, 2019

Option to print output of passed tests in Reporters #583

Closed

manosnoam changed the title ~~Option to print output of passed tests in Reportes~~ Option to print output of passed tests in Reporters Jun 30, 2019

manosnoam force-pushed the junit_passed branch from 5860cdd to f735d68 Compare July 1, 2019 16:24

manosnoam added a commit to manosnoam/ginkgo that referenced this pull request Jul 4, 2019

Doc update for PR onsi#586

4420e7e

manosnoam mentioned this pull request Jul 4, 2019

Doc update for PR #586 #587

Merged

manosnoam force-pushed the junit_passed branch from f735d68 to 8108c70 Compare July 7, 2019 08:27

manosnoam force-pushed the junit_passed branch from 8108c70 to 7b45017 Compare July 7, 2019 08:40

williammartin merged commit 0545415 into onsi:master Jul 16, 2019

williammartin pushed a commit that referenced this pull request Aug 10, 2019

Doc update for PR #586

224d826

manosnoam deleted the junit_passed branch August 11, 2019 07:19

dlipovetsky mentioned this pull request Feb 10, 2021

JUnit reporter reports passing tests using an element that is not in the JUnit XML schema #768

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option to print output of passed tests in Reporters #586

Option to print output of passed tests in Reporters #586

manosnoam commented Jun 25, 2019 •

edited

manosnoam commented Jun 27, 2019

williammartin commented Jun 27, 2019

manosnoam commented Jun 27, 2019

williammartin commented Jun 27, 2019

manosnoam commented Jun 27, 2019 •

edited

manosnoam commented Jun 27, 2019 •

edited

williammartin commented Jul 4, 2019

williammartin commented Jul 4, 2019

manosnoam commented Jul 7, 2019 •

edited

Option to print output of passed tests in Reporters #586

Option to print output of passed tests in Reporters #586

Conversation

manosnoam commented Jun 25, 2019 • edited

manosnoam commented Jun 27, 2019

williammartin commented Jun 27, 2019

manosnoam commented Jun 27, 2019

williammartin commented Jun 27, 2019

manosnoam commented Jun 27, 2019 • edited

manosnoam commented Jun 27, 2019 • edited

williammartin commented Jul 4, 2019

williammartin commented Jul 4, 2019

manosnoam commented Jul 7, 2019 • edited

manosnoam commented Jun 25, 2019 •

edited

manosnoam commented Jun 27, 2019 •

edited

manosnoam commented Jun 27, 2019 •

edited

manosnoam commented Jul 7, 2019 •

edited