From 2ad51e0c36c35c2e919fc0bf4aaab9af1887012a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Thu, 12 Dec 2024 16:36:50 +0100 Subject: [PATCH] refactor: move reports management into caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- pkg/commands/test/command.go | 20 +++++++++++++------- pkg/runner/internal/test_deps_test.go | 5 +++++ pkg/runner/runner.go | 19 ++++++------------- pkg/runner/runner_test.go | 17 +++++++---------- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/pkg/commands/test/command.go b/pkg/commands/test/command.go index 07c451389..b6e1ef52f 100644 --- a/pkg/commands/test/command.go +++ b/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,21 @@ 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 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 { diff --git a/pkg/runner/internal/test_deps_test.go b/pkg/runner/internal/test_deps_test.go index 15237e404..d39889826 100644 --- a/pkg/runner/internal/test_deps_test.go +++ b/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)) diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index a1deaf107..8d220585e 100644 --- a/pkg/runner/runner.go +++ b/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() { diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 8be80845b..420c25acc 100644 --- a/pkg/runner/runner_test.go +++ b/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 { @@ -325,7 +322,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 {