refactor: move reports management into caller (#2230)

* refactor: move reports management into caller

Signed-off-by: Charles-Edouard Brétéché <[email protected]>

* fix tests

Signed-off-by: Charles-Edouard Brétéché <[email protected]>

---------

Signed-off-by: Charles-Edouard Brétéché <[email protected]>

pkg/commands/test/command.go

@@ -12,6 +12,7 @@ import (
 	"github.com/kyverno/chainsaw/pkg/discovery"
 	"github.com/kyverno/chainsaw/pkg/loaders/config"
 	"github.com/kyverno/chainsaw/pkg/loaders/values"
+	"github.com/kyverno/chainsaw/pkg/report"
 	"github.com/kyverno/chainsaw/pkg/runner"
 	runnerflags "github.com/kyverno/chainsaw/pkg/runner/flags"
 	flagutils "github.com/kyverno/chainsaw/pkg/utils/flag"
@@ -351,16 +352,23 @@ func Command() *cobra.Command {
 			if err := runnerflags.SetupFlags(configuration.Spec); err != nil {
 				return err
 			}
-			summary, err := runner.Run(ctx, configuration.Spec, tc, testToRun...)
-			if summary != nil {
-				fmt.Fprintln(stdOut, "Tests Summary...")
-				fmt.Fprintln(stdOut, "- Passed  tests", summary.Passed())
-				fmt.Fprintln(stdOut, "- Failed  tests", summary.Failed())
-				fmt.Fprintln(stdOut, "- Skipped tests", summary.Skipped())
+			err = runner.Run(ctx, configuration.Spec, tc, testToRun...)
+			fmt.Fprintln(stdOut, "Tests Summary...")
+			fmt.Fprintln(stdOut, "- Passed  tests", tc.Passed())
+			fmt.Fprintln(stdOut, "- Failed  tests", tc.Failed())
+			fmt.Fprintln(stdOut, "- Skipped tests", tc.Skipped())
+			// process report
+			if err == nil {
+				if configuration.Spec.Report != nil && configuration.Spec.Report.Format != "" {
+					fmt.Fprintln(stdOut, "Saving report...")
+					if err := report.Save(tc.Report, configuration.Spec.Report.Format, configuration.Spec.Report.Path, configuration.Spec.Report.Name); err != nil {
+						return err
+					}
+				}
 			}
 			if err != nil {
 				fmt.Fprintln(stdOut, "Done with error.")
-			} else if summary != nil && summary.Failed() > 0 {
+			} else if tc.Failed() > 0 {
 				fmt.Fprintln(stdOut, "Done with failures.")
 				err = errors.New("some tests failed")
 			} else {

pkg/commands/test/command_test.go

@@ -11,6 +11,7 @@ import (
 )
 
 func TestChainsawCommand(t *testing.T) {
+	path := "../../../.temp"
 	basePath := "../../../testdata/commands/test"
 	tests := []struct {
 		name    string
@@ -116,6 +117,8 @@ func TestChainsawCommand(t *testing.T) {
 		args: []string{
 			"--config",
 			filepath.Join(basePath, "config/config_all_fields.yaml"),
+			"--report-path",
+			path,
 		},
 		wantErr: false,
 		out:     filepath.Join(basePath, "config_all_fields.txt"),
@@ -135,6 +138,8 @@ func TestChainsawCommand(t *testing.T) {
 			"--parallel=24",
 			"--repeat-count=12",
 			"--report-format=XML",
+			"--report-path",
+			path,
 			"--report-name=foo",
 			"--namespace=bar",
 			"--full-name=true",

pkg/runner/internal/test_deps_test.go

@@ -80,6 +80,11 @@ func TestTestDeps_MatchString(t *testing.T) {
 	}
 }
 
+func TestTestDeps_SetPanicOnExit0(t *testing.T) {
+	d := &TestDeps{}
+	d.SetPanicOnExit0(true)
+}
+
 func TestTestDeps_StartCPUProfile(t *testing.T) {
 	d := &TestDeps{}
 	assert.NoError(t, d.StartCPUProfile(nil))

pkg/runner/runner.go

@@ -9,7 +9,6 @@ import (
 	"github.com/kyverno/chainsaw/pkg/discovery"
 	"github.com/kyverno/chainsaw/pkg/engine/clusters"
 	"github.com/kyverno/chainsaw/pkg/model"
-	"github.com/kyverno/chainsaw/pkg/report"
 	enginecontext "github.com/kyverno/chainsaw/pkg/runner/context"
 	"github.com/kyverno/chainsaw/pkg/runner/internal"
 	"github.com/kyverno/chainsaw/pkg/testing"
@@ -18,7 +17,7 @@ import (
 )
 
 type Runner interface {
-	Run(context.Context, model.Configuration, enginecontext.TestContext, ...discovery.Test) (model.SummaryResult, error)
+	Run(context.Context, model.Configuration, enginecontext.TestContext, ...discovery.Test) error
 }
 
 func New(clock clock.PassiveClock, onFailure func()) Runner {
@@ -34,14 +33,14 @@ type runner struct {
 	deps      *internal.TestDeps
 }
 
-func (r *runner) Run(ctx context.Context, config model.Configuration, tc enginecontext.TestContext, tests ...discovery.Test) (model.SummaryResult, error) {
+func (r *runner) Run(ctx context.Context, config model.Configuration, tc enginecontext.TestContext, tests ...discovery.Test) error {
 	return r.run(ctx, nil, config, tc, tests...)
 }
 
-func (r *runner) run(ctx context.Context, m mainstart, config model.Configuration, tc enginecontext.TestContext, tests ...discovery.Test) (model.SummaryResult, error) {
+func (r *runner) run(ctx context.Context, m mainstart, config model.Configuration, tc enginecontext.TestContext, tests ...discovery.Test) error {
 	// sanity check
 	if len(tests) == 0 {
-		return nil, nil
+		return nil
 	}
 	internalTests := []testing.InternalTest{{
 		Name: "chainsaw",
@@ -66,16 +65,10 @@ func (r *runner) run(ctx context.Context, m mainstart, config model.Configuratio
 	// In our case, we consider an error only when running the tests was not possible.
 	// For now, the case where some of the tests failed will be covered by the summary.
 	if code := m.Run(); code > 1 {
-		return nil, fmt.Errorf("testing framework exited with non zero code %d", code)
+		return fmt.Errorf("testing framework exited with non zero code %d", code)
 	}
 	tc.Report.EndTime = time.Now()
-	// TODO: move to the caller
-	if config.Report != nil && config.Report.Format != "" {
-		if err := report.Save(tc.Report, config.Report.Format, config.Report.Path, config.Report.Name); err != nil {
-			return tc, err
-		}
-	}
-	return tc, nil
+	return nil
 }
 
 func (r *runner) onFail() {

pkg/runner/runner_test.go

@@ -94,19 +94,16 @@ func Test_runner_Run(t *testing.T) {
 			// to allow unit tests to work
 			assert.NoError(t, flags.SetupFlags(tt.config))
 			assert.NoError(t, flag.Set("test.testlogfile", ""))
-			got, err := r.Run(context.TODO(), tt.config, tt.tc, tt.tests...)
+			err := r.Run(context.TODO(), tt.config, tt.tc, tt.tests...)
 			if tt.wantErr {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
 			}
-			if tt.want == nil {
-				assert.Nil(t, got)
-			} else {
-				assert.NotNil(t, got)
-				assert.Equal(t, tt.want.failed, got.Failed())
-				assert.Equal(t, tt.want.passed, got.Passed())
-				assert.Equal(t, tt.want.skipped, got.Skipped())
+			if tt.want != nil {
+				assert.Equal(t, tt.want.failed, tc.Failed())
+				assert.Equal(t, tt.want.passed, tc.Passed())
+				assert.Equal(t, tt.want.skipped, tc.Skipped())
 			}
 		})
 	}
@@ -158,7 +155,7 @@ func Test_runner_run(t *testing.T) {
 			r := &runner{
 				clock: clock.RealClock{},
 			}
-			_, err := r.run(context.TODO(), tt.m, model.Configuration{}, enginecontext.EmptyContext(), tt.tests...)
+			err := r.run(context.TODO(), tt.m, model.Configuration{}, enginecontext.EmptyContext(), tt.tests...)
 			if tt.wantErr {
 				assert.Error(t, err)
 			} else {
@@ -229,17 +226,6 @@ func TestRun(t *testing.T) {
 		},
 		restConfig: nil,
 		wantErr:    false,
-	}, {
-		name:  "Zero Tests with JSON Report",
-		tests: []discovery.Test{},
-		config: model.Configuration{
-			Timeouts: config.Spec.Timeouts,
-			Report: &v1alpha2.ReportOptions{
-				Format: v1alpha2.JSONFormat,
-			},
-		},
-		restConfig: &rest.Config{},
-		wantErr:    false,
 	}, {
 		name: "Success Case with 1 Test",
 		tests: []discovery.Test{
@@ -270,49 +256,6 @@ func TestRun(t *testing.T) {
 		restConfig: &rest.Config{},
 		mockReturn: 2,
 		wantErr:    true,
-	}, {
-		name: "Success Case with 1 Test with XML Report",
-		tests: []discovery.Test{
-			{
-				Err: nil,
-				Test: &model.Test{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: "test1",
-					},
-				},
-			},
-		},
-		config: model.Configuration{
-			Timeouts: config.Spec.Timeouts,
-			Report: &v1alpha2.ReportOptions{
-				Format: v1alpha2.XMLFormat,
-				Name:   "chainsaw",
-			},
-		},
-		restConfig: &rest.Config{},
-		mockReturn: 0,
-		wantErr:    false,
-	}, {
-		name: "Error in saving Report",
-		tests: []discovery.Test{
-			{
-				Err: nil,
-				Test: &model.Test{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: "test1",
-					},
-				},
-			},
-		},
-		config: model.Configuration{
-			Timeouts: config.Spec.Timeouts,
-			Report: &v1alpha2.ReportOptions{
-				Format: "abc",
-			},
-		},
-		restConfig: &rest.Config{},
-		mockReturn: 0,
-		wantErr:    true,
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -325,7 +268,7 @@ func TestRun(t *testing.T) {
 			ctx := context.TODO()
 			tc, err := InitContext(tt.config, tt.restConfig, nil)
 			assert.NoError(t, err)
-			_, err = runner.run(ctx, mockMainStart, tt.config, tc, tt.tests...)
+			err = runner.run(ctx, mockMainStart, tt.config, tc, tt.tests...)
 			if tt.wantErr {
 				assert.Error(t, err)
 			} else {

testdata/commands/test/all_flags.txt

@@ -6,6 +6,7 @@ Loading default configuration...
 - FailFast false
 - ReportFormat 'XML'
 - ReportName 'foo'
+- ReportPath '../../../.temp'
 - Namespace 'bar'
 - FullName true
 - IncludeTestRegex '^.*$'
@@ -26,4 +27,9 @@ Loading default configuration...
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
+Saving report...
 Done.

testdata/commands/test/config_all_fields.txt

@@ -6,6 +6,7 @@ Loading config (../../../testdata/commands/test/config/config_all_fields.yaml)..
 - FailFast true
 - ReportFormat 'JSON'
 - ReportName 'custom-chainsaw-report'
+- ReportPath '../../../.temp'
 - Namespace 'test-namespace'
 - FullName true
 - IncludeTestRegex '^include-.*'
@@ -24,4 +25,9 @@ Loading config (../../../testdata/commands/test/config/config_all_fields.yaml)..
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
+Saving report...
 Done.

testdata/commands/test/default.txt

@@ -21,4 +21,8 @@ Loading default configuration...
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
 Done.

testdata/commands/test/with_regex.txt

@@ -21,4 +21,8 @@ Loading default configuration...
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
 Done.

testdata/commands/test/with_repeat_count.txt

@@ -22,4 +22,8 @@ Loading default configuration...
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
 Done.

testdata/commands/test/with_suppress.txt

@@ -24,4 +24,8 @@ Loading default configuration...
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
 Done.

testdata/commands/test/with_test_dirs.txt

@@ -21,4 +21,8 @@ Loading default configuration...
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
 Done.

testdata/commands/test/with_timeout.txt

@@ -21,4 +21,8 @@ Loading default configuration...
 Loading tests...
 Loading values...
 Running tests...
+Tests Summary...
+- Passed  tests 0
+- Failed  tests 0
+- Skipped tests 0
 Done.