From 22e2920b17f93907e5d5a33dc11a2788731ae3d3 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 19 Jun 2022 11:28:45 +0100 Subject: [PATCH 1/7] TestStep variables proof of concept Signed-off-by: Ilya --- pkg/jobmanager/bundles.go | 1 - pkg/pluginregistry/bundles.go | 70 ++++++- pkg/pluginregistry/errors.go | 14 ++ pkg/pluginregistry/pluginregistry_test.go | 21 +- pkg/runner/base_test_suite_test.go | 20 +- pkg/runner/job_runner_test.go | 35 ++-- pkg/runner/step_runner.go | 8 +- pkg/runner/step_runner_test.go | 66 +++++-- pkg/runner/test_runner.go | 181 +++++++++++++++++- pkg/runner/test_runner_test.go | 132 ++++++++++++- pkg/storage/limits/limits.go | 20 +- pkg/storage/limits/limits_test.go | 8 +- pkg/test/step.go | 37 +++- plugins/teststeps/cmd/cmd.go | 9 +- plugins/teststeps/cpucmd/cpucmd.go | 9 +- plugins/teststeps/echo/echo.go | 9 +- plugins/teststeps/example/example.go | 9 +- plugins/teststeps/exec/exec.go | 9 +- plugins/teststeps/randecho/randecho.go | 9 +- .../teststeps/s3fileupload/s3fileupload.go | 10 +- plugins/teststeps/sleep/sleep.go | 9 +- plugins/teststeps/sshcmd/sshcmd.go | 9 +- .../terminalexpect/terminalexpect.go | 11 +- plugins/teststeps/waitport/waitport.go | 9 +- plugins/teststeps/waitport/waitport_test.go | 2 +- tests/e2e/e2e_test.go | 52 ++--- tests/e2e/test-resume.yaml | 14 +- tests/e2e/test-retry.yaml | 6 +- tests/e2e/test-simple.yaml | 8 +- .../teststeps/badtargets/badtargets.go | 9 +- tests/plugins/teststeps/channels/channels.go | 9 +- tests/plugins/teststeps/crash/crash.go | 9 +- tests/plugins/teststeps/fail/fail.go | 9 +- tests/plugins/teststeps/hanging/hanging.go | 9 +- tests/plugins/teststeps/noop/noop.go | 9 +- tests/plugins/teststeps/noreturn/noreturn.go | 9 +- .../plugins/teststeps/panicstep/panicstep.go | 9 +- tests/plugins/teststeps/readmeta/readmeta.go | 9 +- tests/plugins/teststeps/slowecho/slowecho.go | 9 +- tests/plugins/teststeps/teststep/teststep.go | 9 +- 40 files changed, 747 insertions(+), 149 deletions(-) diff --git a/pkg/jobmanager/bundles.go b/pkg/jobmanager/bundles.go index 991d8909..73134380 100644 --- a/pkg/jobmanager/bundles.go +++ b/pkg/jobmanager/bundles.go @@ -60,7 +60,6 @@ func newBundlesFromSteps(ctx xcontext.Context, descriptors []*test.TestStepDescr // look up test step plugins in the plugin registry var stepBundles []test.TestStepBundle - for idx, descriptor := range descriptors { if descriptor == nil { return nil, fmt.Errorf("test step description is null") diff --git a/pkg/pluginregistry/bundles.go b/pkg/pluginregistry/bundles.go index da8d9e38..05fd8447 100644 --- a/pkg/pluginregistry/bundles.go +++ b/pkg/pluginregistry/bundles.go @@ -7,6 +7,7 @@ package pluginregistry import ( "fmt" + "strings" "github.com/linuxboot/contest/pkg/job" "github.com/linuxboot/contest/pkg/target" @@ -28,14 +29,45 @@ func (r *PluginRegistry) NewTestStepBundle(ctx xcontext.Context, testStepDescrip return nil, err } label := testStepDescriptor.Label - if label == "" { + if len(label) == 0 { return nil, ErrStepLabelIsMandatory{TestStepDescriptor: testStepDescriptor} } + + var variablesMapping test.StepVariablesMapping + if testStepDescriptor.VariablesMapping != nil { + variablesMapping = make(test.StepVariablesMapping) + for internalName, mappedName := range testStepDescriptor.VariablesMapping { + if err := checkVariableName(internalName); err != nil { + return nil, InvalidVariableFormat{InvalidName: internalName, Err: err} + } + if _, found := variablesMapping[internalName]; found { + return nil, fmt.Errorf("duplication of '%s' variable", internalName) + } + // NewStepVariable creates a StepVariable object from mapping: "stepName.VariableName" + parts := strings.Split(mappedName, ".") + if len(parts) != 2 { + return nil, fmt.Errorf("variable mapping '%s' should contain a single '.' separator", mappedName) + } + if err := checkVariableName(parts[0]); err != nil { + return nil, InvalidVariableFormat{InvalidName: parts[0], Err: err} + } + if err := checkVariableName(parts[1]); err != nil { + return nil, InvalidVariableFormat{InvalidName: parts[1], Err: err} + } + + variablesMapping[internalName] = test.StepVariable{ + StepName: parts[0], + VariableName: parts[1], + } + } + } + // TODO: check that all testStep labels from variable mappings exist testStepBundle := test.TestStepBundle{ - TestStep: testStep, - TestStepLabel: label, - Parameters: testStepDescriptor.Parameters, - AllowedEvents: allowedEvents, + TestStep: testStep, + TestStepLabel: label, + Parameters: testStepDescriptor.Parameters, + AllowedEvents: allowedEvents, + VariablesMapping: variablesMapping, } return &testStepBundle, nil } @@ -129,3 +161,31 @@ func (r *PluginRegistry) NewFinalReporterBundle(reporterName string, reporterPar } return &reporterBundle, nil } + +func isAlpha(ch int32) bool { + return (ch >= 'a' && ch <= 'z') || (ch >= 'A' || ch <= 'Z') +} + +func checkVariableName(s string) error { + if len(s) == 0 { + return fmt.Errorf("empty variable name") + } + for idx, ch := range s { + if idx == 0 { + if !isAlpha(ch) { + return fmt.Errorf("first character should be alpha, got %c", ch) + } + } + if isAlpha(ch) { + continue + } + if ch >= '0' || ch <= '9' { + continue + } + if ch == '_' { + continue + } + return fmt.Errorf("got unxpected character: %c", ch) + } + return nil +} diff --git a/pkg/pluginregistry/errors.go b/pkg/pluginregistry/errors.go index 6f84de53..bc71b181 100644 --- a/pkg/pluginregistry/errors.go +++ b/pkg/pluginregistry/errors.go @@ -18,3 +18,17 @@ type ErrStepLabelIsMandatory struct { func (err ErrStepLabelIsMandatory) Error() string { return fmt.Sprintf("step has no label, but it is mandatory (step: %+v)", err.TestStepDescriptor) } + +// InvalidVariableFormat tells that a variable name doesn't fit the variable name format (alphanum + '_') +type InvalidVariableFormat struct { + InvalidName string + Err error +} + +func (err InvalidVariableFormat) Error() string { + return fmt.Sprintf("'%s' doesn't match variable name format: %v", err.InvalidName, err.Err) +} + +func (err InvalidVariableFormat) Unwrap() error { + return err.Err +} diff --git a/pkg/pluginregistry/pluginregistry_test.go b/pkg/pluginregistry/pluginregistry_test.go index db008c67..109b39df 100644 --- a/pkg/pluginregistry/pluginregistry_test.go +++ b/pkg/pluginregistry/pluginregistry_test.go @@ -19,10 +19,6 @@ import ( "github.com/stretchr/testify/require" ) -var ( - ctx, _ = logrusctx.NewContext(logger.LevelDebug) -) - // Definition of two dummy TestSteps to be used to test the PluginRegistry // AStep implements a dummy TestStep @@ -44,18 +40,29 @@ func (e AStep) Name() string { } // Run executes the AStep -func (e AStep) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (e AStep) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { return nil, nil } func TestRegisterTestStep(t *testing.T) { + ctx, cancel := logrusctx.NewContext(logger.LevelDebug) + defer cancel() pr := NewPluginRegistry(ctx) - err := pr.RegisterTestStep("AStep", NewAStep, []event.Name{event.Name("AStepEventName")}) + err := pr.RegisterTestStep("AStep", NewAStep, []event.Name{"AStepEventName"}) require.NoError(t, err) } func TestRegisterTestStepDoesNotValidate(t *testing.T) { + ctx, cancel := logrusctx.NewContext(logger.LevelDebug) + defer cancel() pr := NewPluginRegistry(ctx) - err := pr.RegisterTestStep("AStep", NewAStep, []event.Name{event.Name("Event which does not validate")}) + err := pr.RegisterTestStep("AStep", NewAStep, []event.Name{"Event which does not validate"}) require.Error(t, err) } diff --git a/pkg/runner/base_test_suite_test.go b/pkg/runner/base_test_suite_test.go index ed6f52a4..34c21fef 100644 --- a/pkg/runner/base_test_suite_test.go +++ b/pkg/runner/base_test_suite_test.go @@ -73,9 +73,9 @@ func (s *BaseTestSuite) TearDownTest() { } func (s *BaseTestSuite) RegisterStateFullStep( - runFunction func( - ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, - ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error), + runFunction func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, + resumeState json.RawMessage) (json.RawMessage, error), validateFunction func(ctx xcontext.Context, params test.TestStepParameters) error) error { return s.PluginRegistry.RegisterTestStep(stateFullStepName, func() test.TestStep { @@ -86,11 +86,17 @@ func (s *BaseTestSuite) RegisterStateFullStep( }, nil) } -func (s *BaseTestSuite) NewStep(ctx xcontext.Context, label, name string, params test.TestStepParameters) test.TestStepBundle { +func (s *BaseTestSuite) NewStep( + ctx xcontext.Context, + label, name string, + params test.TestStepParameters, + variablesMapping map[string]string, +) test.TestStepBundle { td := test.TestStepDescriptor{ - Name: name, - Label: label, - Parameters: params, + Name: name, + Label: label, + Parameters: params, + VariablesMapping: variablesMapping, } sb, err := s.PluginRegistry.NewTestStepBundle(ctx, td) require.NoError(s.T(), err) diff --git a/pkg/runner/job_runner_test.go b/pkg/runner/job_runner_test.go index 20a77a92..b0db15ac 100644 --- a/pkg/runner/job_runner_test.go +++ b/pkg/runner/job_runner_test.go @@ -58,7 +58,8 @@ func (s *JobRunnerSuite) TestSimpleJobStartFinish() { var resultTargets []*target.Target require.NoError(s.T(), s.RegisterStateFullStep( - func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { + func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) { return teststeps.ForEachTarget(stateFullStepName, ctx, ch, func(ctx xcontext.Context, target *target.Target) error { assert.NotNil(s.T(), target) mu.Lock() @@ -91,7 +92,7 @@ func (s *JobRunnerSuite) TestSimpleJobStartFinish() { TargetManager: targetlist.New(), }, TestStepsBundles: []test.TestStepBundle{ - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), }, }, }, @@ -124,7 +125,8 @@ func (s *JobRunnerSuite) TestJobWithTestRetry() { var callsCount int require.NoError(s.T(), s.RegisterStateFullStep( - func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { + func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) { return teststeps.ForEachTarget(stateFullStepName, ctx, ch, func(ctx xcontext.Context, target *target.Target) error { assert.NotNil(s.T(), target) mu.Lock() @@ -175,11 +177,11 @@ func (s *JobRunnerSuite) TestJobWithTestRetry() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }), - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + }, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), s.NewStep(ctx, "echo2_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("world")}, - }), + }, nil), }, }, }, @@ -280,7 +282,7 @@ func (s *JobRunnerSuite) TestJobRetryOnFailedAcquire() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }), + }, nil), }, }, }, @@ -359,7 +361,7 @@ func (s *JobRunnerSuite) TestAcquireFailed() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }), + }, nil), }, }, }, @@ -429,7 +431,7 @@ func (s *JobRunnerSuite) TestResumeStateBadJobId() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }), + }, nil), }, }, }, @@ -454,8 +456,8 @@ func (s *JobRunnerSuite) TestResumeStateBadJobId() { const stateFullStepName = "statefull" type stateFullStep struct { - runFunction func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, - ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) + runFunction func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) validateFunction func(ctx xcontext.Context, params test.TestStepParameters) error } @@ -463,13 +465,18 @@ func (sfs *stateFullStep) Name() string { return stateFullStepName } -func (sfs *stateFullStep) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, - ev testevent.Emitter, resumeState json.RawMessage, +func (sfs *stateFullStep) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, ) (json.RawMessage, error) { if sfs.runFunction == nil { return nil, fmt.Errorf("stateFullStep run is not initialised") } - return sfs.runFunction(ctx, ch, params, ev, resumeState) + return sfs.runFunction(ctx, ch, ev, stepsVars, params, resumeState) } func (sfs *stateFullStep) ValidateParameters(ctx xcontext.Context, params test.TestStepParameters) error { diff --git a/pkg/runner/step_runner.go b/pkg/runner/step_runner.go index 5375b020..65a5b0c7 100644 --- a/pkg/runner/step_runner.go +++ b/pkg/runner/step_runner.go @@ -90,6 +90,7 @@ func NewStepRunner() *StepRunner { func (sr *StepRunner) Run( ctx xcontext.Context, bundle test.TestStepBundle, + stepsVariables test.StepsVariables, ev testevent.Emitter, resumeState json.RawMessage, resumeStateTargets []target.Target, @@ -132,7 +133,7 @@ func (sr *StepRunner) Run( stepOut := make(chan test.TestStepResult) go func() { defer finish() - sr.runningLoop(ctx, sr.input, stepOut, bundle, ev, resumeState) + sr.runningLoop(ctx, sr.input, stepOut, bundle, stepsVariables, ev, resumeState) ctx.Debugf("Running loop finished") }() @@ -381,6 +382,7 @@ func (sr *StepRunner) runningLoop( stepIn <-chan *target.Target, stepOut chan test.TestStepResult, bundle test.TestStepBundle, + stepsVariables test.StepsVariables, ev testevent.Emitter, resumeState json.RawMessage, ) { @@ -410,9 +412,9 @@ func (sr *StepRunner) runningLoop( }() inChannels := test.TestStepChannels{In: stepIn, Out: stepOut} - return bundle.TestStep.Run(ctx, inChannels, bundle.Parameters, ev, resumeState) + return bundle.TestStep.Run(ctx, inChannels, ev, stepsVariables, bundle.Parameters, resumeState) }() - ctx.Debugf("TestStep finished '%v', rs %s", err, string(resultResumeState)) + ctx.Debugf("TestStep finished '%v', rs: '%s'", err, string(resultResumeState)) sr.mu.Lock() sr.setErrLocked(ctx, err) diff --git a/pkg/runner/step_runner_test.go b/pkg/runner/step_runner_test.go index 29c5b7c2..28dfce4f 100644 --- a/pkg/runner/step_runner_test.go +++ b/pkg/runner/step_runner_test.go @@ -57,7 +57,8 @@ func (s *StepRunnerSuite) TestRunningStep() { var obtainedResumeState json.RawMessage err := s.RegisterStateFullStep( - func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { + func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) { obtainedResumeState = resumeState _, err := teststeps.ForEachTarget(stateFullStepName, ctx, ch, func(ctx xcontext.Context, target *target.Target) error { require.NotNil(s.T(), target) @@ -84,7 +85,8 @@ func (s *StepRunnerSuite) TestRunningStep() { inputResumeState := json.RawMessage("{\"some_input\": 42}") addTarget, resumedTargetsResults, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, inputResumeState, nil, @@ -127,7 +129,8 @@ func (s *StepRunnerSuite) TestAddSameTargetSequentiallyTimes() { const inputTargetID = "input_target_id" err := s.RegisterStateFullStep( - func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { + func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) { _, err := teststeps.ForEachTarget(stateFullStepName, ctx, ch, func(ctx xcontext.Context, target *target.Target) error { require.NotNil(s.T(), target) require.Equal(s.T(), inputTargetID, target.ID) @@ -148,7 +151,8 @@ func (s *StepRunnerSuite) TestAddSameTargetSequentiallyTimes() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, nil, nil, @@ -180,7 +184,8 @@ func (s *StepRunnerSuite) TestAddTargetReturnsErrorIfFailsToInput() { } }() err := s.RegisterStateFullStep( - func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { + func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) { <-hangCh for range ch.In { require.Fail(s.T(), "unexpected input") @@ -199,7 +204,8 @@ func (s *StepRunnerSuite) TestAddTargetReturnsErrorIfFailsToInput() { defer stepRunner.Stop() addTarget, resumedTargetsResults, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, nil, nil, @@ -238,7 +244,8 @@ func (s *StepRunnerSuite) TestStepPanics() { defer cancel() err := s.RegisterStateFullStep( - func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { + func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) { panic("panic") }, nil, @@ -250,7 +257,8 @@ func (s *StepRunnerSuite) TestStepPanics() { defer stepRunner.Stop() addTarget, resumedTargetsResults, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), NewTestStepEventsEmitterFactory( s.MemoryStorage.StorageEngineVault, 1, @@ -288,7 +296,8 @@ func (s *StepRunnerSuite) TestCornerCases() { defer cancel() err := s.RegisterStateFullStep( - func(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { + func(ctx xcontext.Context, ch test.TestStepChannels, ev testevent.Emitter, + stepsVars test.StepsVariables, params test.TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) { _, err := teststeps.ForEachTarget(stateFullStepName, ctx, ch, func(ctx xcontext.Context, target *target.Target) error { return fmt.Errorf("should not be called") }) @@ -307,7 +316,8 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, nil, nil, @@ -330,7 +340,8 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, nil, nil, @@ -340,7 +351,8 @@ func (s *StepRunnerSuite) TestCornerCases() { require.NotNil(s.T(), runResult) addTarget2, _, runResult2, err2 := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, nil, nil, @@ -356,7 +368,8 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, nil, nil, @@ -376,8 +389,10 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() stepRunner.Stop() + addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + newStepsVariablesMock(nil, nil), emitter, nil, nil, @@ -388,3 +403,26 @@ func (s *StepRunnerSuite) TestCornerCases() { checkSuccessfulResult(s.T(), runResult) }) } + +type stepsVariablesMock struct { + add func(tgtID string, name string, value interface{}) error + get func(tgtID string, name string, value interface{}) error +} + +func (sm *stepsVariablesMock) Add(tgtID string, name string, value interface{}) error { + return sm.add(tgtID, name, value) +} + +func (sm *stepsVariablesMock) Get(tgtID string, name string, value interface{}) error { + return sm.get(tgtID, name, value) +} + +func newStepsVariablesMock( + add func(tgtID string, name string, value interface{}) error, + get func(tgtID string, name string, value interface{}) error, +) *stepsVariablesMock { + return &stepsVariablesMock{ + add: add, + get: get, + } +} diff --git a/pkg/runner/test_runner.go b/pkg/runner/test_runner.go index cc7ad47b..faf9d61b 100644 --- a/pkg/runner/test_runner.go +++ b/pkg/runner/test_runner.go @@ -43,6 +43,8 @@ type TestRunner struct { steps []*stepState // The pipeline, in order of execution targets map[string]*targetState // Target state lookup map + stepsVariables *testStepsVariables + // One mutex to rule them all, used to serialize access to all the state above. // Could probably be split into several if necessary. mu sync.Mutex @@ -61,14 +63,18 @@ const ( targetStepPhaseEnd // (5) Finished running a step. ) +// stepVariables represents the emitted variables of the steps +type stepVariables map[string]json.RawMessage + // targetState contains state associated with one target progressing through the pipeline. type targetState struct { tgt *target.Target // This part of state gets serialized into JSON for resumption. - CurStep int `json:"S,omitempty"` // Current step number. - CurPhase targetStepPhase `json:"P,omitempty"` // Current phase of step execution. - Res *xjson.Error `json:"R,omitempty"` // Final result, if reached the end state. + CurStep int `json:"S,omitempty"` // Current step number. + CurPhase targetStepPhase `json:"P,omitempty"` // Current phase of step execution. + Res *xjson.Error `json:"R,omitempty"` // Final result, if reached the end state. + StepsVariables map[string]stepVariables `json:"V,omitempty"` // maps steps onto emitted variables of each handlerRunning bool } @@ -138,6 +144,20 @@ func (tr *TestRunner) Run( } } + var err error + tr.stepsVariables, err = newTestStepsVariables(t.TestStepsBundles) + if err != nil { + ctx.Errorf("Failed to initialise test steps variables: %v", err) + return nil, nil, err + } + + for targetID, targetState := range tr.targets { + if err := tr.stepsVariables.initTargetStepsVariables(targetID, targetState.StepsVariables); err != nil { + ctx.Errorf("Failed to initialise test steps variables for target: %s: %v", targetID, err) + return nil, nil, err + } + } + // Set up the pipeline for i, sb := range t.TestStepsBundles { var srs json.RawMessage @@ -201,7 +221,12 @@ func (tr *TestRunner) Run( numInFlightTargets := 0 for i, tgt := range targets { tgs := tr.targets[tgt.ID] - stepErr := tr.steps[tgs.CurStep].GetError() + tgs.StepsVariables, err = tr.stepsVariables.getTargetStepsVariables(tgt.ID) + if err != nil { + ctx.Errorf("Failed to get steps variables: %v", err) + return nil, nil, err + } + stepErr := tr.steps[tgs.CurStep].runErr if tgs.CurPhase == targetStepPhaseRun { numInFlightTargets++ if stepErr != xcontext.ErrPaused { @@ -549,3 +574,151 @@ func (tgs *targetState) String() string { return fmt.Sprintf("[%s %d %s %t %s]", tgs.tgt, tgs.CurStep, tgs.CurPhase, finished, resText) } + +type testStepsVariables struct { + existingSteps map[string]struct{} + + lock sync.Mutex + stepsVariablesByTarget map[string]map[string]stepVariables +} + +func newTestStepsVariables(bundles []test.TestStepBundle) (*testStepsVariables, error) { + result := &testStepsVariables{ + existingSteps: make(map[string]struct{}), + stepsVariablesByTarget: make(map[string]map[string]stepVariables), + } + for _, bs := range bundles { + if len(bs.TestStepLabel) == 0 { + continue + } + if _, found := result.existingSteps[bs.TestStepLabel]; found { + return nil, fmt.Errorf("duplication of test step label: '%s'", bs.TestStepLabel) + } + result.existingSteps[bs.TestStepLabel] = struct{}{} + } + return result, nil +} + +func (tsv *testStepsVariables) initTargetStepsVariables(tgtID string, stepsVars map[string]stepVariables) error { + if _, found := tsv.stepsVariablesByTarget[tgtID]; found { + return fmt.Errorf("duplication of target ID: '%s'", tgtID) + } + if stepsVars == nil { + tsv.stepsVariablesByTarget[tgtID] = make(map[string]stepVariables) + } else { + tsv.stepsVariablesByTarget[tgtID] = stepsVars + } + return nil +} + +func (tsv *testStepsVariables) getTargetStepsVariables(tgtID string) (map[string]stepVariables, error) { + tsv.lock.Lock() + defer tsv.lock.Unlock() + + vars, found := tsv.stepsVariablesByTarget[tgtID] + if !found { + return nil, fmt.Errorf("target ID: '%s' is not found", tgtID) + } + + // make a copy in case of buggy steps that can still be using it + result := make(map[string]stepVariables) + for name, value := range vars { + result[name] = value + } + return result, nil +} + +func (tsv *testStepsVariables) Add(tgtID string, stepLabel string, name string, value json.RawMessage) error { + if err := tsv.checkInput(tgtID, stepLabel, name); err != nil { + return err + } + + // tsv.stepsVariablesByTarget is a readonly map, though its values may change + targetSteps := tsv.stepsVariablesByTarget[tgtID] + tsv.lock.Lock() + defer tsv.lock.Unlock() + + stepVars, found := targetSteps[stepLabel] + if !found { + stepVars = make(stepVariables) + targetSteps[stepLabel] = stepVars + } + stepVars[name] = value + return nil +} + +func (tsv *testStepsVariables) Get(tgtID string, stepLabel string, name string) (json.RawMessage, error) { + if err := tsv.checkInput(tgtID, stepLabel, name); err != nil { + return nil, err + } + + targetSteps := tsv.stepsVariablesByTarget[tgtID] + tsv.lock.Lock() + defer tsv.lock.Unlock() + + stepVars, found := targetSteps[stepLabel] + if !found { + return nil, fmt.Errorf("step '%s' didn't emit any variables yet", stepLabel) + } + value, found := stepVars[name] + if !found { + return nil, fmt.Errorf("step '%s' didn't emit variable '%s' yet", stepLabel, name) + } + return value, nil +} + +func (tsv *testStepsVariables) checkInput(tgtID string, stepLabel string, name string) error { + if len(stepLabel) == 0 { + return fmt.Errorf("empty step name") + } + if len(tgtID) == 0 { + return fmt.Errorf("empty target ID") + } + if len(name) == 0 { + return fmt.Errorf("empty variable name") + } + return nil +} + +type stepVariablesAccessor struct { + stepLabel string + varsMapping test.StepVariablesMapping + tsv *testStepsVariables +} + +func newStepVariablesAccessor(stepLabel string, varsMapping test.StepVariablesMapping, tsv *testStepsVariables) *stepVariablesAccessor { + result := &stepVariablesAccessor{ + stepLabel: stepLabel, + varsMapping: varsMapping, + tsv: tsv, + } + if result.varsMapping == nil { + result.varsMapping = make(test.StepVariablesMapping) + } + return result +} + +func (sva *stepVariablesAccessor) Add(tgtID string, name string, value interface{}) error { + if len(sva.stepLabel) == 0 { + return nil + } + b, err := json.Marshal(value) + if err != nil { + return fmt.Errorf("failed to serialize variable: %v", value) + } + return sva.tsv.Add(tgtID, sva.stepLabel, name, b) +} + +func (sva *stepVariablesAccessor) Get(tgtID string, name string, out interface{}) error { + stepVar, found := sva.varsMapping[name] + if !found { + return fmt.Errorf("no mapping for variable '%s'", name) + } + b, err := sva.tsv.Get(tgtID, stepVar.StepName, stepVar.VariableName) + if err != nil { + return err + } + return json.Unmarshal(b, out) +} + +var _ test.StepsVariables = (*stepVariablesAccessor)(nil) diff --git a/pkg/runner/test_runner_test.go b/pkg/runner/test_runner_test.go index aec35c58..2f7b2e42 100644 --- a/pkg/runner/test_runner_test.go +++ b/pkg/runner/test_runner_test.go @@ -6,9 +6,11 @@ package runner import ( + "encoding/json" "flag" "fmt" "strings" + "sync" "testing" "time" @@ -18,12 +20,14 @@ import ( "github.com/linuxboot/contest/pkg/cerrors" "github.com/linuxboot/contest/pkg/event" + "github.com/linuxboot/contest/pkg/event/testevent" "github.com/linuxboot/contest/pkg/target" "github.com/linuxboot/contest/pkg/test" "github.com/linuxboot/contest/pkg/types" "github.com/linuxboot/contest/pkg/xcontext" "github.com/linuxboot/contest/pkg/xcontext/bundles/logrusctx" "github.com/linuxboot/contest/pkg/xcontext/logger" + "github.com/linuxboot/contest/plugins/teststeps" "github.com/linuxboot/contest/tests/common" "github.com/linuxboot/contest/tests/common/goroutine_leak_check" "github.com/linuxboot/contest/tests/plugins/teststeps/badtargets" @@ -99,7 +103,7 @@ func (s *TestRunnerSuite) newTestStep(ctx xcontext.Context, label string, failPc teststep.FailPctParam: []test.Param{*test.NewParam(fmt.Sprintf("%d", failPct))}, teststep.FailTargetsParam: []test.Param{*test.NewParam(failTargets)}, teststep.DelayTargetsParam: []test.Param{*test.NewParam(delayTargets)}, - }) + }, nil) } func (s *TestRunnerSuite) runWithTimeout(ctx xcontext.Context, @@ -302,7 +306,7 @@ func (s *TestRunnerSuite) TestNoReturnStepWithCorrectTargetForwarding() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", noreturn.Name, nil), + s.NewStep(ctx, "Step 1", noreturn.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -319,7 +323,7 @@ func (s *TestRunnerSuite) TestStepPanics() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", panicstep.Name, nil), + s.NewStep(ctx, "Step 1", panicstep.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -337,7 +341,7 @@ func (s *TestRunnerSuite) TestStepClosesChannels() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", channels.Name, nil), + s.NewStep(ctx, "Step 1", channels.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -360,7 +364,7 @@ func (s *TestRunnerSuite) TestStepYieldsResultForNonexistentTarget() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("TExtra")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", badtargets.Name, nil), + s.NewStep(ctx, "Step 1", badtargets.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -382,7 +386,7 @@ func (s *TestRunnerSuite) TestStepYieldsDuplicateResult() { []test.TestStepBundle{ // TGood makes it past here unscathed and gets delayed in Step 2, // TDup also emerges fine at first but is then returned again, and that's bad. - s.NewStep(ctx, "Step 1", badtargets.Name, nil), + s.NewStep(ctx, "Step 1", badtargets.Name, nil, nil), s.newTestStep(ctx, "Step 2", 0, "", "TGood=100"), }, ) @@ -399,7 +403,7 @@ func (s *TestRunnerSuite) TestStepLosesTargets() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("TGood"), tgt("TDrop")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", badtargets.Name, nil), + s.NewStep(ctx, "Step 1", badtargets.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -420,7 +424,7 @@ func (s *TestRunnerSuite) TestStepYieldsResultForUnexpectedTarget() { // TExtra2 fails here. s.newTestStep(ctx, "Step 1", 0, "TExtra2", ""), // Yet, a result for it is returned here, which we did not expect. - s.NewStep(ctx, "Step 2", badtargets.Name, nil), + s.NewStep(ctx, "Step 2", badtargets.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -466,6 +470,118 @@ func (s *TestRunnerSuite) TestRandomizedMultiStep() { require.Greater(s.T(), numFinished, 0) } +func (s *TestRunnerSuite) TestVariables() { + ctx, cancel := xcontext.WithCancel(logrusctx.NewContext(logger.LevelDebug)) + defer cancel() + + var ( + pause xcontext.CancelFunc + mu sync.Mutex + ) + require.NoError(s.T(), s.RegisterStateFullStep( + func(ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, + ) (json.RawMessage, error) { + _, err := teststeps.ForEachTargetWithResume(ctx, ch, resumeState, 1, + func(ctx xcontext.Context, target *teststeps.TargetWithData) error { + require.NoError(s.T(), stepsVars.Add(target.Target.ID, "target_id", target.Target.ID)) + + // TODO: we are having a race-condition in event appearance between TargetIn generated by StepRunner + // and events generated by the step. Will address in another PR + time.Sleep(10 * time.Millisecond) + + var resultValue string + err := stepsVars.Get(target.Target.ID, "input", &resultValue) + // only second step will get a variable + if err == nil { + payLoad := json.RawMessage(resultValue) + require.NoError(s.T(), ev.Emit(ctx, testevent.Data{ + Target: target.Target, + EventName: "dummy", + Payload: &payLoad, + })) + } + + return func() error { + mu.Lock() + defer mu.Unlock() + if pause != nil { + pause() + return xcontext.ErrPaused + } + return nil + }() + }) + return nil, err + }, + func(ctx xcontext.Context, params test.TestStepParameters) error { + return nil + }, + )) + + targets := []*target.Target{ + tgt("T1"), + } + + var resumeState []byte + { + ctx1, ctxPause := xcontext.WithNotify(ctx, xcontext.ErrPaused) + ctx1, ctxCancel := xcontext.WithCancel(ctx1) + defer ctxCancel() + + mu.Lock() + pause = ctxPause + mu.Unlock() + + tr := newTestRunner() + var err error + resumeState, _, err = s.runWithTimeout(ctx1, tr, nil, 1, 2*time.Second, + targets, + []test.TestStepBundle{ + s.NewStep(ctx, "step1", stateFullStepName, nil, nil), + s.NewStep(ctx, "step2", stateFullStepName, nil, map[string]string{ + "input": "step1.target_id", + }), + }, + ) + require.IsType(s.T(), xcontext.ErrPaused, err) + require.NotEmpty(s.T(), resumeState) + } + + { + mu.Lock() + pause = nil + mu.Unlock() + + tr := newTestRunner() + var err error + resumeState, _, err = s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, + targets, + []test.TestStepBundle{ + s.NewStep(ctx, "step1", stateFullStepName, nil, nil), + s.NewStep(ctx, "step2", stateFullStepName, nil, map[string]string{ + "input": "step1.target_id", + }), + }, + ) + require.NoError(s.T(), err) + require.Empty(s.T(), resumeState) + } + + require.Equal(s.T(), ` +{[1 1 SimpleTest 0 step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 step1][Target{ID: "T1"} TargetOut]} +{[1 1 SimpleTest 0 step2][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 step2][Target{ID: "T1"} dummy &"T1"]} +{[1 1 SimpleTest 0 step2][Target{ID: "T1"} TargetOut]} +`, s.MemoryStorage.GetTestEvents(ctx, testName)) +} + // Test pausing/resuming a naive step that does not cooperate. // In this case we drain input, wait for all targets to emerge and exit gracefully. func (s *TestRunnerSuite) TestPauseResumeSimple() { diff --git a/pkg/storage/limits/limits.go b/pkg/storage/limits/limits.go index 0c4efa8c..847d537b 100644 --- a/pkg/storage/limits/limits.go +++ b/pkg/storage/limits/limits.go @@ -37,15 +37,15 @@ const MaxTestNameLen = 32 // ValidateTestName retruns error if the test name does not match storage limitations func (v *Validator) ValidateTestName(testName string) error { - return v.validate(testName, "Test name", MaxTestNameLen) + return v.validate(testName, "Test name", false, MaxTestNameLen) } // MaxTestStepLabelLen is a max length of test step label field const MaxTestStepLabelLen = 32 -// ValidateTestStepLabel retruns error if the test step label does not match storage limitations -func (v *Validator) ValidateTestStepLabel(testStepLabel string) error { - return v.validate(testStepLabel, "Test step label", MaxTestStepLabelLen) +// ValidateTestStepLabel returns error if the test step label does not match storage limitations +func (v *Validator) ValidateTestStepLabel(testStepName string) error { + return v.validate(testStepName, "Test step label", false, MaxTestStepLabelLen) } // MaxJobNameLen is a max length of job name field @@ -53,7 +53,7 @@ const MaxJobNameLen = 64 // ValidateJobName retruns error if the job name does not match storage limitations func (v *Validator) ValidateJobName(jobName string) error { - return v.validate(jobName, "Job name", MaxJobNameLen) + return v.validate(jobName, "Job name", true, MaxJobNameLen) } // MaxEventNameLen is a max length of event name field @@ -61,7 +61,7 @@ const MaxEventNameLen = 32 // ValidateEventName retruns error if the event name does not match storage limitations func (v *Validator) ValidateEventName(eventName string) error { - return v.validate(eventName, "Event name", MaxEventNameLen) + return v.validate(eventName, "Event name", true, MaxEventNameLen) } // MaxReporterNameLen is a max length of reporter name field @@ -69,7 +69,7 @@ const MaxReporterNameLen = 32 // ValidateReporterName retruns error if the reporter name does not match storage limitations func (v *Validator) ValidateReporterName(reporterName string) error { - return v.validate(reporterName, "Reporter name", MaxReporterNameLen) + return v.validate(reporterName, "Reporter name", true, MaxReporterNameLen) } // MaxRequestorNameLen is a max length of Requestor name field @@ -77,7 +77,7 @@ const MaxRequestorNameLen = 32 // ValidateRequestorName retruns error if the requestor name does not match storage limitations func (v *Validator) ValidateRequestorName(requestorName string) error { - return v.validate(requestorName, "Requestor name", MaxRequestorNameLen) + return v.validate(requestorName, "Requestor name", true, MaxRequestorNameLen) } // MaxServerIDLen is a max length of server id field @@ -85,10 +85,10 @@ const MaxServerIDLen = 64 // ValidateServerID retruns error if the ServerID does not match storage limitations func (v *Validator) ValidateServerID(serverID string) error { - return v.validate(serverID, "Server ID", MaxServerIDLen) + return v.validate(serverID, "Server ID", true, MaxServerIDLen) } -func (v *Validator) validate(data string, dataName string, maxDataLen int) error { +func (v *Validator) validate(data string, dataName string, allowEmpty bool, maxDataLen int) error { if l := len(data); l > maxDataLen { return ErrParameterIsTooLong{DataName: dataName, MaxLen: maxDataLen, ActualLen: l} } diff --git a/pkg/storage/limits/limits_test.go b/pkg/storage/limits/limits_test.go index c2184ec1..e186de12 100644 --- a/pkg/storage/limits/limits_test.go +++ b/pkg/storage/limits/limits_test.go @@ -128,12 +128,18 @@ func TestTestStepLabel(t *testing.T) { require.NoError(t, err) err = pluginRegistry.RegisterTestFetcher(literal.Load()) require.NoError(t, err) + err = pluginRegistry.RegisterTestStep(echo.Load()) + require.NoError(t, err) err = pluginRegistry.RegisterReporter(noop.Load()) require.NoError(t, err) testFetchParams, err := json.Marshal(&literal.FetchParameters{ - TestName: "AA", + TestName: "AAA", Steps: []*test.TestStepDescriptor{{ + Name: echo.Name, + Parameters: test.TestStepParameters{ + "text": []test.Param{*test.NewParam("\"abc\"")}, + }, Label: strings.Repeat("A", limits.MaxTestStepLabelLen+1), }}, }) diff --git a/pkg/test/step.go b/pkg/test/step.go index 8f281b7c..e48d4cbf 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -9,14 +9,22 @@ import ( "encoding/json" "errors" "fmt" - "strconv" - "github.com/linuxboot/contest/pkg/event" "github.com/linuxboot/contest/pkg/event/testevent" "github.com/linuxboot/contest/pkg/target" "github.com/linuxboot/contest/pkg/xcontext" + "strconv" ) +// StepVariable is a pair that indicates a variable name produces by a step +type StepVariable struct { + StepName string + VariableName string +} + +// StepVariablesMapping is a mapping between produced and consumed variables +type StepVariablesMapping map[string]StepVariable + // TestStepParameters represents the parameters that a TestStep should consume // according to the Test descriptor fetched by the TestFetcher type TestStepParameters map[string][]Param @@ -78,18 +86,20 @@ type TestStepsDescriptors struct { // TestStepDescriptor is the definition of a test step matching a test step // configuration. type TestStepDescriptor struct { - Name string - Label string - Parameters TestStepParameters + Name string + Label string + Parameters TestStepParameters + VariablesMapping map[string]string } // TestStepBundle bundles the selected TestStep together with its parameters as // specified in the Test descriptor fetched by the TestFetcher type TestStepBundle struct { - TestStep TestStep - TestStepLabel string - Parameters TestStepParameters - AllowedEvents map[event.Name]bool + TestStep TestStep + TestStepLabel string + Parameters TestStepParameters + VariablesMapping StepVariablesMapping + AllowedEvents map[event.Name]bool } // TestStepResult is used by TestSteps to report result for a particular target. @@ -107,13 +117,20 @@ type TestStepChannels struct { Out chan<- TestStepResult } +// StepsVariables represents a read/write access for step variables +type StepsVariables interface { + Add(tgtID string, name string, v interface{}) error + Get(tgtID string, name string, out interface{}) error +} + // TestStep is the interface that all steps need to implement to be executed // by the TestRunner type TestStep interface { // Name returns the name of the step Name() string // Run runs the test step. The test step is expected to be synchronous. - Run(ctx xcontext.Context, ch TestStepChannels, params TestStepParameters, ev testevent.Emitter, + Run(ctx xcontext.Context, ch TestStepChannels, ev testevent.Emitter, + stepsVars StepsVariables, params TestStepParameters, resumeState json.RawMessage) (json.RawMessage, error) // ValidateParameters checks that the parameters are correct before passing // them to Run. diff --git a/plugins/teststeps/cmd/cmd.go b/plugins/teststeps/cmd/cmd.go index b77c08d9..825f086a 100644 --- a/plugins/teststeps/cmd/cmd.go +++ b/plugins/teststeps/cmd/cmd.go @@ -93,7 +93,14 @@ func emitEvent(ctx xcontext.Context, name event.Name, payload interface{}, tgt * } // Run executes the cmd step. -func (ts *Cmd) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *Cmd) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { log := ctx.Logger() if err := ts.validateAndPopulate(params); err != nil { diff --git a/plugins/teststeps/cpucmd/cpucmd.go b/plugins/teststeps/cpucmd/cpucmd.go index 775e1ca0..ebf2b8ee 100644 --- a/plugins/teststeps/cpucmd/cpucmd.go +++ b/plugins/teststeps/cpucmd/cpucmd.go @@ -70,7 +70,14 @@ func (ts CPUCmd) Name() string { } // Run executes the cmd step. -func (ts *CPUCmd) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *CPUCmd) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { log := ctx.Logger() // XXX: Dragons ahead! The target (%t) substitution, and function // expression evaluations are done at run-time, so they may still fail diff --git a/plugins/teststeps/echo/echo.go b/plugins/teststeps/echo/echo.go index 0c3f2672..a3bb4514 100644 --- a/plugins/teststeps/echo/echo.go +++ b/plugins/teststeps/echo/echo.go @@ -51,7 +51,14 @@ func (e Step) Name() string { } // Run executes the step -func (e Step) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (e Step) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { for { select { case target, ok := <-ch.In: diff --git a/plugins/teststeps/example/example.go b/plugins/teststeps/example/example.go index 6185ac4a..8d262473 100644 --- a/plugins/teststeps/example/example.go +++ b/plugins/teststeps/example/example.go @@ -61,7 +61,14 @@ func (ts *Step) shouldFail(t *target.Target) bool { } // Run executes the example step. -func (ts *Step) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *Step) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { f := func(ctx xcontext.Context, target *target.Target) error { ctx.Infof("Executing on target %s", target) // NOTE: you may want more robust error handling here, possibly just diff --git a/plugins/teststeps/exec/exec.go b/plugins/teststeps/exec/exec.go index 73f3b826..1392cf29 100644 --- a/plugins/teststeps/exec/exec.go +++ b/plugins/teststeps/exec/exec.go @@ -52,7 +52,14 @@ func (ts TestStep) Name() string { } // Run executes the step. -func (ts *TestStep) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *TestStep) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { if err := ts.populateParams(params); err != nil { return nil, err } diff --git a/plugins/teststeps/randecho/randecho.go b/plugins/teststeps/randecho/randecho.go index 700f9d75..a3b39820 100644 --- a/plugins/teststeps/randecho/randecho.go +++ b/plugins/teststeps/randecho/randecho.go @@ -54,7 +54,14 @@ func (e Step) Name() string { } // Run executes the step -func (e Step) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (e Step) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { return teststeps.ForEachTarget(Name, ctx, ch, func(ctx xcontext.Context, target *target.Target) error { r := rand.Intn(2) diff --git a/plugins/teststeps/s3fileupload/s3fileupload.go b/plugins/teststeps/s3fileupload/s3fileupload.go index 258a7e84..e06f82a2 100644 --- a/plugins/teststeps/s3fileupload/s3fileupload.go +++ b/plugins/teststeps/s3fileupload/s3fileupload.go @@ -83,8 +83,14 @@ func emitEvent(ctx xcontext.Context, name event.Name, payload interface{}, tgt * } // Run executes the awsFileUpload. -func (ts *FileUpload) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, - ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *FileUpload) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { // Validate the parameter if err := ts.validateAndPopulate(params); err != nil { return nil, err diff --git a/plugins/teststeps/sleep/sleep.go b/plugins/teststeps/sleep/sleep.go index f51c6013..9040d061 100644 --- a/plugins/teststeps/sleep/sleep.go +++ b/plugins/teststeps/sleep/sleep.go @@ -68,7 +68,14 @@ type sleepStepData struct { } // Run executes the step -func (ss *sleepStep) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ss *sleepStep) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { dur, err := getDuration(params) if err != nil { return nil, err diff --git a/plugins/teststeps/sshcmd/sshcmd.go b/plugins/teststeps/sshcmd/sshcmd.go index c2eb2cfe..6337062c 100644 --- a/plugins/teststeps/sshcmd/sshcmd.go +++ b/plugins/teststeps/sshcmd/sshcmd.go @@ -69,7 +69,14 @@ func (ts SSHCmd) Name() string { } // Run executes the cmd step. -func (ts *SSHCmd) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *SSHCmd) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { log := ctx.Logger() // XXX: Dragons ahead! The target (%t) substitution, and function diff --git a/plugins/teststeps/terminalexpect/terminalexpect.go b/plugins/teststeps/terminalexpect/terminalexpect.go index fe523ff6..3f44a857 100644 --- a/plugins/teststeps/terminalexpect/terminalexpect.go +++ b/plugins/teststeps/terminalexpect/terminalexpect.go @@ -13,13 +13,13 @@ import ( "strings" "time" + "github.com/insomniacslk/termhook" "github.com/linuxboot/contest/pkg/event" "github.com/linuxboot/contest/pkg/event/testevent" "github.com/linuxboot/contest/pkg/target" "github.com/linuxboot/contest/pkg/test" "github.com/linuxboot/contest/pkg/xcontext" "github.com/linuxboot/contest/plugins/teststeps" - "github.com/insomniacslk/termhook" ) // Name is the name used to look this plugin up. @@ -54,7 +54,14 @@ func match(match string, log xcontext.Logger) termhook.LineHandler { } // Run executes the terminal step. -func (ts *TerminalExpect) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *TerminalExpect) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { log := ctx.Logger() if err := ts.validateAndPopulate(params); err != nil { diff --git a/plugins/teststeps/waitport/waitport.go b/plugins/teststeps/waitport/waitport.go index 5e2ff1dc..1526457f 100644 --- a/plugins/teststeps/waitport/waitport.go +++ b/plugins/teststeps/waitport/waitport.go @@ -44,7 +44,14 @@ func (ts *WaitPort) Name() string { } // Run executes the cmd step. -func (ts *WaitPort) Run(ctx xcontext.Context, ch test.TestStepChannels, inputParams test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *WaitPort) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { params, err := parseParameters(inputParams) if err != nil { return nil, err diff --git a/plugins/teststeps/waitport/waitport_test.go b/plugins/teststeps/waitport/waitport_test.go index a26fd5e8..e04875fb 100644 --- a/plugins/teststeps/waitport/waitport_test.go +++ b/plugins/teststeps/waitport/waitport_test.go @@ -75,7 +75,7 @@ func TestWaitForTCPPort(t *testing.T) { } plugin := &WaitPort{} - if _, err = plugin.Run(ctx, testStepChannels, params, ev, nil); err != nil { + if _, err = plugin.Run(ctx, testStepChannels, ev, nil, params, nil); err != nil { t.Errorf("Plugin run failed: '%v'", err) } wg.Wait() diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index ff841546..ae6b2a53 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -256,20 +256,20 @@ func (ts *E2ETestSuite) TestSimple() { require.Equal(ts.T(), fmt.Sprintf(` {[%d 1 Test 1 0 ][Target{ID: "T1"} TargetAcquired]} -{[%d 1 Test 1 0 Test 1 Step 1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} -{[%d 1 Test 1 0 Test 1 Step 2][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 2, target T1\\n\"}"]} +{[%d 1 Test 1 0 Test1Step1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} +{[%d 1 Test 1 0 Test1Step2][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 2, target T1\\n\"}"]} {[%d 1 Test 1 0 ][Target{ID: "T1"} TargetReleased]} {[%d 1 Test 2 0 ][Target{ID: "T2"} TargetAcquired]} -{[%d 1 Test 2 0 Test 2 Step 1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} -{[%d 1 Test 2 0 Test 2 Step 2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 2, target T2\\n\"}"]} +{[%d 1 Test 2 0 Test2Step1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} +{[%d 1 Test 2 0 Test2Step2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 2, target T2\\n\"}"]} {[%d 1 Test 2 0 ][Target{ID: "T2"} TargetReleased]} {[%d 2 Test 1 0 ][Target{ID: "T1"} TargetAcquired]} -{[%d 2 Test 1 0 Test 1 Step 1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} -{[%d 2 Test 1 0 Test 1 Step 2][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 2, target T1\\n\"}"]} +{[%d 2 Test 1 0 Test1Step1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} +{[%d 2 Test 1 0 Test1Step2][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 2, target T1\\n\"}"]} {[%d 2 Test 1 0 ][Target{ID: "T1"} TargetReleased]} {[%d 2 Test 2 0 ][Target{ID: "T2"} TargetAcquired]} -{[%d 2 Test 2 0 Test 2 Step 1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} -{[%d 2 Test 2 0 Test 2 Step 2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 2, target T2\\n\"}"]} +{[%d 2 Test 2 0 Test2Step1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} +{[%d 2 Test 2 0 Test2Step2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 2, target T2\\n\"}"]} {[%d 2 Test 2 0 ][Target{ID: "T2"} TargetReleased]} `, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID), es, @@ -321,22 +321,22 @@ func (ts *E2ETestSuite) TestPauseResume() { require.Equal(ts.T(), fmt.Sprintf(` {[%d 1 Test 1 0 ][Target{ID: "T1"} TargetAcquired]} -{[%d 1 Test 1 0 Test 1 Step 1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} -{[%d 1 Test 1 0 Test 1 Step 4][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 3, target T1\\n\"}"]} +{[%d 1 Test 1 0 Test1Step1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} +{[%d 1 Test 1 0 Test1Step4][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 3, target T1\\n\"}"]} {[%d 1 Test 1 0 ][Target{ID: "T1"} TargetReleased]} {[%d 1 Test 2 0 ][Target{ID: "T2"} TargetAcquired]} -{[%d 1 Test 2 0 Test 2 Step 1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} -{[%d 1 Test 2 0 Test 2 Step 2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"\"}"]} -{[%d 1 Test 2 0 Test 2 Step 3][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 3, target T2\\n\"}"]} +{[%d 1 Test 2 0 Test2Step1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} +{[%d 1 Test 2 0 Test2Step2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"\"}"]} +{[%d 1 Test 2 0 Test2Step3][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 3, target T2\\n\"}"]} {[%d 1 Test 2 0 ][Target{ID: "T2"} TargetReleased]} {[%d 2 Test 1 0 ][Target{ID: "T1"} TargetAcquired]} -{[%d 2 Test 1 0 Test 1 Step 1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} -{[%d 2 Test 1 0 Test 1 Step 4][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 3, target T1\\n\"}"]} +{[%d 2 Test 1 0 Test1Step1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} +{[%d 2 Test 1 0 Test1Step4][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 3, target T1\\n\"}"]} {[%d 2 Test 1 0 ][Target{ID: "T1"} TargetReleased]} {[%d 2 Test 2 0 ][Target{ID: "T2"} TargetAcquired]} -{[%d 2 Test 2 0 Test 2 Step 1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} -{[%d 2 Test 2 0 Test 2 Step 2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"\"}"]} -{[%d 2 Test 2 0 Test 2 Step 3][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 3, target T2\\n\"}"]} +{[%d 2 Test 2 0 Test2Step1][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 1, target T2\\n\"}"]} +{[%d 2 Test 2 0 Test2Step2][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"\"}"]} +{[%d 2 Test 2 0 Test2Step3][Target{ID: "T2"} CmdStdout &"{\"Msg\":\"Test 2, Step 3, target T2\\n\"}"]} {[%d 2 Test 2 0 ][Target{ID: "T2"} TargetReleased]} `, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID), es, @@ -430,16 +430,16 @@ func (ts *E2ETestSuite) TestRetries() { require.Equal(ts.T(), fmt.Sprintf(` {[%d 1 Test 1 0 ][Target{ID: "T1"} TargetAcquired]} -{[%d 1 Test 1 0 Step 1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} -{[%d 1 Test 1 0 Step 1][Target{ID: "T1"} TargetOut]} -{[%d 1 Test 1 0 Step 2][Target{ID: "T1"} TargetErr &"{\"Error\":\"context deadline exceeded\"}"]} +{[%d 1 Test 1 0 Step1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} +{[%d 1 Test 1 0 Step1][Target{ID: "T1"} TargetOut]} +{[%d 1 Test 1 0 Step2][Target{ID: "T1"} TargetErr &"{\"Error\":\"context deadline exceeded\"}"]} {[%d 1 Test 1 0 ][Target{ID: "T1"} TargetReleased]} {[%d 1 Test 1 1 ][Target{ID: "T1"} TargetAcquired]} -{[%d 1 Test 1 1 Step 1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} -{[%d 1 Test 1 1 Step 1][Target{ID: "T1"} TargetOut]} -{[%d 1 Test 1 1 Step 2][Target{ID: "T1"} TargetOut]} -{[%d 1 Test 1 1 Step 3][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} -{[%d 1 Test 1 1 Step 3][Target{ID: "T1"} TargetOut]} +{[%d 1 Test 1 1 Step1][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} +{[%d 1 Test 1 1 Step1][Target{ID: "T1"} TargetOut]} +{[%d 1 Test 1 1 Step2][Target{ID: "T1"} TargetOut]} +{[%d 1 Test 1 1 Step3][Target{ID: "T1"} CmdStdout &"{\"Msg\":\"Test 1, Step 1, target T1\\n\"}"]} +{[%d 1 Test 1 1 Step3][Target{ID: "T1"} TargetOut]} {[%d 1 Test 1 1 ][Target{ID: "T1"} TargetReleased]} `, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID, jobID), es, diff --git a/tests/e2e/test-resume.yaml b/tests/e2e/test-resume.yaml index 1e050809..8d3b1143 100644 --- a/tests/e2e/test-resume.yaml +++ b/tests/e2e/test-resume.yaml @@ -12,22 +12,22 @@ TestDescriptors: TestName: Test 1 Steps: - name: cmd - label: Test 1 Step 1 + label: Test1Step1 parameters: executable: [echo] args: ["Test 1, Step 1, target {{ .ID }}"] emit_stdout: [true] emit_stderr: [true] - name: sleep # Supports pause / resume. - label: Test 1 Step 2 + label: Test1Step2 parameters: duration: [2s] - name: sleep # Supports pause / resume. - label: Test 1 Step 3 + label: Test1Step3 parameters: duration: [2s] - name: cmd - label: Test 1 Step 4 + label: Test1Step4 parameters: executable: [echo] args: ["Test 1, Step 3, target {{ .ID }}"] @@ -43,21 +43,21 @@ TestDescriptors: TestName: Test 2 Steps: - name: cmd - label: Test 2 Step 1 + label: Test2Step1 parameters: executable: [echo] args: ["Test 2, Step 1, target {{ .ID }}"] emit_stdout: [true] emit_stderr: [true] - name: cmd # Does not support pause, will have to wait. - label: Test 2 Step 2 + label: Test2Step2 parameters: executable: [sleep] args: [2] emit_stdout: [true] emit_stderr: [true] - name: cmd - label: Test 2 Step 3 + label: Test2Step3 parameters: executable: [echo] args: ["Test 2, Step 3, target {{ .ID }}"] diff --git a/tests/e2e/test-retry.yaml b/tests/e2e/test-retry.yaml index 71c798e9..1eb33782 100644 --- a/tests/e2e/test-retry.yaml +++ b/tests/e2e/test-retry.yaml @@ -14,14 +14,14 @@ TestDescriptors: TestName: Test 1 Steps: - name: cmd - label: Step 1 + label: Step1 parameters: executable: [echo] args: ["Test 1, Step 1, target {{ .ID }}"] emit_stdout: [true] emit_stderr: [true] - name: waitport - label: Step 2 + label: Step2 parameters: target: ["localhost"] port: ["[[ .WaitPort]]"] @@ -29,7 +29,7 @@ TestDescriptors: protocol: ["tcp"] timeout: ["500ms"] - name: cmd - label: Step 3 + label: Step3 parameters: executable: [ echo ] args: [ "Test 1, Step 1, target {{ .ID }}" ] diff --git a/tests/e2e/test-simple.yaml b/tests/e2e/test-simple.yaml index 5541bfb0..f169b9cf 100644 --- a/tests/e2e/test-simple.yaml +++ b/tests/e2e/test-simple.yaml @@ -15,14 +15,14 @@ TestDescriptors: TestName: Test 1 Steps: - name: cmd - label: Test 1 Step 1 + label: Test1Step1 parameters: executable: [echo] args: ["Test 1, Step 1, target {{ .ID }}"] emit_stdout: [true] emit_stderr: [true] - name: cmd - label: Test 1 Step 2 + label: Test1Step2 parameters: executable: [echo] args: ["Test 1, Step 2, target {{ .ID }}"] @@ -38,14 +38,14 @@ TestDescriptors: TestName: Test 2 Steps: - name: cmd - label: Test 2 Step 1 + label: Test2Step1 parameters: executable: [echo] args: ["Test 2, Step 1, target {{ .ID }}"] emit_stdout: [true] emit_stderr: [true] - name: cmd - label: Test 2 Step 2 + label: Test2Step2 parameters: executable: [echo] args: ["Test 2, Step 2, target {{ .ID }}"] diff --git a/tests/plugins/teststeps/badtargets/badtargets.go b/tests/plugins/teststeps/badtargets/badtargets.go index 858779a1..2ea7e4ae 100644 --- a/tests/plugins/teststeps/badtargets/badtargets.go +++ b/tests/plugins/teststeps/badtargets/badtargets.go @@ -31,7 +31,14 @@ func (ts *badTargets) Name() string { } // Run executes a step that messes up the flow of targets. -func (ts *badTargets) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *badTargets) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { for { select { case tgt, ok := <-ch.In: diff --git a/tests/plugins/teststeps/channels/channels.go b/tests/plugins/teststeps/channels/channels.go index 22a8178b..971cd387 100644 --- a/tests/plugins/teststeps/channels/channels.go +++ b/tests/plugins/teststeps/channels/channels.go @@ -29,7 +29,14 @@ func (ts *channels) Name() string { } // Run executes a step that runs fine but closes its output channels on exit. -func (ts *channels) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *channels) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { for target := range ch.In { ch.Out <- test.TestStepResult{Target: target} } diff --git a/tests/plugins/teststeps/crash/crash.go b/tests/plugins/teststeps/crash/crash.go index e0c54875..d3de211e 100644 --- a/tests/plugins/teststeps/crash/crash.go +++ b/tests/plugins/teststeps/crash/crash.go @@ -30,7 +30,14 @@ func (ts *crash) Name() string { } // Run executes a step which returns an error -func (ts *crash) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *crash) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { return nil, fmt.Errorf("TestStep crashed") } diff --git a/tests/plugins/teststeps/fail/fail.go b/tests/plugins/teststeps/fail/fail.go index 0c155cb2..2042b08b 100644 --- a/tests/plugins/teststeps/fail/fail.go +++ b/tests/plugins/teststeps/fail/fail.go @@ -32,7 +32,14 @@ func (ts *fail) Name() string { } // Run executes a step that fails all the targets it receives. -func (ts *fail) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *fail) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { return teststeps.ForEachTarget(Name, ctx, ch, func(ctx xcontext.Context, t *target.Target) error { return fmt.Errorf("Integration test failure for %v", t) }) diff --git a/tests/plugins/teststeps/hanging/hanging.go b/tests/plugins/teststeps/hanging/hanging.go index 34757174..434af72a 100644 --- a/tests/plugins/teststeps/hanging/hanging.go +++ b/tests/plugins/teststeps/hanging/hanging.go @@ -29,7 +29,14 @@ func (ts *hanging) Name() string { } // Run executes a step that does not process any targets and never returns. -func (ts *hanging) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *hanging) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { channel := make(chan struct{}) <-channel return nil, nil diff --git a/tests/plugins/teststeps/noop/noop.go b/tests/plugins/teststeps/noop/noop.go index 8f021e1a..c380f431 100644 --- a/tests/plugins/teststeps/noop/noop.go +++ b/tests/plugins/teststeps/noop/noop.go @@ -31,7 +31,14 @@ func (ts *noop) Name() string { } // Run executes a step that does nothing and returns targets with success. -func (ts *noop) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *noop) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { return teststeps.ForEachTarget(Name, ctx, ch, func(ctx xcontext.Context, t *target.Target) error { return nil }) diff --git a/tests/plugins/teststeps/noreturn/noreturn.go b/tests/plugins/teststeps/noreturn/noreturn.go index 2c677444..e145bbb9 100644 --- a/tests/plugins/teststeps/noreturn/noreturn.go +++ b/tests/plugins/teststeps/noreturn/noreturn.go @@ -29,7 +29,14 @@ func (ts *noreturnStep) Name() string { } // Run executes a step that never returns. -func (ts *noreturnStep) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *noreturnStep) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { for target := range ch.In { ch.Out <- test.TestStepResult{Target: target} } diff --git a/tests/plugins/teststeps/panicstep/panicstep.go b/tests/plugins/teststeps/panicstep/panicstep.go index 20322e58..5b565902 100644 --- a/tests/plugins/teststeps/panicstep/panicstep.go +++ b/tests/plugins/teststeps/panicstep/panicstep.go @@ -29,7 +29,14 @@ func (ts *panicStep) Name() string { } // Run executes the example step. -func (ts *panicStep) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *panicStep) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { panic("panic step") } diff --git a/tests/plugins/teststeps/readmeta/readmeta.go b/tests/plugins/teststeps/readmeta/readmeta.go index 312c362e..7ad58a56 100644 --- a/tests/plugins/teststeps/readmeta/readmeta.go +++ b/tests/plugins/teststeps/readmeta/readmeta.go @@ -37,7 +37,14 @@ func (ts *readmeta) Name() string { } // Run executes a step that reads the job metadata that must be in the context and panics if it is missing. -func (ts *readmeta) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *readmeta) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + inputParams test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { return teststeps.ForEachTarget(Name, ctx, ch, func(ctx xcontext.Context, t *target.Target) error { jobID, ok1 := types.JobIDFromContext(ctx) if jobID == 0 || !ok1 { diff --git a/tests/plugins/teststeps/slowecho/slowecho.go b/tests/plugins/teststeps/slowecho/slowecho.go index 97e20104..280b0d86 100644 --- a/tests/plugins/teststeps/slowecho/slowecho.go +++ b/tests/plugins/teststeps/slowecho/slowecho.go @@ -80,7 +80,14 @@ func (e *Step) ValidateParameters(_ xcontext.Context, params test.TestStepParame } // Run executes the step -func (e *Step) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (e *Step) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { sleep, err := sleepTime(params.GetOne("sleep").String()) if err != nil { return nil, err diff --git a/tests/plugins/teststeps/teststep/teststep.go b/tests/plugins/teststeps/teststep/teststep.go index d9a3e18d..831f75ad 100644 --- a/tests/plugins/teststeps/teststep/teststep.go +++ b/tests/plugins/teststeps/teststep/teststep.go @@ -65,7 +65,14 @@ func (ts *Step) shouldFail(t *target.Target, params test.TestStepParameters) boo } // Run executes the example step. -func (ts *Step) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, ev testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *Step) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + ev testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { f := func(ctx xcontext.Context, target *target.Target) error { // Sleep to ensure TargetIn fires first. This simplifies test assertions. time.Sleep(50 * time.Millisecond) From 63085054ede9866e69e8cf592b63677d842fdbde Mon Sep 17 00:00:00 2001 From: Ilya Date: Mon, 27 Jun 2022 11:15:18 +0100 Subject: [PATCH 2/7] Move CheckVariableName to test package Add unit tests for CheckVariableName Code-style improvements based on PR's comments Signed-off-by: Ilya --- pkg/jobmanager/job_test.go | 4 +- pkg/pluginregistry/bundles.go | 37 +--- pkg/runner/test_runner.go | 148 --------------- pkg/runner/test_runner_test.go | 222 +++++++++++------------ pkg/runner/test_steps_variables.go | 156 ++++++++++++++++ pkg/test/step.go | 27 ++- pkg/test/step_test.go | 8 + tests/integ/jobmanager/common.go | 16 +- tests/integ/jobmanager/jobdescriptors.go | 6 +- 9 files changed, 319 insertions(+), 305 deletions(-) create mode 100644 pkg/runner/test_steps_variables.go diff --git a/pkg/jobmanager/job_test.go b/pkg/jobmanager/job_test.go index 98f0eeb8..3b8490ff 100644 --- a/pkg/jobmanager/job_test.go +++ b/pkg/jobmanager/job_test.go @@ -26,7 +26,7 @@ func TestDisabledTestDescriptor(t *testing.T) { "Steps": [ { "name": "echo", - "label": "echo text", + "label": "echotext", "parameters": { "text": ["Some text1"] } @@ -38,7 +38,7 @@ func TestDisabledTestDescriptor(t *testing.T) { "Steps": [ { "name": "echo", - "label": "echo text", + "label": "echotext", "parameters": { "text": ["Some text1"] } diff --git a/pkg/pluginregistry/bundles.go b/pkg/pluginregistry/bundles.go index 05fd8447..1c2ba4d7 100644 --- a/pkg/pluginregistry/bundles.go +++ b/pkg/pluginregistry/bundles.go @@ -32,12 +32,15 @@ func (r *PluginRegistry) NewTestStepBundle(ctx xcontext.Context, testStepDescrip if len(label) == 0 { return nil, ErrStepLabelIsMandatory{TestStepDescriptor: testStepDescriptor} } + if err := test.CheckVariableName(label); err != nil { + return nil, InvalidVariableFormat{InvalidName: label, Err: err} + } var variablesMapping test.StepVariablesMapping if testStepDescriptor.VariablesMapping != nil { variablesMapping = make(test.StepVariablesMapping) for internalName, mappedName := range testStepDescriptor.VariablesMapping { - if err := checkVariableName(internalName); err != nil { + if err := test.CheckVariableName(internalName); err != nil { return nil, InvalidVariableFormat{InvalidName: internalName, Err: err} } if _, found := variablesMapping[internalName]; found { @@ -48,10 +51,10 @@ func (r *PluginRegistry) NewTestStepBundle(ctx xcontext.Context, testStepDescrip if len(parts) != 2 { return nil, fmt.Errorf("variable mapping '%s' should contain a single '.' separator", mappedName) } - if err := checkVariableName(parts[0]); err != nil { + if err := test.CheckVariableName(parts[0]); err != nil { return nil, InvalidVariableFormat{InvalidName: parts[0], Err: err} } - if err := checkVariableName(parts[1]); err != nil { + if err := test.CheckVariableName(parts[1]); err != nil { return nil, InvalidVariableFormat{InvalidName: parts[1], Err: err} } @@ -161,31 +164,3 @@ func (r *PluginRegistry) NewFinalReporterBundle(reporterName string, reporterPar } return &reporterBundle, nil } - -func isAlpha(ch int32) bool { - return (ch >= 'a' && ch <= 'z') || (ch >= 'A' || ch <= 'Z') -} - -func checkVariableName(s string) error { - if len(s) == 0 { - return fmt.Errorf("empty variable name") - } - for idx, ch := range s { - if idx == 0 { - if !isAlpha(ch) { - return fmt.Errorf("first character should be alpha, got %c", ch) - } - } - if isAlpha(ch) { - continue - } - if ch >= '0' || ch <= '9' { - continue - } - if ch == '_' { - continue - } - return fmt.Errorf("got unxpected character: %c", ch) - } - return nil -} diff --git a/pkg/runner/test_runner.go b/pkg/runner/test_runner.go index faf9d61b..0562a7a8 100644 --- a/pkg/runner/test_runner.go +++ b/pkg/runner/test_runner.go @@ -574,151 +574,3 @@ func (tgs *targetState) String() string { return fmt.Sprintf("[%s %d %s %t %s]", tgs.tgt, tgs.CurStep, tgs.CurPhase, finished, resText) } - -type testStepsVariables struct { - existingSteps map[string]struct{} - - lock sync.Mutex - stepsVariablesByTarget map[string]map[string]stepVariables -} - -func newTestStepsVariables(bundles []test.TestStepBundle) (*testStepsVariables, error) { - result := &testStepsVariables{ - existingSteps: make(map[string]struct{}), - stepsVariablesByTarget: make(map[string]map[string]stepVariables), - } - for _, bs := range bundles { - if len(bs.TestStepLabel) == 0 { - continue - } - if _, found := result.existingSteps[bs.TestStepLabel]; found { - return nil, fmt.Errorf("duplication of test step label: '%s'", bs.TestStepLabel) - } - result.existingSteps[bs.TestStepLabel] = struct{}{} - } - return result, nil -} - -func (tsv *testStepsVariables) initTargetStepsVariables(tgtID string, stepsVars map[string]stepVariables) error { - if _, found := tsv.stepsVariablesByTarget[tgtID]; found { - return fmt.Errorf("duplication of target ID: '%s'", tgtID) - } - if stepsVars == nil { - tsv.stepsVariablesByTarget[tgtID] = make(map[string]stepVariables) - } else { - tsv.stepsVariablesByTarget[tgtID] = stepsVars - } - return nil -} - -func (tsv *testStepsVariables) getTargetStepsVariables(tgtID string) (map[string]stepVariables, error) { - tsv.lock.Lock() - defer tsv.lock.Unlock() - - vars, found := tsv.stepsVariablesByTarget[tgtID] - if !found { - return nil, fmt.Errorf("target ID: '%s' is not found", tgtID) - } - - // make a copy in case of buggy steps that can still be using it - result := make(map[string]stepVariables) - for name, value := range vars { - result[name] = value - } - return result, nil -} - -func (tsv *testStepsVariables) Add(tgtID string, stepLabel string, name string, value json.RawMessage) error { - if err := tsv.checkInput(tgtID, stepLabel, name); err != nil { - return err - } - - // tsv.stepsVariablesByTarget is a readonly map, though its values may change - targetSteps := tsv.stepsVariablesByTarget[tgtID] - tsv.lock.Lock() - defer tsv.lock.Unlock() - - stepVars, found := targetSteps[stepLabel] - if !found { - stepVars = make(stepVariables) - targetSteps[stepLabel] = stepVars - } - stepVars[name] = value - return nil -} - -func (tsv *testStepsVariables) Get(tgtID string, stepLabel string, name string) (json.RawMessage, error) { - if err := tsv.checkInput(tgtID, stepLabel, name); err != nil { - return nil, err - } - - targetSteps := tsv.stepsVariablesByTarget[tgtID] - tsv.lock.Lock() - defer tsv.lock.Unlock() - - stepVars, found := targetSteps[stepLabel] - if !found { - return nil, fmt.Errorf("step '%s' didn't emit any variables yet", stepLabel) - } - value, found := stepVars[name] - if !found { - return nil, fmt.Errorf("step '%s' didn't emit variable '%s' yet", stepLabel, name) - } - return value, nil -} - -func (tsv *testStepsVariables) checkInput(tgtID string, stepLabel string, name string) error { - if len(stepLabel) == 0 { - return fmt.Errorf("empty step name") - } - if len(tgtID) == 0 { - return fmt.Errorf("empty target ID") - } - if len(name) == 0 { - return fmt.Errorf("empty variable name") - } - return nil -} - -type stepVariablesAccessor struct { - stepLabel string - varsMapping test.StepVariablesMapping - tsv *testStepsVariables -} - -func newStepVariablesAccessor(stepLabel string, varsMapping test.StepVariablesMapping, tsv *testStepsVariables) *stepVariablesAccessor { - result := &stepVariablesAccessor{ - stepLabel: stepLabel, - varsMapping: varsMapping, - tsv: tsv, - } - if result.varsMapping == nil { - result.varsMapping = make(test.StepVariablesMapping) - } - return result -} - -func (sva *stepVariablesAccessor) Add(tgtID string, name string, value interface{}) error { - if len(sva.stepLabel) == 0 { - return nil - } - b, err := json.Marshal(value) - if err != nil { - return fmt.Errorf("failed to serialize variable: %v", value) - } - return sva.tsv.Add(tgtID, sva.stepLabel, name, b) -} - -func (sva *stepVariablesAccessor) Get(tgtID string, name string, out interface{}) error { - stepVar, found := sva.varsMapping[name] - if !found { - return fmt.Errorf("no mapping for variable '%s'", name) - } - b, err := sva.tsv.Get(tgtID, stepVar.StepName, stepVar.VariableName) - if err != nil { - return err - } - return json.Unmarshal(b, out) -} - -var _ test.StepsVariables = (*stepVariablesAccessor)(nil) diff --git a/pkg/runner/test_runner_test.go b/pkg/runner/test_runner_test.go index 2f7b2e42..b6b645ad 100644 --- a/pkg/runner/test_runner_test.go +++ b/pkg/runner/test_runner_test.go @@ -144,7 +144,7 @@ func (s *TestRunnerSuite) Test1Step1Success() { _, targetsResults, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 0, "", ""), + s.newTestStep(ctx, "Step1", 0, "", ""), }, ) require.NoError(s.T(), err) @@ -153,14 +153,14 @@ func (s *TestRunnerSuite) Test1Step1Success() { }, targetsResults) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepFinishedEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepFinishedEvent]} `, s.MemoryStorage.GetStepEvents(ctx, testName, "")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestFinishedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetOut]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestFinishedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetOut]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) } @@ -174,7 +174,7 @@ func (s *TestRunnerSuite) Test1StepLongerThanShutdown1Success() { _, targetsResults, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 0, "", "T1=500"), + s.newTestStep(ctx, "Step1", 0, "", "T1=500"), }, ) require.NoError(s.T(), err) @@ -183,14 +183,14 @@ func (s *TestRunnerSuite) Test1StepLongerThanShutdown1Success() { }, targetsResults) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepFinishedEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepFinishedEvent]} `, s.MemoryStorage.GetStepEvents(ctx, testName, "")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestFinishedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetOut]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestFinishedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetOut]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) } @@ -203,7 +203,7 @@ func (s *TestRunnerSuite) Test1Step1Fail() { _, targetsResults, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 100, "", ""), + s.newTestStep(ctx, "Step1", 100, "", ""), }, ) require.NoError(s.T(), err) @@ -211,14 +211,14 @@ func (s *TestRunnerSuite) Test1Step1Fail() { require.Error(s.T(), targetsResults["T1"]) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepFinishedEvent]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 1")) +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepFinishedEvent]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step1")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestFailedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestFailedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) } @@ -231,25 +231,25 @@ func (s *TestRunnerSuite) Test1Step1Success1Fail() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1"), tgt("T2")}, []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 0, "T1", "T2=100"), + s.newTestStep(ctx, "Step1", 0, "T1", "T2=100"), }, ) require.NoError(s.T(), err) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepFinishedEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepFinishedEvent]} `, s.MemoryStorage.GetStepEvents(ctx, testName, "")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestFailedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestFailedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TestFinishedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TargetOut]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TestFinishedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TargetOut]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T2")) } @@ -263,36 +263,36 @@ func (s *TestRunnerSuite) Test3StepsNotReachedStepNotRun() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1"), tgt("T2")}, []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 0, "T1", ""), - s.newTestStep(ctx, "Step 2", 0, "T2", ""), - s.newTestStep(ctx, "Step 3", 0, "", ""), + s.newTestStep(ctx, "Step1", 0, "T1", ""), + s.newTestStep(ctx, "Step2", 0, "T2", ""), + s.newTestStep(ctx, "Step3", 0, "", ""), }, ) require.NoError(s.T(), err) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepFinishedEvent]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 1")) +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepFinishedEvent]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step1")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 2][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 2][(*Target)(nil) TestStepFinishedEvent]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 2")) +{[1 1 SimpleTest 0 Step2][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step2][(*Target)(nil) TestStepFinishedEvent]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step2")) require.Equal(s.T(), "\n\n", s.MemoryStorage.GetStepEvents(ctx, testName, "Step 3")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestFailedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestFailedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TestFinishedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TargetOut]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TargetIn]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TestFailedEvent]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TargetErr &"{\"Error\":\"target failed\"}"]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TestFinishedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TargetOut]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TargetIn]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TestFailedEvent]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TargetErr &"{\"Error\":\"target failed\"}"]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T2")) } @@ -306,12 +306,12 @@ func (s *TestRunnerSuite) TestNoReturnStepWithCorrectTargetForwarding() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", noreturn.Name, nil, nil), + s.NewStep(ctx, "Step1", noreturn.Name, nil, nil), }, ) require.Error(s.T(), err) require.IsType(s.T(), &cerrors.ErrTestStepsNeverReturned{}, err) - require.Contains(s.T(), s.MemoryStorage.GetStepEvents(ctx, testName, "Step 1"), "step [Step 1] did not return") + require.Contains(s.T(), s.MemoryStorage.GetStepEvents(ctx, testName, "Step1"), "step [Step1] did not return") } // A misbehaving step that panics. @@ -323,13 +323,13 @@ func (s *TestRunnerSuite) TestStepPanics() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", panicstep.Name, nil, nil), + s.NewStep(ctx, "Step1", panicstep.Name, nil, nil), }, ) require.Error(s.T(), err) require.IsType(s.T(), &cerrors.ErrTestStepPaniced{}, err) require.Equal(s.T(), "\n\n", s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) - require.Contains(s.T(), s.MemoryStorage.GetStepEvents(ctx, testName, "Step 1"), "step Step 1 paniced") + require.Contains(s.T(), s.MemoryStorage.GetStepEvents(ctx, testName, "Step1"), "step Step1 paniced") } // A misbehaving step that closes its output channel. @@ -341,18 +341,18 @@ func (s *TestRunnerSuite) TestStepClosesChannels() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", channels.Name, nil, nil), + s.NewStep(ctx, "Step1", channels.Name, nil, nil), }, ) require.Error(s.T(), err) require.IsType(s.T(), &cerrors.ErrTestStepClosedChannels{}, err) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetOut]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetOut]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestError &"\"test step Step 1 closed output channels (api violation)\""]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 1")) +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestError &"\"test step Step1 closed output channels (api violation)\""]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step1")) } // A misbehaving step that yields a result for a target that does not exist. @@ -364,15 +364,15 @@ func (s *TestRunnerSuite) TestStepYieldsResultForNonexistentTarget() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("TExtra")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", badtargets.Name, nil, nil), + s.NewStep(ctx, "Step1", badtargets.Name, nil, nil), }, ) require.Error(s.T(), err) require.IsType(s.T(), &cerrors.ErrTestStepReturnedUnexpectedResult{}, err) require.Equal(s.T(), "\n\n", s.MemoryStorage.GetTargetEvents(ctx, testName, "TExtra2")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestError &"\"test step Step 1 returned unexpected result for TExtra2\""]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 1")) +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestError &"\"test step Step1 returned unexpected result for TExtra2\""]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step1")) } // A misbehaving step that yields a duplicate result for a target. @@ -386,8 +386,8 @@ func (s *TestRunnerSuite) TestStepYieldsDuplicateResult() { []test.TestStepBundle{ // TGood makes it past here unscathed and gets delayed in Step 2, // TDup also emerges fine at first but is then returned again, and that's bad. - s.NewStep(ctx, "Step 1", badtargets.Name, nil, nil), - s.newTestStep(ctx, "Step 2", 0, "", "TGood=100"), + s.NewStep(ctx, "Step1", badtargets.Name, nil, nil), + s.newTestStep(ctx, "Step2", 0, "", "TGood=100"), }, ) require.Error(s.T(), err) @@ -403,7 +403,7 @@ func (s *TestRunnerSuite) TestStepLosesTargets() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("TGood"), tgt("TDrop")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step 1", badtargets.Name, nil, nil), + s.NewStep(ctx, "Step1", badtargets.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -422,9 +422,9 @@ func (s *TestRunnerSuite) TestStepYieldsResultForUnexpectedTarget() { []*target.Target{tgt("TExtra"), tgt("TExtra2")}, []test.TestStepBundle{ // TExtra2 fails here. - s.newTestStep(ctx, "Step 1", 0, "TExtra2", ""), + s.newTestStep(ctx, "Step1", 0, "TExtra2", ""), // Yet, a result for it is returned here, which we did not expect. - s.NewStep(ctx, "Step 2", badtargets.Name, nil, nil), + s.NewStep(ctx, "Step2", badtargets.Name, nil, nil), }, ) require.Error(s.T(), err) @@ -444,24 +444,24 @@ func (s *TestRunnerSuite) TestRandomizedMultiStep() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, targets, []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 0, "", "*=10"), // All targets pass the first step, with a slight delay - s.newTestStep(ctx, "Step 2", 25, "", ""), // 25% don't make it past the second step - s.newTestStep(ctx, "Step 3", 25, "", "*=10"), // Another 25% fail at the third step + s.newTestStep(ctx, "Step1", 0, "", "*=10"), // All targets pass the first step, with a slight delay + s.newTestStep(ctx, "Step2", 25, "", ""), // 25% don't make it past the second step + s.newTestStep(ctx, "Step3", 25, "", "*=10"), // Another 25% fail at the third step }, ) require.NoError(s.T(), err) // Every target mush have started and finished the first step. numFinished := 0 for _, tgt := range targets { - s1n := "Step 1" + s1n := "Step1" require.Equal(s.T(), fmt.Sprintf(` -{[1 1 SimpleTest 0 Step 1][Target{ID: "%s"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "%s"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "%s"} TestFinishedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "%s"} TargetOut]} +{[1 1 SimpleTest 0 Step1][Target{ID: "%s"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "%s"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "%s"} TestFinishedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "%s"} TargetOut]} `, tgt.ID, tgt.ID, tgt.ID, tgt.ID), common.GetTestEventsAsString(ctx, s.MemoryStorage.Storage, testName, &tgt.ID, &s1n)) - s3n := "Step 3" + s3n := "Step3" if strings.Contains(common.GetTestEventsAsString(ctx, s.MemoryStorage.Storage, testName, &tgt.ID, &s3n), "TestFinishedEvent") { numFinished++ } @@ -592,10 +592,10 @@ func (s *TestRunnerSuite) TestPauseResumeSimple() { var resumeState []byte targets := []*target.Target{tgt("T1"), tgt("T2"), tgt("T3")} steps := []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 0, "T1", ""), + s.newTestStep(ctx, "Step1", 0, "T1", ""), // T2 and T3 will be paused here, the step will be given time to finish. - s.newTestStep(ctx, "Step 2", 0, "", "T2=200,T3=200"), - s.newTestStep(ctx, "Step 3", 0, "", ""), + s.newTestStep(ctx, "Step2", 0, "", "T2=200,T3=200"), + s.newTestStep(ctx, "Step3", 0, "", ""), } { tr1 := newTestRunner() @@ -645,9 +645,9 @@ func (s *TestRunnerSuite) TestPauseResumeSimple() { // Don't use the same pointers ot make sure there is no reliance on that. []*target.Target{tgt("T1"), tgt("T2"), tgt("T3")}, []test.TestStepBundle{ - s.newTestStep(ctx, "Step 1", 0, "T1", ""), - s.newTestStep(ctx, "Step 2", 0, "", "T2=200,T3=200"), - s.newTestStep(ctx, "Step 3", 0, "", ""), + s.newTestStep(ctx, "Step1", 0, "T1", ""), + s.newTestStep(ctx, "Step2", 0, "", "T2=200,T3=200"), + s.newTestStep(ctx, "Step3", 0, "", ""), }, ) require.NoError(s.T(), err) @@ -656,38 +656,38 @@ func (s *TestRunnerSuite) TestPauseResumeSimple() { // Steps 1 and 2 are executed entirely within the first runner instance // and never started in the second. require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 1][(*Target)(nil) TestStepFinishedEvent]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 1")) +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step1][(*Target)(nil) TestStepFinishedEvent]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step1")) require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 2][(*Target)(nil) TestStepRunningEvent]} -{[1 1 SimpleTest 0 Step 2][(*Target)(nil) TestStepFinishedEvent]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 2")) +{[1 1 SimpleTest 0 Step2][(*Target)(nil) TestStepRunningEvent]} +{[1 1 SimpleTest 0 Step2][(*Target)(nil) TestStepFinishedEvent]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step2")) // Step 3 did not get to start in the first instance and ran in the second. require.Equal(s.T(), ` -{[1 5 SimpleTest 0 Step 3][(*Target)(nil) TestStepRunningEvent]} -{[1 5 SimpleTest 0 Step 3][(*Target)(nil) TestStepFinishedEvent]} -`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step 3")) +{[1 5 SimpleTest 0 Step3][(*Target)(nil) TestStepRunningEvent]} +{[1 5 SimpleTest 0 Step3][(*Target)(nil) TestStepFinishedEvent]} +`, s.MemoryStorage.GetStepEvents(ctx, testName, "Step3")) // T1 failed entirely within the first run. require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TestFailedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TestFailedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T1"} TargetErr &"{\"Error\":\"target failed\"}"]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T1")) // T2 and T3 ran in both. require.Equal(s.T(), ` -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TargetIn]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TestFinishedEvent]} -{[1 1 SimpleTest 0 Step 1][Target{ID: "T2"} TargetOut]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TargetIn]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TestStartedEvent]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TestFinishedEvent]} -{[1 1 SimpleTest 0 Step 2][Target{ID: "T2"} TargetOut]} -{[1 5 SimpleTest 0 Step 3][Target{ID: "T2"} TargetIn]} -{[1 5 SimpleTest 0 Step 3][Target{ID: "T2"} TestStartedEvent]} -{[1 5 SimpleTest 0 Step 3][Target{ID: "T2"} TestFinishedEvent]} -{[1 5 SimpleTest 0 Step 3][Target{ID: "T2"} TargetOut]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TargetIn]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TestFinishedEvent]} +{[1 1 SimpleTest 0 Step1][Target{ID: "T2"} TargetOut]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TargetIn]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TestStartedEvent]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TestFinishedEvent]} +{[1 1 SimpleTest 0 Step2][Target{ID: "T2"} TargetOut]} +{[1 5 SimpleTest 0 Step3][Target{ID: "T2"} TargetIn]} +{[1 5 SimpleTest 0 Step3][Target{ID: "T2"} TestStartedEvent]} +{[1 5 SimpleTest 0 Step3][Target{ID: "T2"} TestFinishedEvent]} +{[1 5 SimpleTest 0 Step3][Target{ID: "T2"} TargetOut]} `, s.MemoryStorage.GetTargetEvents(ctx, testName, "T2")) } diff --git a/pkg/runner/test_steps_variables.go b/pkg/runner/test_steps_variables.go new file mode 100644 index 00000000..31e7049a --- /dev/null +++ b/pkg/runner/test_steps_variables.go @@ -0,0 +1,156 @@ +package runner + +import ( + "encoding/json" + "fmt" + "sync" + + "github.com/linuxboot/contest/pkg/test" +) + +type testStepsVariables struct { + existingSteps map[string]struct{} + + lock sync.Mutex + stepsVariablesByTarget map[string]map[string]stepVariables +} + +func newTestStepsVariables(bundles []test.TestStepBundle) (*testStepsVariables, error) { + result := &testStepsVariables{ + existingSteps: make(map[string]struct{}), + stepsVariablesByTarget: make(map[string]map[string]stepVariables), + } + for _, bs := range bundles { + if len(bs.TestStepLabel) == 0 { + continue + } + if _, found := result.existingSteps[bs.TestStepLabel]; found { + return nil, fmt.Errorf("duplication of test step label: '%s'", bs.TestStepLabel) + } + result.existingSteps[bs.TestStepLabel] = struct{}{} + } + return result, nil +} + +func (tsv *testStepsVariables) initTargetStepsVariables(tgtID string, stepsVars map[string]stepVariables) error { + if _, found := tsv.stepsVariablesByTarget[tgtID]; found { + return fmt.Errorf("duplication of target ID: '%s'", tgtID) + } + if stepsVars == nil { + tsv.stepsVariablesByTarget[tgtID] = make(map[string]stepVariables) + } else { + tsv.stepsVariablesByTarget[tgtID] = stepsVars + } + return nil +} + +func (tsv *testStepsVariables) getTargetStepsVariables(tgtID string) (map[string]stepVariables, error) { + tsv.lock.Lock() + defer tsv.lock.Unlock() + + vars, found := tsv.stepsVariablesByTarget[tgtID] + if !found { + return nil, fmt.Errorf("target ID: '%s' is not found", tgtID) + } + + // make a copy in case of buggy steps that can still be using it + result := make(map[string]stepVariables) + for name, value := range vars { + result[name] = value + } + return result, nil +} + +func (tsv *testStepsVariables) Add(tgtID string, stepLabel string, name string, value json.RawMessage) error { + if err := tsv.checkInput(tgtID, stepLabel, name); err != nil { + return err + } + + // tsv.stepsVariablesByTarget is a readonly map, though its values may change + targetSteps := tsv.stepsVariablesByTarget[tgtID] + tsv.lock.Lock() + defer tsv.lock.Unlock() + + stepVars, found := targetSteps[stepLabel] + if !found { + stepVars = make(stepVariables) + targetSteps[stepLabel] = stepVars + } + stepVars[name] = value + return nil +} + +func (tsv *testStepsVariables) Get(tgtID string, stepLabel string, name string) (json.RawMessage, error) { + if err := tsv.checkInput(tgtID, stepLabel, name); err != nil { + return nil, err + } + + targetSteps := tsv.stepsVariablesByTarget[tgtID] + tsv.lock.Lock() + defer tsv.lock.Unlock() + + stepVars, found := targetSteps[stepLabel] + if !found { + return nil, fmt.Errorf("step '%s' didn't emit any variables yet", stepLabel) + } + value, found := stepVars[name] + if !found { + return nil, fmt.Errorf("step '%s' didn't emit variable '%s' yet for target '%s'", stepLabel, name, tgtID) + } + return value, nil +} + +func (tsv *testStepsVariables) checkInput(tgtID string, stepLabel string, name string) error { + if len(stepLabel) == 0 { + return fmt.Errorf("empty step name") + } + if len(tgtID) == 0 { + return fmt.Errorf("empty target ID") + } + if len(name) == 0 { + return fmt.Errorf("empty variable name") + } + return nil +} + +type stepVariablesAccessor struct { + stepLabel string + varsMapping test.StepVariablesMapping + tsv *testStepsVariables +} + +func newStepVariablesAccessor(stepLabel string, varsMapping test.StepVariablesMapping, tsv *testStepsVariables) *stepVariablesAccessor { + return &stepVariablesAccessor{ + stepLabel: stepLabel, + varsMapping: varsMapping, + tsv: tsv, + } +} + +func (sva *stepVariablesAccessor) Add(tgtID string, name string, in interface{}) error { + if len(sva.stepLabel) == 0 { + return nil + } + b, err := json.Marshal(in) + if err != nil { + return fmt.Errorf("failed to serialize variable: %v", in) + } + return sva.tsv.Add(tgtID, sva.stepLabel, name, b) +} + +func (sva *stepVariablesAccessor) Get(tgtID string, mappedName string, out interface{}) error { + if sva.varsMapping == nil { + return fmt.Errorf("step doesn't have variables mapping") + } + stepVar, found := sva.varsMapping[mappedName] + if !found { + return fmt.Errorf("no mapping for variable '%s'", mappedName) + } + b, err := sva.tsv.Get(tgtID, stepVar.StepName, stepVar.VariableName) + if err != nil { + return err + } + return json.Unmarshal(b, out) +} + +var _ test.StepsVariables = (*stepVariablesAccessor)(nil) diff --git a/pkg/test/step.go b/pkg/test/step.go index e48d4cbf..3e9822ee 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -14,6 +14,7 @@ import ( "github.com/linuxboot/contest/pkg/target" "github.com/linuxboot/contest/pkg/xcontext" "strconv" + "unicode" ) // StepVariable is a pair that indicates a variable name produces by a step @@ -119,8 +120,11 @@ type TestStepChannels struct { // StepsVariables represents a read/write access for step variables type StepsVariables interface { - Add(tgtID string, name string, v interface{}) error - Get(tgtID string, name string, out interface{}) error + // Add adds a new or replaces existing variable associated with current test step and target + Add(tgtID string, name string, in interface{}) error + + // Get obtains existing variable by a mappedName which should be specified in variables mapping + Get(tgtID string, mappedName string, out interface{}) error } // TestStep is the interface that all steps need to implement to be executed @@ -136,3 +140,22 @@ type TestStep interface { // them to Run. ValidateParameters(ctx xcontext.Context, params TestStepParameters) error } + +// CheckVariableName checks that input argument forms a valid variable name +func CheckVariableName(s string) error { + if len(s) == 0 { + return fmt.Errorf("empty variable name") + } + for idx, ch := range s { + if idx == 0 { + if !unicode.IsLetter(ch) { + return fmt.Errorf("first character should be alpha, got %c", ch) + } + } + if unicode.IsLetter(ch) || unicode.IsDigit(ch) || ch == '_' { + continue + } + return fmt.Errorf("got unxpected character: %c", ch) + } + return nil +} diff --git a/pkg/test/step_test.go b/pkg/test/step_test.go index 105898a3..c0b1fe31 100644 --- a/pkg/test/step_test.go +++ b/pkg/test/step_test.go @@ -47,3 +47,11 @@ func TestTestStepParametersUnmarshalNested(t *testing.T) { require.Equal(t, "bar", substruct.Val2) require.Equal(t, "baz", substruct.More_nesting["foobar"]) } + +func TestCheckVariableName(t *testing.T) { + require.NoError(t, CheckVariableName("Abc123_XYZ")) + require.Error(t, CheckVariableName("")) + require.Error(t, CheckVariableName("1AAA")) + require.Error(t, CheckVariableName("a b")) + require.Error(t, CheckVariableName("a()+b")) +} diff --git a/tests/integ/jobmanager/common.go b/tests/integ/jobmanager/common.go index d95a61ab..25dd62fa 100644 --- a/tests/integ/jobmanager/common.go +++ b/tests/integ/jobmanager/common.go @@ -554,16 +554,16 @@ func (suite *TestJobManagerSuite) testPauseAndResume( // Verify emitted events. Despite pausing ad different stages this should look perfectly normal. require.Equal(suite.T(), strings.Replace(` {[JOBID 1 IntegrationTest: resume 0 ][Target{ID: "id1"} TargetAcquired]} -{[JOBID 1 IntegrationTest: resume 0 Step 1][Target{ID: "id1"} TargetIn]} -{[JOBID 1 IntegrationTest: resume 0 Step 1][Target{ID: "id1"} TargetOut]} -{[JOBID 1 IntegrationTest: resume 0 Step 2][Target{ID: "id1"} TargetIn]} -{[JOBID 1 IntegrationTest: resume 0 Step 2][Target{ID: "id1"} TargetOut]} +{[JOBID 1 IntegrationTest: resume 0 Step1][Target{ID: "id1"} TargetIn]} +{[JOBID 1 IntegrationTest: resume 0 Step1][Target{ID: "id1"} TargetOut]} +{[JOBID 1 IntegrationTest: resume 0 Step2][Target{ID: "id1"} TargetIn]} +{[JOBID 1 IntegrationTest: resume 0 Step2][Target{ID: "id1"} TargetOut]} {[JOBID 1 IntegrationTest: resume 0 ][Target{ID: "id1"} TargetReleased]} {[JOBID 2 IntegrationTest: resume 0 ][Target{ID: "id1"} TargetAcquired]} -{[JOBID 2 IntegrationTest: resume 0 Step 1][Target{ID: "id1"} TargetIn]} -{[JOBID 2 IntegrationTest: resume 0 Step 1][Target{ID: "id1"} TargetOut]} -{[JOBID 2 IntegrationTest: resume 0 Step 2][Target{ID: "id1"} TargetIn]} -{[JOBID 2 IntegrationTest: resume 0 Step 2][Target{ID: "id1"} TargetOut]} +{[JOBID 2 IntegrationTest: resume 0 Step1][Target{ID: "id1"} TargetIn]} +{[JOBID 2 IntegrationTest: resume 0 Step1][Target{ID: "id1"} TargetOut]} +{[JOBID 2 IntegrationTest: resume 0 Step2][Target{ID: "id1"} TargetIn]} +{[JOBID 2 IntegrationTest: resume 0 Step2][Target{ID: "id1"} TargetOut]} {[JOBID 2 IntegrationTest: resume 0 ][Target{ID: "id1"} TargetReleased]} `, "JOBID", fmt.Sprintf("%d", jobID), -1), suite.getTargetEvents("IntegrationTest: resume", "id1")) diff --git a/tests/integ/jobmanager/jobdescriptors.go b/tests/integ/jobmanager/jobdescriptors.go index fc17855e..84dc5d0c 100644 --- a/tests/integ/jobmanager/jobdescriptors.go +++ b/tests/integ/jobmanager/jobdescriptors.go @@ -289,15 +289,15 @@ var jobDescriptorSlowEcho2 = descriptorMust2(jobDescriptorTemplate, &templateDat "Steps": [ { "name": "slowecho", - "label": "Step 1", + "label": "Step1", "parameters": { "sleep": ["0.5"], - "text": ["Hello step 1"] + "text": ["Hello step1"] } }, { "name": "slowecho", - "label": "Step 2", + "label": "Step2", "parameters": { "sleep": ["0"], "text": ["Hello step 2"] From 75fd705bf45435a9ee9bfa2f65de320b5fc5bb37 Mon Sep 17 00:00:00 2001 From: Ilya Date: Wed, 29 Jun 2022 12:22:29 +0100 Subject: [PATCH 3/7] Remove using StepVariablesMapping Signed-off-by: Ilya --- pkg/jobmanager/bundles.go | 2 ++ pkg/pluginregistry/bundles.go | 40 +++------------------- pkg/runner/base_test_suite_test.go | 8 ++--- pkg/runner/job_runner_test.go | 14 ++++---- pkg/runner/step_runner_test.go | 27 +++++++-------- pkg/runner/test_runner_test.go | 54 +++++++++--------------------- pkg/runner/test_steps_variables.go | 23 ++++--------- pkg/storage/limits/limits.go | 18 +++++----- pkg/storage/limits/limits_test.go | 2 +- pkg/test/step.go | 20 +++-------- 10 files changed, 66 insertions(+), 142 deletions(-) diff --git a/pkg/jobmanager/bundles.go b/pkg/jobmanager/bundles.go index 73134380..ecb66232 100644 --- a/pkg/jobmanager/bundles.go +++ b/pkg/jobmanager/bundles.go @@ -60,6 +60,7 @@ func newBundlesFromSteps(ctx xcontext.Context, descriptors []*test.TestStepDescr // look up test step plugins in the plugin registry var stepBundles []test.TestStepBundle + for idx, descriptor := range descriptors { if descriptor == nil { return nil, fmt.Errorf("test step description is null") @@ -98,5 +99,6 @@ func newStepBundles(ctx xcontext.Context, descriptors test.TestStepsDescriptors, } labels[bundle.TestStepLabel] = true } + // TODO: verify that test variables refer to existing steps return testStepBundles, nil } diff --git a/pkg/pluginregistry/bundles.go b/pkg/pluginregistry/bundles.go index 1c2ba4d7..9457bd57 100644 --- a/pkg/pluginregistry/bundles.go +++ b/pkg/pluginregistry/bundles.go @@ -7,8 +7,6 @@ package pluginregistry import ( "fmt" - "strings" - "github.com/linuxboot/contest/pkg/job" "github.com/linuxboot/contest/pkg/target" "github.com/linuxboot/contest/pkg/test" @@ -36,41 +34,11 @@ func (r *PluginRegistry) NewTestStepBundle(ctx xcontext.Context, testStepDescrip return nil, InvalidVariableFormat{InvalidName: label, Err: err} } - var variablesMapping test.StepVariablesMapping - if testStepDescriptor.VariablesMapping != nil { - variablesMapping = make(test.StepVariablesMapping) - for internalName, mappedName := range testStepDescriptor.VariablesMapping { - if err := test.CheckVariableName(internalName); err != nil { - return nil, InvalidVariableFormat{InvalidName: internalName, Err: err} - } - if _, found := variablesMapping[internalName]; found { - return nil, fmt.Errorf("duplication of '%s' variable", internalName) - } - // NewStepVariable creates a StepVariable object from mapping: "stepName.VariableName" - parts := strings.Split(mappedName, ".") - if len(parts) != 2 { - return nil, fmt.Errorf("variable mapping '%s' should contain a single '.' separator", mappedName) - } - if err := test.CheckVariableName(parts[0]); err != nil { - return nil, InvalidVariableFormat{InvalidName: parts[0], Err: err} - } - if err := test.CheckVariableName(parts[1]); err != nil { - return nil, InvalidVariableFormat{InvalidName: parts[1], Err: err} - } - - variablesMapping[internalName] = test.StepVariable{ - StepName: parts[0], - VariableName: parts[1], - } - } - } - // TODO: check that all testStep labels from variable mappings exist testStepBundle := test.TestStepBundle{ - TestStep: testStep, - TestStepLabel: label, - Parameters: testStepDescriptor.Parameters, - AllowedEvents: allowedEvents, - VariablesMapping: variablesMapping, + TestStep: testStep, + TestStepLabel: label, + Parameters: testStepDescriptor.Parameters, + AllowedEvents: allowedEvents, } return &testStepBundle, nil } diff --git a/pkg/runner/base_test_suite_test.go b/pkg/runner/base_test_suite_test.go index 34c21fef..6f3d4edf 100644 --- a/pkg/runner/base_test_suite_test.go +++ b/pkg/runner/base_test_suite_test.go @@ -90,13 +90,11 @@ func (s *BaseTestSuite) NewStep( ctx xcontext.Context, label, name string, params test.TestStepParameters, - variablesMapping map[string]string, ) test.TestStepBundle { td := test.TestStepDescriptor{ - Name: name, - Label: label, - Parameters: params, - VariablesMapping: variablesMapping, + Name: name, + Label: label, + Parameters: params, } sb, err := s.PluginRegistry.NewTestStepBundle(ctx, td) require.NoError(s.T(), err) diff --git a/pkg/runner/job_runner_test.go b/pkg/runner/job_runner_test.go index b0db15ac..fa34658e 100644 --- a/pkg/runner/job_runner_test.go +++ b/pkg/runner/job_runner_test.go @@ -92,7 +92,7 @@ func (s *JobRunnerSuite) TestSimpleJobStartFinish() { TargetManager: targetlist.New(), }, TestStepsBundles: []test.TestStepBundle{ - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), }, }, }, @@ -177,11 +177,11 @@ func (s *JobRunnerSuite) TestJobWithTestRetry() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }, nil), - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + }), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), s.NewStep(ctx, "echo2_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("world")}, - }, nil), + }), }, }, }, @@ -282,7 +282,7 @@ func (s *JobRunnerSuite) TestJobRetryOnFailedAcquire() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }, nil), + }), }, }, }, @@ -361,7 +361,7 @@ func (s *JobRunnerSuite) TestAcquireFailed() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }, nil), + }), }, }, }, @@ -431,7 +431,7 @@ func (s *JobRunnerSuite) TestResumeStateBadJobId() { TestStepsBundles: []test.TestStepBundle{ s.NewStep(ctx, "echo1_step_label", echo.Name, map[string][]test.Param{ "text": {*test.NewParam("hello")}, - }, nil), + }), }, }, }, diff --git a/pkg/runner/step_runner_test.go b/pkg/runner/step_runner_test.go index 28dfce4f..788e0846 100644 --- a/pkg/runner/step_runner_test.go +++ b/pkg/runner/step_runner_test.go @@ -85,7 +85,7 @@ func (s *StepRunnerSuite) TestRunningStep() { inputResumeState := json.RawMessage("{\"some_input\": 42}") addTarget, resumedTargetsResults, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, inputResumeState, @@ -151,7 +151,7 @@ func (s *StepRunnerSuite) TestAddSameTargetSequentiallyTimes() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, nil, @@ -204,7 +204,7 @@ func (s *StepRunnerSuite) TestAddTargetReturnsErrorIfFailsToInput() { defer stepRunner.Stop() addTarget, resumedTargetsResults, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, nil, @@ -257,7 +257,7 @@ func (s *StepRunnerSuite) TestStepPanics() { defer stepRunner.Stop() addTarget, resumedTargetsResults, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), NewTestStepEventsEmitterFactory( s.MemoryStorage.StorageEngineVault, @@ -316,7 +316,7 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, nil, @@ -340,7 +340,7 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, nil, @@ -351,7 +351,7 @@ func (s *StepRunnerSuite) TestCornerCases() { require.NotNil(s.T(), runResult) addTarget2, _, runResult2, err2 := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, nil, @@ -368,7 +368,7 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, nil, @@ -389,9 +389,8 @@ func (s *StepRunnerSuite) TestCornerCases() { defer stepRunner.Stop() stepRunner.Stop() - addTarget, _, runResult, err := stepRunner.Run(ctx, - s.NewStep(ctx, "test_step_label", stateFullStepName, nil, nil), + s.NewStep(ctx, "test_step_label", stateFullStepName, nil), newStepsVariablesMock(nil, nil), emitter, nil, @@ -406,20 +405,20 @@ func (s *StepRunnerSuite) TestCornerCases() { type stepsVariablesMock struct { add func(tgtID string, name string, value interface{}) error - get func(tgtID string, name string, value interface{}) error + get func(tgtID string, stepLabel, name string, value interface{}) error } func (sm *stepsVariablesMock) Add(tgtID string, name string, value interface{}) error { return sm.add(tgtID, name, value) } -func (sm *stepsVariablesMock) Get(tgtID string, name string, value interface{}) error { - return sm.get(tgtID, name, value) +func (sm *stepsVariablesMock) Get(tgtID string, stepLabel, name string, value interface{}) error { + return sm.get(tgtID, stepLabel, name, value) } func newStepsVariablesMock( add func(tgtID string, name string, value interface{}) error, - get func(tgtID string, name string, value interface{}) error, + get func(tgtID string, stepLabel, name string, value interface{}) error, ) *stepsVariablesMock { return &stepsVariablesMock{ add: add, diff --git a/pkg/runner/test_runner_test.go b/pkg/runner/test_runner_test.go index b6b645ad..7584b0dd 100644 --- a/pkg/runner/test_runner_test.go +++ b/pkg/runner/test_runner_test.go @@ -103,7 +103,7 @@ func (s *TestRunnerSuite) newTestStep(ctx xcontext.Context, label string, failPc teststep.FailPctParam: []test.Param{*test.NewParam(fmt.Sprintf("%d", failPct))}, teststep.FailTargetsParam: []test.Param{*test.NewParam(failTargets)}, teststep.DelayTargetsParam: []test.Param{*test.NewParam(delayTargets)}, - }, nil) + }) } func (s *TestRunnerSuite) runWithTimeout(ctx xcontext.Context, @@ -306,7 +306,7 @@ func (s *TestRunnerSuite) TestNoReturnStepWithCorrectTargetForwarding() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step1", noreturn.Name, nil, nil), + s.NewStep(ctx, "Step1", noreturn.Name, nil), }, ) require.Error(s.T(), err) @@ -323,7 +323,7 @@ func (s *TestRunnerSuite) TestStepPanics() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step1", panicstep.Name, nil, nil), + s.NewStep(ctx, "Step1", panicstep.Name, nil), }, ) require.Error(s.T(), err) @@ -341,7 +341,7 @@ func (s *TestRunnerSuite) TestStepClosesChannels() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("T1")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step1", channels.Name, nil, nil), + s.NewStep(ctx, "Step1", channels.Name, nil), }, ) require.Error(s.T(), err) @@ -364,7 +364,7 @@ func (s *TestRunnerSuite) TestStepYieldsResultForNonexistentTarget() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("TExtra")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step1", badtargets.Name, nil, nil), + s.NewStep(ctx, "Step1", badtargets.Name, nil), }, ) require.Error(s.T(), err) @@ -386,7 +386,7 @@ func (s *TestRunnerSuite) TestStepYieldsDuplicateResult() { []test.TestStepBundle{ // TGood makes it past here unscathed and gets delayed in Step 2, // TDup also emerges fine at first but is then returned again, and that's bad. - s.NewStep(ctx, "Step1", badtargets.Name, nil, nil), + s.NewStep(ctx, "Step1", badtargets.Name, nil), s.newTestStep(ctx, "Step2", 0, "", "TGood=100"), }, ) @@ -403,7 +403,7 @@ func (s *TestRunnerSuite) TestStepLosesTargets() { _, _, err := s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, []*target.Target{tgt("TGood"), tgt("TDrop")}, []test.TestStepBundle{ - s.NewStep(ctx, "Step1", badtargets.Name, nil, nil), + s.NewStep(ctx, "Step1", badtargets.Name, nil), }, ) require.Error(s.T(), err) @@ -424,7 +424,7 @@ func (s *TestRunnerSuite) TestStepYieldsResultForUnexpectedTarget() { // TExtra2 fails here. s.newTestStep(ctx, "Step1", 0, "TExtra2", ""), // Yet, a result for it is returned here, which we did not expect. - s.NewStep(ctx, "Step2", badtargets.Name, nil, nil), + s.NewStep(ctx, "Step2", badtargets.Name, nil), }, ) require.Error(s.T(), err) @@ -490,21 +490,10 @@ func (s *TestRunnerSuite) TestVariables() { func(ctx xcontext.Context, target *teststeps.TargetWithData) error { require.NoError(s.T(), stepsVars.Add(target.Target.ID, "target_id", target.Target.ID)) - // TODO: we are having a race-condition in event appearance between TargetIn generated by StepRunner - // and events generated by the step. Will address in another PR - time.Sleep(10 * time.Millisecond) - var resultValue string - err := stepsVars.Get(target.Target.ID, "input", &resultValue) - // only second step will get a variable - if err == nil { - payLoad := json.RawMessage(resultValue) - require.NoError(s.T(), ev.Emit(ctx, testevent.Data{ - Target: target.Target, - EventName: "dummy", - Payload: &payLoad, - })) - } + err := stepsVars.Get(target.Target.ID, "step1", "target_id", &resultValue) + require.NoError(s.T(), err) + require.Equal(s.T(), "T1", resultValue) return func() error { mu.Lock() @@ -542,10 +531,8 @@ func (s *TestRunnerSuite) TestVariables() { resumeState, _, err = s.runWithTimeout(ctx1, tr, nil, 1, 2*time.Second, targets, []test.TestStepBundle{ - s.NewStep(ctx, "step1", stateFullStepName, nil, nil), - s.NewStep(ctx, "step2", stateFullStepName, nil, map[string]string{ - "input": "step1.target_id", - }), + s.NewStep(ctx, "step1", stateFullStepName, nil), + s.NewStep(ctx, "step2", stateFullStepName, nil), }, ) require.IsType(s.T(), xcontext.ErrPaused, err) @@ -562,24 +549,13 @@ func (s *TestRunnerSuite) TestVariables() { resumeState, _, err = s.runWithTimeout(ctx, tr, nil, 1, 2*time.Second, targets, []test.TestStepBundle{ - s.NewStep(ctx, "step1", stateFullStepName, nil, nil), - s.NewStep(ctx, "step2", stateFullStepName, nil, map[string]string{ - "input": "step1.target_id", - }), + s.NewStep(ctx, "step1", stateFullStepName, nil), + s.NewStep(ctx, "step2", stateFullStepName, nil), }, ) require.NoError(s.T(), err) require.Empty(s.T(), resumeState) } - - require.Equal(s.T(), ` -{[1 1 SimpleTest 0 step1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 step1][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 step1][Target{ID: "T1"} TargetOut]} -{[1 1 SimpleTest 0 step2][Target{ID: "T1"} TargetIn]} -{[1 1 SimpleTest 0 step2][Target{ID: "T1"} dummy &"T1"]} -{[1 1 SimpleTest 0 step2][Target{ID: "T1"} TargetOut]} -`, s.MemoryStorage.GetTestEvents(ctx, testName)) } // Test pausing/resuming a naive step that does not cooperate. diff --git a/pkg/runner/test_steps_variables.go b/pkg/runner/test_steps_variables.go index 31e7049a..7836c6e6 100644 --- a/pkg/runner/test_steps_variables.go +++ b/pkg/runner/test_steps_variables.go @@ -114,16 +114,14 @@ func (tsv *testStepsVariables) checkInput(tgtID string, stepLabel string, name s } type stepVariablesAccessor struct { - stepLabel string - varsMapping test.StepVariablesMapping - tsv *testStepsVariables + stepLabel string + tsv *testStepsVariables } -func newStepVariablesAccessor(stepLabel string, varsMapping test.StepVariablesMapping, tsv *testStepsVariables) *stepVariablesAccessor { +func newStepVariablesAccessor(stepLabel string, tsv *testStepsVariables) *stepVariablesAccessor { return &stepVariablesAccessor{ - stepLabel: stepLabel, - varsMapping: varsMapping, - tsv: tsv, + stepLabel: stepLabel, + tsv: tsv, } } @@ -138,15 +136,8 @@ func (sva *stepVariablesAccessor) Add(tgtID string, name string, in interface{}) return sva.tsv.Add(tgtID, sva.stepLabel, name, b) } -func (sva *stepVariablesAccessor) Get(tgtID string, mappedName string, out interface{}) error { - if sva.varsMapping == nil { - return fmt.Errorf("step doesn't have variables mapping") - } - stepVar, found := sva.varsMapping[mappedName] - if !found { - return fmt.Errorf("no mapping for variable '%s'", mappedName) - } - b, err := sva.tsv.Get(tgtID, stepVar.StepName, stepVar.VariableName) +func (sva *stepVariablesAccessor) Get(tgtID string, stepLabel, name string, out interface{}) error { + b, err := sva.tsv.Get(tgtID, stepLabel, name) if err != nil { return err } diff --git a/pkg/storage/limits/limits.go b/pkg/storage/limits/limits.go index 847d537b..cafeeeae 100644 --- a/pkg/storage/limits/limits.go +++ b/pkg/storage/limits/limits.go @@ -37,15 +37,15 @@ const MaxTestNameLen = 32 // ValidateTestName retruns error if the test name does not match storage limitations func (v *Validator) ValidateTestName(testName string) error { - return v.validate(testName, "Test name", false, MaxTestNameLen) + return v.validate(testName, "Test name", MaxTestNameLen) } // MaxTestStepLabelLen is a max length of test step label field const MaxTestStepLabelLen = 32 // ValidateTestStepLabel returns error if the test step label does not match storage limitations -func (v *Validator) ValidateTestStepLabel(testStepName string) error { - return v.validate(testStepName, "Test step label", false, MaxTestStepLabelLen) +func (v *Validator) ValidateTestStepLabel(testStepLabel string) error { + return v.validate(testStepLabel, "Test step label", MaxTestStepLabelLen) } // MaxJobNameLen is a max length of job name field @@ -53,7 +53,7 @@ const MaxJobNameLen = 64 // ValidateJobName retruns error if the job name does not match storage limitations func (v *Validator) ValidateJobName(jobName string) error { - return v.validate(jobName, "Job name", true, MaxJobNameLen) + return v.validate(jobName, "Job name", MaxJobNameLen) } // MaxEventNameLen is a max length of event name field @@ -61,7 +61,7 @@ const MaxEventNameLen = 32 // ValidateEventName retruns error if the event name does not match storage limitations func (v *Validator) ValidateEventName(eventName string) error { - return v.validate(eventName, "Event name", true, MaxEventNameLen) + return v.validate(eventName, "Event name", MaxEventNameLen) } // MaxReporterNameLen is a max length of reporter name field @@ -69,7 +69,7 @@ const MaxReporterNameLen = 32 // ValidateReporterName retruns error if the reporter name does not match storage limitations func (v *Validator) ValidateReporterName(reporterName string) error { - return v.validate(reporterName, "Reporter name", true, MaxReporterNameLen) + return v.validate(reporterName, "Reporter name", MaxReporterNameLen) } // MaxRequestorNameLen is a max length of Requestor name field @@ -77,7 +77,7 @@ const MaxRequestorNameLen = 32 // ValidateRequestorName retruns error if the requestor name does not match storage limitations func (v *Validator) ValidateRequestorName(requestorName string) error { - return v.validate(requestorName, "Requestor name", true, MaxRequestorNameLen) + return v.validate(requestorName, "Requestor name", MaxRequestorNameLen) } // MaxServerIDLen is a max length of server id field @@ -85,10 +85,10 @@ const MaxServerIDLen = 64 // ValidateServerID retruns error if the ServerID does not match storage limitations func (v *Validator) ValidateServerID(serverID string) error { - return v.validate(serverID, "Server ID", true, MaxServerIDLen) + return v.validate(serverID, "Server ID", MaxServerIDLen) } -func (v *Validator) validate(data string, dataName string, allowEmpty bool, maxDataLen int) error { +func (v *Validator) validate(data string, dataName string, maxDataLen int) error { if l := len(data); l > maxDataLen { return ErrParameterIsTooLong{DataName: dataName, MaxLen: maxDataLen, ActualLen: l} } diff --git a/pkg/storage/limits/limits_test.go b/pkg/storage/limits/limits_test.go index e186de12..9183e9dd 100644 --- a/pkg/storage/limits/limits_test.go +++ b/pkg/storage/limits/limits_test.go @@ -134,7 +134,7 @@ func TestTestStepLabel(t *testing.T) { require.NoError(t, err) testFetchParams, err := json.Marshal(&literal.FetchParameters{ - TestName: "AAA", + TestName: "AA", Steps: []*test.TestStepDescriptor{{ Name: echo.Name, Parameters: test.TestStepParameters{ diff --git a/pkg/test/step.go b/pkg/test/step.go index 3e9822ee..d4e20cae 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -17,15 +17,6 @@ import ( "unicode" ) -// StepVariable is a pair that indicates a variable name produces by a step -type StepVariable struct { - StepName string - VariableName string -} - -// StepVariablesMapping is a mapping between produced and consumed variables -type StepVariablesMapping map[string]StepVariable - // TestStepParameters represents the parameters that a TestStep should consume // according to the Test descriptor fetched by the TestFetcher type TestStepParameters map[string][]Param @@ -96,11 +87,10 @@ type TestStepDescriptor struct { // TestStepBundle bundles the selected TestStep together with its parameters as // specified in the Test descriptor fetched by the TestFetcher type TestStepBundle struct { - TestStep TestStep - TestStepLabel string - Parameters TestStepParameters - VariablesMapping StepVariablesMapping - AllowedEvents map[event.Name]bool + TestStep TestStep + TestStepLabel string + Parameters TestStepParameters + AllowedEvents map[event.Name]bool } // TestStepResult is used by TestSteps to report result for a particular target. @@ -124,7 +114,7 @@ type StepsVariables interface { Add(tgtID string, name string, in interface{}) error // Get obtains existing variable by a mappedName which should be specified in variables mapping - Get(tgtID string, mappedName string, out interface{}) error + Get(tgtID string, stepLabel, name string, out interface{}) error } // TestStep is the interface that all steps need to implement to be executed From 1b4a39b29533bd96bca200942c243963853bc2c1 Mon Sep 17 00:00:00 2001 From: Ilya Date: Fri, 8 Jul 2022 21:47:44 +0100 Subject: [PATCH 4/7] Code-style improvements Signed-off-by: Ilya --- pkg/pluginregistry/bundles.go | 4 ++-- pkg/pluginregistry/errors.go | 8 +++---- pkg/runner/test_runner.go | 8 +++---- pkg/runner/test_runner_test.go | 2 +- pkg/test/step.go | 42 ++++++++++++++++++++++------------ pkg/test/step_test.go | 10 ++++---- 6 files changed, 43 insertions(+), 31 deletions(-) diff --git a/pkg/pluginregistry/bundles.go b/pkg/pluginregistry/bundles.go index 9457bd57..6c81cd66 100644 --- a/pkg/pluginregistry/bundles.go +++ b/pkg/pluginregistry/bundles.go @@ -30,8 +30,8 @@ func (r *PluginRegistry) NewTestStepBundle(ctx xcontext.Context, testStepDescrip if len(label) == 0 { return nil, ErrStepLabelIsMandatory{TestStepDescriptor: testStepDescriptor} } - if err := test.CheckVariableName(label); err != nil { - return nil, InvalidVariableFormat{InvalidName: label, Err: err} + if err := test.CheckIdentifier(label); err != nil { + return nil, ErrInvalidStepLabelFormat{InvalidName: label, Err: err} } testStepBundle := test.TestStepBundle{ diff --git a/pkg/pluginregistry/errors.go b/pkg/pluginregistry/errors.go index bc71b181..e169e593 100644 --- a/pkg/pluginregistry/errors.go +++ b/pkg/pluginregistry/errors.go @@ -19,16 +19,16 @@ func (err ErrStepLabelIsMandatory) Error() string { return fmt.Sprintf("step has no label, but it is mandatory (step: %+v)", err.TestStepDescriptor) } -// InvalidVariableFormat tells that a variable name doesn't fit the variable name format (alphanum + '_') -type InvalidVariableFormat struct { +// ErrInvalidStepLabelFormat tells that a variable name doesn't fit the variable name format (alphanum + '_') +type ErrInvalidStepLabelFormat struct { InvalidName string Err error } -func (err InvalidVariableFormat) Error() string { +func (err ErrInvalidStepLabelFormat) Error() string { return fmt.Sprintf("'%s' doesn't match variable name format: %v", err.InvalidName, err.Err) } -func (err InvalidVariableFormat) Unwrap() error { +func (err ErrInvalidStepLabelFormat) Unwrap() error { return err.Err } diff --git a/pkg/runner/test_runner.go b/pkg/runner/test_runner.go index 0562a7a8..b9f5a12d 100644 --- a/pkg/runner/test_runner.go +++ b/pkg/runner/test_runner.go @@ -43,7 +43,7 @@ type TestRunner struct { steps []*stepState // The pipeline, in order of execution targets map[string]*targetState // Target state lookup map - stepsVariables *testStepsVariables + stepOutputs *testStepsVariables // contains emitted steps variables // One mutex to rule them all, used to serialize access to all the state above. // Could probably be split into several if necessary. @@ -145,14 +145,14 @@ func (tr *TestRunner) Run( } var err error - tr.stepsVariables, err = newTestStepsVariables(t.TestStepsBundles) + tr.stepOutputs, err = newTestStepsVariables(t.TestStepsBundles) if err != nil { ctx.Errorf("Failed to initialise test steps variables: %v", err) return nil, nil, err } for targetID, targetState := range tr.targets { - if err := tr.stepsVariables.initTargetStepsVariables(targetID, targetState.StepsVariables); err != nil { + if err := tr.stepOutputs.initTargetStepsVariables(targetID, targetState.StepsVariables); err != nil { ctx.Errorf("Failed to initialise test steps variables for target: %s: %v", targetID, err) return nil, nil, err } @@ -221,7 +221,7 @@ func (tr *TestRunner) Run( numInFlightTargets := 0 for i, tgt := range targets { tgs := tr.targets[tgt.ID] - tgs.StepsVariables, err = tr.stepsVariables.getTargetStepsVariables(tgt.ID) + tgs.StepsVariables, err = tr.stepOutputs.getTargetStepsVariables(tgt.ID) if err != nil { ctx.Errorf("Failed to get steps variables: %v", err) return nil, nil, err diff --git a/pkg/runner/test_runner_test.go b/pkg/runner/test_runner_test.go index 7584b0dd..77721047 100644 --- a/pkg/runner/test_runner_test.go +++ b/pkg/runner/test_runner_test.go @@ -471,7 +471,7 @@ func (s *TestRunnerSuite) TestRandomizedMultiStep() { } func (s *TestRunnerSuite) TestVariables() { - ctx, cancel := xcontext.WithCancel(logrusctx.NewContext(logger.LevelDebug)) + ctx, cancel := logrusctx.NewContext(logger.LevelDebug) defer cancel() var ( diff --git a/pkg/test/step.go b/pkg/test/step.go index d4e20cae..c3456d3a 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -9,12 +9,13 @@ import ( "encoding/json" "errors" "fmt" + "regexp" + "strconv" + "github.com/linuxboot/contest/pkg/event" "github.com/linuxboot/contest/pkg/event/testevent" "github.com/linuxboot/contest/pkg/target" "github.com/linuxboot/contest/pkg/xcontext" - "strconv" - "unicode" ) // TestStepParameters represents the parameters that a TestStep should consume @@ -109,6 +110,14 @@ type TestStepChannels struct { } // StepsVariables represents a read/write access for step variables +// Example: +// var sv StepsVariables +// intVar := 42 +// sv.Add("dummy-target-id", "varname", intVar) +// +// var recvIntVar int +// sv.Get(("dummy-target-id", "varname", &recvIntVar) +// assert recvIntVar == 42 type StepsVariables interface { // Add adds a new or replaces existing variable associated with current test step and target Add(tgtID string, name string, in interface{}) error @@ -131,21 +140,24 @@ type TestStep interface { ValidateParameters(ctx xcontext.Context, params TestStepParameters) error } -// CheckVariableName checks that input argument forms a valid variable name -func CheckVariableName(s string) error { +var identifierRegexPattern *regexp.Regexp + +func init() { + var err error + identifierRegexPattern, err = regexp.Compile(`[a-zA-Z]\w*`) + if err != nil { + panic(fmt.Sprintf("invalid identifier matching regexp expression: %v", err)) + } +} + +// CheckIdentifier checks that input argument forms a valid identifier name +func CheckIdentifier(s string) error { if len(s) == 0 { - return fmt.Errorf("empty variable name") + return fmt.Errorf("empty identifier") } - for idx, ch := range s { - if idx == 0 { - if !unicode.IsLetter(ch) { - return fmt.Errorf("first character should be alpha, got %c", ch) - } - } - if unicode.IsLetter(ch) || unicode.IsDigit(ch) || ch == '_' { - continue - } - return fmt.Errorf("got unxpected character: %c", ch) + match := identifierRegexPattern.FindString(s) + if match != s { + return fmt.Errorf("identifier should be an non-empty string that matches: %s", identifierRegexPattern.String()) } return nil } diff --git a/pkg/test/step_test.go b/pkg/test/step_test.go index c0b1fe31..be546f72 100644 --- a/pkg/test/step_test.go +++ b/pkg/test/step_test.go @@ -49,9 +49,9 @@ func TestTestStepParametersUnmarshalNested(t *testing.T) { } func TestCheckVariableName(t *testing.T) { - require.NoError(t, CheckVariableName("Abc123_XYZ")) - require.Error(t, CheckVariableName("")) - require.Error(t, CheckVariableName("1AAA")) - require.Error(t, CheckVariableName("a b")) - require.Error(t, CheckVariableName("a()+b")) + require.NoError(t, CheckIdentifier("Abc123_XYZ")) + require.Error(t, CheckIdentifier("")) + require.Error(t, CheckIdentifier("1AAA")) + require.Error(t, CheckIdentifier("a b")) + require.Error(t, CheckIdentifier("a()+b")) } From 960c4b4892dfa32bb0ca78464ed8bdd55d347133 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 7 Aug 2022 16:37:23 +0100 Subject: [PATCH 5/7] Use test variables in StepState class Signed-off-by: Ilya --- pkg/runner/step_state.go | 8 +++++++- pkg/runner/test_runner.go | 11 ++++------- pkg/test/step.go | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/pkg/runner/step_state.go b/pkg/runner/step_state.go index caa21f27..fe76dcce 100644 --- a/pkg/runner/step_state.go +++ b/pkg/runner/step_state.go @@ -22,6 +22,7 @@ type stepState struct { sb test.TestStepBundle // The test bundle. ev testevent.Emitter + tsv *testStepsVariables stepRunner *StepRunner addTarget AddTargetToStep stopped chan struct{} @@ -37,6 +38,7 @@ func newStepState( stepIndex int, sb test.TestStepBundle, emitterFactory TestStepEventsEmitterFactory, + tsv *testStepsVariables, resumeState json.RawMessage, resumeStateTargets []target.Target, onError func(err error), @@ -45,6 +47,7 @@ func newStepState( stepIndex: stepIndex, sb: sb, ev: emitterFactory.New(sb.TestStepLabel), + tsv: tsv, stepRunner: NewStepRunner(), stopped: make(chan struct{}), resumeState: resumeState, @@ -113,7 +116,10 @@ func (ss *stepState) Run(ctx xcontext.Context) error { stepCtx = stepCtx.WithField("step_index", strconv.Itoa(ss.stepIndex)) stepCtx = stepCtx.WithField("step_label", ss.sb.TestStepLabel) - addTarget, resumeTargetsNotifiers, stepRunResult, err := ss.stepRunner.Run(stepCtx, ss.sb, ss.ev, ss.resumeState, ss.resumeStateTargets) + addTarget, resumeTargetsNotifiers, stepRunResult, err := ss.stepRunner.Run( + stepCtx, ss.sb, newStepVariablesAccessor(ss.sb.TestStepLabel, ss.tsv), ss.ev, ss.resumeState, + ss.resumeStateTargets, + ) if err != nil { return fmt.Errorf("failed to launch step runner: %v", err) } diff --git a/pkg/runner/test_runner.go b/pkg/runner/test_runner.go index b9f5a12d..b007a0d8 100644 --- a/pkg/runner/test_runner.go +++ b/pkg/runner/test_runner.go @@ -43,8 +43,6 @@ type TestRunner struct { steps []*stepState // The pipeline, in order of execution targets map[string]*targetState // Target state lookup map - stepOutputs *testStepsVariables // contains emitted steps variables - // One mutex to rule them all, used to serialize access to all the state above. // Could probably be split into several if necessary. mu sync.Mutex @@ -144,15 +142,14 @@ func (tr *TestRunner) Run( } } - var err error - tr.stepOutputs, err = newTestStepsVariables(t.TestStepsBundles) + stepOutputs, err := newTestStepsVariables(t.TestStepsBundles) if err != nil { ctx.Errorf("Failed to initialise test steps variables: %v", err) return nil, nil, err } for targetID, targetState := range tr.targets { - if err := tr.stepOutputs.initTargetStepsVariables(targetID, targetState.StepsVariables); err != nil { + if err := stepOutputs.initTargetStepsVariables(targetID, targetState.StepsVariables); err != nil { ctx.Errorf("Failed to initialise test steps variables for target: %s: %v", targetID, err) return nil, nil, err } @@ -174,7 +171,7 @@ func (tr *TestRunner) Run( } // Step handlers will be started from target handlers as targets reach them. - tr.steps = append(tr.steps, newStepState(i, sb, emitterFactory, srs, resumeStateTargets, func(err error) { + tr.steps = append(tr.steps, newStepState(i, sb, emitterFactory, stepOutputs, srs, resumeStateTargets, func(err error) { tr.monitorCond.Signal() })) } @@ -221,7 +218,7 @@ func (tr *TestRunner) Run( numInFlightTargets := 0 for i, tgt := range targets { tgs := tr.targets[tgt.ID] - tgs.StepsVariables, err = tr.stepOutputs.getTargetStepsVariables(tgt.ID) + tgs.StepsVariables, err = stepOutputs.getTargetStepsVariables(tgt.ID) if err != nil { ctx.Errorf("Failed to get steps variables: %v", err) return nil, nil, err diff --git a/pkg/test/step.go b/pkg/test/step.go index c3456d3a..730e6e39 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -156,7 +156,7 @@ func CheckIdentifier(s string) error { return fmt.Errorf("empty identifier") } match := identifierRegexPattern.FindString(s) - if match != s { + if len(match) != len(s) { return fmt.Errorf("identifier should be an non-empty string that matches: %s", identifierRegexPattern.String()) } return nil From 4ae036c14bc66eb40b62f76b0087bb297c142f30 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 7 Aug 2022 16:46:17 +0100 Subject: [PATCH 6/7] Add test variables in GatherCmd plugin Signed-off-by: Ilya --- plugins/teststeps/gathercmd/gathercmd.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plugins/teststeps/gathercmd/gathercmd.go b/plugins/teststeps/gathercmd/gathercmd.go index 5286d8a3..f447c794 100644 --- a/plugins/teststeps/gathercmd/gathercmd.go +++ b/plugins/teststeps/gathercmd/gathercmd.go @@ -126,7 +126,14 @@ func (ts *GatherCmd) returnTargets(ctx xcontext.Context, ch test.TestStepChannel } // Run executes the step -func (ts *GatherCmd) Run(ctx xcontext.Context, ch test.TestStepChannels, params test.TestStepParameters, emitter testevent.Emitter, resumeState json.RawMessage) (json.RawMessage, error) { +func (ts *GatherCmd) Run( + ctx xcontext.Context, + ch test.TestStepChannels, + emitter testevent.Emitter, + stepsVars test.StepsVariables, + params test.TestStepParameters, + resumeState json.RawMessage, +) (json.RawMessage, error) { log := ctx.Logger() if err := ts.setParams(params); err != nil { From 8196bfadf90d0441f22bf8148026d43223ce00f2 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 7 Aug 2022 17:00:47 +0100 Subject: [PATCH 7/7] Fix incorrect rebasing issue: use StepState.GetError() method instead of directly accessing internal variables Signed-off-by: Ilya --- pkg/runner/test_runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/runner/test_runner.go b/pkg/runner/test_runner.go index b007a0d8..86707c60 100644 --- a/pkg/runner/test_runner.go +++ b/pkg/runner/test_runner.go @@ -223,7 +223,7 @@ func (tr *TestRunner) Run( ctx.Errorf("Failed to get steps variables: %v", err) return nil, nil, err } - stepErr := tr.steps[tgs.CurStep].runErr + stepErr := tr.steps[tgs.CurStep].GetError() if tgs.CurPhase == targetStepPhaseRun { numInFlightTargets++ if stepErr != xcontext.ErrPaused {