Repository: kudobuilder/kuttl (go)

Issue:
Optional more granular reporting for JUnit reports
<!-- Please only use this template for submitting enhancement requests.
Implementing your enhancement will follow the KEP process: https://github.com/kudobuilder/kuttl/blob/main/keps/0001-kep-process.md
-->

**What would you like to be added**:

https://github.com/kudobuilder/kuttl/pull/509 changed the format of the JUnit reports to be more granular. A statement at the end of the Caveats section says 

> If needed, I could add a knob to make this high-granularity opt-in.in the end 

This request is about introducing such knob to allow people to use the previous, less granular reporting format.

**Why is this needed**:

This potential need was already highlighted in the commit which changed the format, as highlighted above. I'd like to add that in the project I contribute, OpenStack Kubernetes Operators, we are starting to collect the results of KUTTL tests in a consistent way for internal reporting. The way each operator structured its tests is to have to have a basic mapping of 1-1 between tests and features. We don't want nor need to mark each step for reporting. So instead of having to post-process the JUnitXML file, it would be nice if kuttl could produce also (with some flag) the report in the old format.


Pull request:
feat: Configurable report granularity
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://github.com/kudobuilder/kuttl/blob/main/CONTRIBUTING.md
2. Make sure you have added and ran the tests before submitting your PR
3. If the PR is unfinished, start it as a Draft PR: https://github.blog/2019-02-14-introducing-draft-pull-requests/
-->

**What this PR does / why we need it**:

https://github.com/kudobuilder/kuttl/pull/509 changed the report format from test to step granularity. This change introduces a setting that makes it possible to use the previous granularity.

Technically speaking the previous change was backwards incompatible, but a few releases happened since then so I'm keeping the default at step granularity to prevent another backwards incompatible change.

TODO:
- [x] Merge #580 first
- [x] Rebase on `main`

<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #575 


Patch:
diff --git a/pkg/report/report.go b/pkg/report/report.go
index 6bf3def..a7cac16 100644
--- a/pkg/report/report.go
+++ b/pkg/report/report.go
@@ -87,8 +87,9 @@ type Testsuite struct {
 	// Testcases is a collection of test cases.
 	Testcases []*Testcase `xml:"testcase" json:"testcase,omitempty"`
 	// SubSuites is a collection of child test suites.
-	SubSuites []*Testsuite `xml:"testsuite" json:"testsuite,omitempty"`
-	lock      sync.Mutex
+	SubSuites         []*Testsuite `xml:"testsuite" json:"testsuite,omitempty"`
+	lock              sync.Mutex
+	reportGranularity string
 }
 
 // Testsuites is a collection of Testsuite and defines the rollup summary of all stats.
@@ -135,9 +136,9 @@ func NewSuiteCollection(name string) *Testsuites {
 }
 
 // NewSuite returns the address of a newly created TestSuite
-func NewSuite(name string) *Testsuite {
+func NewSuite(name string, reportGranularity string) *Testsuite {
 	start := time.Now()
-	return &Testsuite{Name: name, Timestamp: start}
+	return &Testsuite{Name: name, Timestamp: start, reportGranularity: reportGranularity}
 }
 
 // NewCase returns the address of a newly create Testcase
@@ -190,7 +191,7 @@ func (ts *Testsuite) AddProperty(property Property) {
 
 // NewSubSuite creates a new child suite and returns it.
 func (ts *Testsuite) NewSubSuite(name string) *Testsuite {
-	s := NewSuite(name)
+	s := NewSuite(name, "")
 	ts.lock.Lock()
 	defer ts.lock.Unlock()
 	ts.SubSuites = append(ts.SubSuites, s)
@@ -223,9 +224,15 @@ func (ts *Testsuite) summarize() time.Time {
 	return end
 }
 
-func (ts *Testsuite) NewTest(name string) TestReporter {
-	subSuite := ts.NewSubSuite(name)
-	return &testReporter{suite: subSuite}
+func (ts *Testsuite) NewTestReporter(name string) TestReporter {
+	switch ts.reportGranularity {
+	case "test":
+		tc := NewCase(name)
+		return &testReporter{testCase: tc, suite: ts}
+	default:
+		subSuite := ts.NewSubSuite(name)
+		return &testReporter{suite: subSuite}
+	}
 }
 
 type stepReport struct {
@@ -246,9 +253,17 @@ func (s *stepReport) AddAssertions(i int) {
 	s.assertions += i
 }
 
+func (s *stepReport) populate(testCase *Testcase) {
+	if s.failed {
+		testCase.Failure = NewFailure(s.failureMsg, s.errors)
+	}
+	testCase.Assertions += s.assertions
+}
+
 type testReporter struct {
 	suite       *Testsuite
 	stepReports []*stepReport
+	testCase    *Testcase
 }
 
 func (r *testReporter) Step(stepName string) StepReporter {
@@ -258,12 +273,18 @@ func (r *testReporter) Step(stepName string) StepReporter {
 }
 
 func (r *testReporter) Done() {
+	if r.testCase != nil {
+		// Reporting with test granularity.
+		for _, report := range r.stepReports {
+			report.populate(r.testCase)
+		}
+		r.suite.AddTestcase(r.testCase)
+		return
+	}
+	// Reporting with step granularity.
 	for _, report := range r.stepReports {
 		testCase := NewCase(report.name)
-		if report.failed {
-			testCase.Failure = NewFailure(report.failureMsg, report.errors)
-		}
-		testCase.Assertions += report.assertions
+		report.populate(testCase)
 		r.suite.AddTestcase(testCase)
 	}
 }
@@ -346,13 +367,6 @@ func ensureDir(dir string) error {
 	return err
 }
 
-// NewSuite creates and assigns a TestSuite to the TestSuites (then returns the suite)
-func (ts *Testsuites) NewSuite(name string) *Testsuite {
-	suite := NewSuite(name)
-	ts.AddTestSuite(suite)
-	return suite
-}
-
 // SetFailure adds a failure to the TestSuites collection for startup failures in the test harness
 func (ts *Testsuites) SetFailure(message string) {
 	ts.Failure = &Failure{


Test patch:
diff --git a/docs/testing/reference.md b/docs/testing/reference.md
index f3d6e3f..50a949d 100644
--- a/docs/testing/reference.md
+++ b/docs/testing/reference.md
@@ -35,6 +35,7 @@ artifactsDir      | string           | The directory to output artifacts to (cur
 commands          | list of [Commands](#commands) | Commands to run prior to running the tests.                                   | []
 kindContainers    | list of strings  | List of Docker images to load into the KIND cluster once it is started.                  | []
 reportFormat      | string           | Determines the report format. If empty, no report is generated. One of: JSON, XML.       |
+reportGranularity | string           | What granularity to report failures at. One of: `step`, `test`.                          | `step`
 reportName        | string           | The name of report to create. This field is not used unless reportFormat is set.         | "kuttl-test"
 namespace         | string           | The namespace to use for tests. This namespace will be created if it does not exist and removed if it was created (unless `skipDelete` is set). If no namespace is set, one will be auto-generated. |
 suppress          | list of strings  | Suppresses log collection of the specified types. Currently only `events` is supported.  |
diff --git a/pkg/apis/testharness/v1beta1/test_types.go b/pkg/apis/testharness/v1beta1/test_types.go
index 28d3c51..722076c 100644
--- a/pkg/apis/testharness/v1beta1/test_types.go
+++ b/pkg/apis/testharness/v1beta1/test_types.go
@@ -84,6 +84,10 @@ type TestSuite struct {
 
 	// ReportName defines the name of report to create.  It defaults to "kuttl-report" and is not used unless ReportFormat is defined.
 	ReportName string `json:"reportName"`
+
+	// ReportGranularity defines the granularity at which failures are reported. It defaults to "step".
+	ReportGranularity string `json:"reportGranularity"`
+
 	// Namespace defines the namespace to use for tests
 	// The value "" means to auto-generate tests namespaces, these namespaces will be created and removed for each test
 	// Any other value is the name of the namespace to use.  This namespace will be created if it does not exist and will
diff --git a/pkg/kuttlctl/cmd/test.go b/pkg/kuttlctl/cmd/test.go
index 9b80db6..e708b8c 100644
--- a/pkg/kuttlctl/cmd/test.go
+++ b/pkg/kuttlctl/cmd/test.go
@@ -59,6 +59,7 @@ func newTestCmd() *cobra.Command { //nolint:gocyclo
 	timeout := 30
 	reportFormat := ""
 	reportName := "kuttl-report"
+	reportGranularity := "kuttl-report"
 	namespace := ""
 	suppress := []string{}
 	var runLabels labelSetValue
@@ -183,6 +184,13 @@ For more detailed documentation, visit: https://kuttl.dev`,
 				options.ReportName = reportName
 			}
 
+			if isSet(flags, "report-granularity") {
+				if reportGranularity != "step" && reportGranularity != "test" {
+					return fmt.Errorf("unrecognized report granularity %q", reportGranularity)
+				}
+				options.ReportGranularity = reportGranularity
+			}
+
 			if isSet(flags, "artifacts-dir") {
 				options.ArtifactsDir = artifactsDir
 			}
@@ -257,6 +265,7 @@ For more detailed documentation, visit: https://kuttl.dev`,
 	testCmd.Flags().IntVar(&timeout, "timeout", 30, "The timeout to use as default for TestSuite configuration.")
 	testCmd.Flags().StringVar(&reportFormat, "report", "", "Specify JSON|XML for report.  Report location determined by --artifacts-dir.")
 	testCmd.Flags().StringVar(&reportName, "report-name", "kuttl-report", "Name for the report.  Report location determined by --artifacts-dir and report file type determined by --report.")
+	testCmd.Flags().StringVar(&reportGranularity, "report-granularity", "step", "Report granularity. Can be 'step' (default) or 'test'.")
 	testCmd.Flags().StringVarP(&namespace, "namespace", "n", "", "Namespace to use for tests. Provided namespaces must exist prior to running tests.")
 	testCmd.Flags().StringSliceVar(&suppress, "suppress-log", []string{}, "Suppress logging for these kinds of logs (events).")
 	testCmd.Flags().Var(&runLabels, "test-run-labels", "Labels to use for this test run.")
diff --git a/pkg/test/harness.go b/pkg/test/harness.go
index 51a23b4..56af654 100644
--- a/pkg/test/harness.go
+++ b/pkg/test/harness.go
@@ -379,7 +379,7 @@ func (h *Harness) RunTests() {
 
 	h.T.Run("harness", func(t *testing.T) {
 		for testDir, tests := range realTestSuite {
-			suiteReport := h.report.NewSuite(testDir)
+			suiteReport := h.NewSuiteReport(testDir)
 			for _, test := range tests {
 				test := test
 
@@ -397,8 +397,7 @@ func (h *Harness) RunTests() {
 						t.Fatal(err)
 					}
 
-					testReporter := suiteReport.NewTest(test.Name)
-					test.Run(t, testReporter)
+					test.Run(t, suiteReport.NewTestReporter(test.Name))
 				})
 			}
 		}
@@ -612,6 +611,13 @@ func (h *Harness) Report() {
 	}
 }
 
+// NewSuiteReport creates and assigns a TestSuite to the TestSuites (then returns the suite),
+func (h *Harness) NewSuiteReport(name string) *report.Testsuite {
+	suite := report.NewSuite(name, h.TestSuite.ReportGranularity)
+	h.report.AddTestSuite(suite)
+	return suite
+}
+
 // reportName returns the configured ReportName.
 func (h *Harness) reportName() string {
 	if h.TestSuite.ReportName != "" {
diff --git a/test/junit/.gitignore b/test/junit/.gitignore
index fa0c69d..283bcb7 100644
--- a/test/junit/.gitignore
+++ b/test/junit/.gitignore
@@ -1,6 +1,12 @@
 /kuttl-ouput-step-json.txt
 /kuttl-ouput-step-xml.txt
+/kuttl-ouput-test-json.txt
+/kuttl-ouput-test-xml.txt
 /kuttl-report-step.json
 /kuttl-report-step.json.normalized
 /kuttl-report-step.xml
 /kuttl-report-step.xml.normalized
+/kuttl-report-test.json
+/kuttl-report-test.json.normalized
+/kuttl-report-test.xml
+/kuttl-report-test.xml.normalized
diff --git a/test/junit/Makefile b/test/junit/Makefile
index 1c8f6f6..c4df1f3 100644
--- a/test/junit/Makefile
+++ b/test/junit/Makefile
@@ -2,18 +2,28 @@
 test:
 	$(MAKE) -C ../../ cli
 	rm -f kuttl-report-step.xml.normalized kuttl-report-step.json.normalized
-	../../bin/kubectl-kuttl test --config /dev/null --start-control-plane --report-name kuttl-report-step --timeout 10 --report xml suite1 suite2 > kuttl-ouput-step-xml.txt 2>&1 || true # this is meant to fail
+	rm -f kuttl-report-test.xml.normalized kuttl-report-test.json.normalized
+	../../bin/kubectl-kuttl test --config /dev/null --start-control-plane --report-name kuttl-report-step --report-granularity step --timeout 10 --report xml suite1 suite2 > kuttl-ouput-step-xml.txt 2>&1 || true # this is meant to fail
 	if [ ! -e kuttl-report-step.xml ]; then cat kuttl-output-step-xml.txt; exit 1; fi
-	../../bin/kubectl-kuttl test --config /dev/null --start-control-plane --report-name kuttl-report-step --timeout 10 --report json suite1 suite2 > kuttl-ouput-step-json.txt 2>&1 || true # this is meant to fail
+	../../bin/kubectl-kuttl test --config /dev/null --start-control-plane --report-name kuttl-report-step --report-granularity step --timeout 10 --report json suite1 suite2 > kuttl-ouput-step-json.txt 2>&1 || true # this is meant to fail
 	if [ ! -e kuttl-report-step.json ]; then cat kuttl-output-step-json.txt; exit 1; fi
+	../../bin/kubectl-kuttl test --config /dev/null --start-control-plane --report-name kuttl-report-test --report-granularity test --timeout 10 --report xml suite1 suite2 > kuttl-ouput-test-xml.txt 2>&1 || true # this is meant to fail
+	if [ ! -e kuttl-report-test.xml ]; then cat kuttl-output-test-xml.txt; exit 1; fi
+	../../bin/kubectl-kuttl test --config /dev/null --start-control-plane --report-name kuttl-report-test --report-granularity test --timeout 10 --report json suite1 suite2 > kuttl-ouput-test-json.txt 2>&1 || true # this is meant to fail
+	if [ ! -e kuttl-report-test.json ]; then cat kuttl-output-test-json.txt; exit 1; fi
 	$(MAKE) kuttl-report-step.xml.normalized kuttl-report-step.json.normalized
+	$(MAKE) kuttl-report-test.xml.normalized kuttl-report-test.json.normalized
 	diff -u kuttl-report-step.xml.golden kuttl-report-step.xml.normalized
 	diff -u kuttl-report-step.json.golden kuttl-report-step.json.normalized
+	diff -u kuttl-report-test.xml.golden kuttl-report-test.xml.normalized
+	diff -u kuttl-report-test.json.golden kuttl-report-test.json.normalized
 
 .PHONY: update-golden
 update-golden:
 	cp kuttl-report-step.json.normalized kuttl-report-step.json.golden
 	cp kuttl-report-step.xml.normalized kuttl-report-step.xml.golden
+	cp kuttl-report-test.json.normalized kuttl-report-test.json.golden
+	cp kuttl-report-test.xml.normalized kuttl-report-test.xml.golden
 
 # The following targets replace all timestamps and durations with dummy values to make comparisons easy.
 
diff --git a/test/junit/kuttl-report-test.json.golden b/test/junit/kuttl-report-test.json.golden
new file mode 100644
index 0000000..7a87bf9
--- /dev/null
+++ b/test/junit/kuttl-report-test.json.golden
@@ -0,0 +1,78 @@
+{
+   "name": "",
+   "tests": 6,
+   "failures": 4,
+   "time": "1.0",
+   "testsuite": [
+     {
+       "tests": 3,
+       "failures": 2,
+       "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+       "time": "1.0",
+       "name": "suite1",
+       "testcase": [
+         {
+           "classname": "suite1",
+           "name": "test0",
+           "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+           "time": "1.0"
+         },
+         {
+           "classname": "suite1",
+           "name": "test1",
+           "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+           "time": "1.0",
+           "failure": {
+             "text": "command \"echo step stdout\\\\n echo \u003e\u00262 step stderr\\\\n false\" failed, exit status 1",
+             "message": "failed in step 2-run"
+           }
+         },
+         {
+           "classname": "suite1",
+           "name": "test2",
+           "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+           "time": "1.0",
+           "failure": {
+             "text": "command \"echo assert stdout\\\\n echo \u003e\u00262 assert stderr\\\\n false\" failed, exit status 1",
+             "message": "failed in step 1-run"
+           }
+         }
+       ]
+     },
+     {
+       "tests": 3,
+       "failures": 2,
+       "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+       "time": "1.0",
+       "name": "suite2",
+       "testcase": [
+         {
+           "classname": "suite2",
+           "name": "test0",
+           "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+           "time": "1.0"
+         },
+         {
+           "classname": "suite2",
+           "name": "test1",
+           "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+           "time": "1.0",
+           "failure": {
+             "text": "command \"echo step stdout\\\\n echo \u003e\u00262 step stderr\\\\n false\" failed, exit status 1",
+             "message": "failed in step 2-run"
+           }
+         },
+         {
+           "classname": "suite2",
+           "name": "test2",
+           "timestamp": "2000-01-01T00:00:00.00000000+00:00",
+           "time": "1.0",
+           "failure": {
+             "text": "command \"echo assert stdout\\\\n echo \u003e\u00262 assert stderr\\\\n false\" failed, exit status 1",
+             "message": "failed in step 1-run"
+           }
+         }
+       ]
+     }
+   ]
+ }
\ No newline at end of file
diff --git a/test/junit/kuttl-report-test.xml.golden b/test/junit/kuttl-report-test.xml.golden
new file mode 100644
index 0000000..4be0143
--- /dev/null
+++ b/test/junit/kuttl-report-test.xml.golden
@@ -0,0 +1,20 @@
+ <testsuites name="" tests="6" failures="4" time="1.0">
+   <testsuite tests="3" failures="2" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" name="suite1">
+     <testcase classname="suite1" name="test0" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" assertions="0"></testcase>
+     <testcase classname="suite1" name="test1" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" assertions="0">
+       <failure message="failed in step 2-run" type="">command &#34;echo step stdout\\n echo &gt;&amp;2 step stderr\\n false&#34; failed, exit status 1</failure>
+     </testcase>
+     <testcase classname="suite1" name="test2" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" assertions="0">
+       <failure message="failed in step 1-run" type="">command &#34;echo assert stdout\\n echo &gt;&amp;2 assert stderr\\n false&#34; failed, exit status 1</failure>
+     </testcase>
+   </testsuite>
+   <testsuite tests="3" failures="2" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" name="suite2">
+     <testcase classname="suite2" name="test0" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" assertions="0"></testcase>
+     <testcase classname="suite2" name="test1" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" assertions="0">
+       <failure message="failed in step 2-run" type="">command &#34;echo step stdout\\n echo &gt;&amp;2 step stderr\\n false&#34; failed, exit status 1</failure>
+     </testcase>
+     <testcase classname="suite2" name="test2" timestamp="2000-01-01T00:00:00.00000000+00:00" time="1.0" assertions="0">
+       <failure message="failed in step 1-run" type="">command &#34;echo assert stdout\\n echo &gt;&amp;2 assert stderr\\n false&#34; failed, exit status 1</failure>
+     </testcase>
+   </testsuite>
+ </testsuites>
\ No newline at end of file