chore: pre-factor code related to junit reporting (#580)

* report ordering
* e2e test suite
* reporting abstraction

Signed-off-by: Marcin Owsiany <[email protected]>

.github/workflows/e2e-test.yml

@@ -0,0 +1,22 @@
+name: "End-to-end tests"
+
+on:
+  push:
+    branches:
+      - main
+      - releases/*
+  pull_request:
+    branches:
+      - main
+      - releases/*
+
+jobs:
+  e2e-tests:
+    runs-on: ubuntu-20.04
+    steps:
+    - uses: actions/[email protected]
+      with:
+        go-version: 1.21
+    - uses: actions/[email protected]
+    - name: "Run end-to-end tests"
+      run: make e2e-test

Makefile

@@ -122,7 +122,7 @@ todo: ## Shows todos from code
 ##@ Tests
 
 .PHONY: all
-all: lint test integration-test  ## Runs lint, unit and integration tests
+all: lint test integration-test e2e-test  ## Runs lint, unit, integration and e2e tests
 
 # Run unit tests
 .PHONY: test
@@ -139,6 +139,11 @@ endif
 integration-test: envtest  ## Runs integration tests
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" ./hack/run-integration-tests.sh
 
+.PHONY: e2e-test
+# Run e2e tests
+e2e-test: envtest  ## Runs end-to-end tests
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" $(MAKE) -C ./test/junit
+
 ##@ Build Dependencies
 
 ## Location to install dependencies to

pkg/report/report.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"sync"
 	"time"
 )
@@ -110,6 +111,21 @@ type Testsuites struct {
 	// communicate test infra failures, such as failed auth, or connection issues.
 	Failure *Failure `xml:"failure" json:"failure,omitempty"`
 	start   time.Time
+	lock    sync.Mutex
+}
+
+// StepReporter is an interface for reporting status of a test step.
+type StepReporter interface {
+	Failure(message string, errors ...error)
+	AddAssertions(i int)
+}
+
+// TestReporter is an interface for reporting status of a test.
+// For each step, call Step and use the returned step reporter.
+// Make sure to call Done when a test ends (preferably using defer).
+type TestReporter interface {
+	Step(stepName string) StepReporter
+	Done()
 }
 
 // NewSuiteCollection returns the address of a newly created TestSuites
@@ -178,6 +194,10 @@ func (ts *Testsuite) NewSubSuite(name string) *Testsuite {
 	ts.lock.Lock()
 	defer ts.lock.Unlock()
 	ts.SubSuites = append(ts.SubSuites, s)
+	// Ensure consistent ordering to make testing easier.
+	sort.Slice(ts.SubSuites, func(i, j int) bool {
+		return ts.SubSuites[i].Name < ts.SubSuites[j].Name
+	})
 	return s
 }
 
@@ -203,10 +223,64 @@ func (ts *Testsuite) summarize() time.Time {
 	return end
 }
 
+func (ts *Testsuite) NewTest(name string) TestReporter {
+	subSuite := ts.NewSubSuite(name)
+	return &testReporter{suite: subSuite}
+}
+
+type stepReport struct {
+	name       string
+	failed     bool
+	failureMsg string
+	errors     []error
+	assertions int
+}
+
+func (s *stepReport) Failure(message string, errors ...error) {
+	s.failed = true
+	s.failureMsg = message
+	s.errors = append(s.errors, errors...)
+}
+
+func (s *stepReport) AddAssertions(i int) {
+	s.assertions += i
+}
+
+type testReporter struct {
+	suite       *Testsuite
+	stepReports []*stepReport
+}
+
+func (r *testReporter) Step(stepName string) StepReporter {
+	step := &stepReport{name: stepName}
+	r.stepReports = append(r.stepReports, step)
+	return step
+}
+
+func (r *testReporter) Done() {
+	for _, report := range r.stepReports {
+		testCase := NewCase(report.name)
+		if report.failed {
+			testCase.Failure = NewFailure(report.failureMsg, report.errors)
+		}
+		testCase.Assertions += report.assertions
+		r.suite.AddTestcase(testCase)
+	}
+}
+
+var _ TestReporter = (*testReporter)(nil)
+var _ StepReporter = (*stepReport)(nil)
+
 // AddTestSuite is a convenience method to add a testsuite to the collection in testsuites
 func (ts *Testsuites) AddTestSuite(testsuite *Testsuite) {
 	// testsuite is added prior to stat availability, stat management in the close of the testsuites
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
 	ts.Testsuite = append(ts.Testsuite, testsuite)
+	// Ensure consistent ordering to make testing easier.
+	sort.Slice(ts.Testsuite, func(i, j int) bool {
+		return ts.Testsuite[i].Name < ts.Testsuite[j].Name
+	})
 }
 
 // AddProperty adds a property to a testsuites

pkg/report/report_test.go

@@ -67,9 +67,9 @@ AssertionError`,
 		},
 	}
 
-	x, _ := xml.MarshalIndent(suites, " ", "  ")
+	x, _ := xml.MarshalIndent(suites, " ", "  ") //nolint:govet
 	xout := string(x)
-	j, _ := json.MarshalIndent(suites, " ", "  ")
+	j, _ := json.MarshalIndent(suites, " ", "  ") //nolint:govet
 	jout := string(j)
 
 	xmlFile := filepath.Join("testdata", goldenXML+".golden")

pkg/test/case.go

@@ -316,19 +316,18 @@ func shortString(obj *corev1.ObjectReference) string {
 }
 
 // Run runs a test case including all of its steps.
-func (t *Case) Run(test *testing.T, ts *report.Testsuite) {
-	setupReport := report.NewCase("setup")
+func (t *Case) Run(test *testing.T, rep report.TestReporter) {
+	defer rep.Done()
+	setupReport := rep.Step("setup")
 	ns, err := t.determineNamespace()
 	if err != nil {
-		setupReport.Failure = report.NewFailure(err.Error(), nil)
-		ts.AddTestcase(setupReport)
+		setupReport.Failure(err.Error())
 		test.Fatal(err)
 	}
 
 	cl, err := t.Client(false)
 	if err != nil {
-		setupReport.Failure = report.NewFailure(err.Error(), nil)
-		ts.AddTestcase(setupReport)
+		setupReport.Failure(err.Error())
 		test.Fatal(err)
 	}
 
@@ -341,8 +340,7 @@ func (t *Case) Run(test *testing.T, ts *report.Testsuite) {
 
 		cl, err = newClient(testStep.Kubeconfig, testStep.Context)(false)
 		if err != nil {
-			setupReport.Failure = report.NewFailure(err.Error(), nil)
-			ts.AddTestcase(setupReport)
+			setupReport.Failure(err.Error())
 			test.Fatal(err)
 		}
 
@@ -353,15 +351,13 @@ func (t *Case) Run(test *testing.T, ts *report.Testsuite) {
 		if err = t.CreateNamespace(test, c, ns); k8serrors.IsAlreadyExists(err) {
 			t.Logger.Logf("namespace %q already exists, using kubeconfig %q", ns.Name, kc)
 		} else if err != nil {
-			setupReport.Failure = report.NewFailure("failed to create test namespace", []error{err})
-			ts.AddTestcase(setupReport)
+			setupReport.Failure("failed to create test namespace", err)
 			test.Fatal(err)
 		}
 	}
-	ts.AddTestcase(setupReport)
 
 	for _, testStep := range t.Steps {
-		tc := report.NewCase("step " + testStep.String())
+		stepReport := rep.Step("step " + testStep.String())
 		testStep.Client = t.Client
 		if testStep.Kubeconfig != "" {
 			testStep.Client = newClient(testStep.Kubeconfig, testStep.Context)
@@ -371,8 +367,8 @@ func (t *Case) Run(test *testing.T, ts *report.Testsuite) {
 			testStep.DiscoveryClient = newDiscoveryClient(testStep.Kubeconfig, testStep.Context)
 		}
 		testStep.Logger = t.Logger.WithPrefix(testStep.String())
-		tc.Assertions += len(testStep.Asserts)
-		tc.Assertions += len(testStep.Errors)
+		stepReport.AddAssertions(len(testStep.Asserts))
+		stepReport.AddAssertions(len(testStep.Errors))
 
 		errs := []error{}
 
@@ -395,15 +391,12 @@ func (t *Case) Run(test *testing.T, ts *report.Testsuite) {
 
 		if len(errs) > 0 {
 			caseErr := fmt.Errorf("failed in step %s", testStep.String())
-			tc.Failure = report.NewFailure(caseErr.Error(), errs)
+			stepReport.Failure(caseErr.Error(), errs...)
 
 			test.Error(caseErr)
 			for _, err := range errs {
 				test.Error(err)
 			}
-		}
-		ts.AddTestcase(tc)
-		if len(errs) > 0 {
 			break
 		}
 	}

pkg/test/case_integration_test.go

@@ -111,5 +111,14 @@ func TestMultiClusterCase(t *testing.T) {
 		},
 	}
 
-	c.Run(t, &report.Testsuite{})
+	c.Run(t, &noOpReporter{})
 }
+
+type noOpReporter struct{}
+
+func (r *noOpReporter) Done() {}
+func (r *noOpReporter) Step(string) report.StepReporter {
+	return r
+}
+func (r *noOpReporter) AddAssertions(int)        {}
+func (r *noOpReporter) Failure(string, ...error) {}

pkg/test/harness.go

@@ -397,8 +397,8 @@ func (h *Harness) RunTests() {
 						t.Fatal(err)
 					}
 
-					testReport := suiteReport.NewSubSuite(test.Name)
-					test.Run(t, testReport)
+					testReporter := suiteReport.NewTest(test.Name)
+					test.Run(t, testReporter)
 				})
 			}
 		}

test/junit/.gitignore

@@ -0,0 +1,6 @@
+/kuttl-ouput-step-json.txt
+/kuttl-ouput-step-xml.txt
+/kuttl-report-step.json
+/kuttl-report-step.json.normalized
+/kuttl-report-step.xml
+/kuttl-report-step.xml.normalized

test/junit/Makefile

@@ -0,0 +1,24 @@
+.PHONY: test
+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
+	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
+	if [ ! -e kuttl-report-step.json ]; then cat kuttl-output-step-json.txt; exit 1; fi
+	$(MAKE) kuttl-report-step.xml.normalized kuttl-report-step.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
+
+.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
+
+# The following targets replace all timestamps and durations with dummy values to make comparisons easy.
+
+%.xml.normalized: %.xml
+	sed -E -e 's/time="[^"]+"/time="1.0"/g; s/[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}[.][0-9]{6,10}(Z|[-+][0-9]{2}:[0-9]{2})/2000-01-01T00:00:00.00000000+00:00/g' < $< > $@
+
+%.json.normalized: %.json
+	sed -E -e 's/"time": *"[^"]+"/"time": "1.0"/g; s/[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}[.][0-9]{6,10}(Z|[-+][0-9]{2}:[0-9]{2})/2000-01-01T00:00:00.00000000+00:00/g' < $< > $@