Skip to content

Commit

Permalink
Remove using StepVariablesMapping
Browse files Browse the repository at this point in the history 
Signed-off-by: Ilya <[email protected]>
  • Loading branch information
@rihter007
rihter007 committed Jul 20, 2022
1 parent ecb1676 commit 51fa609
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 142 deletions.
2 changes: 2 additions & 0 deletions pkg/jobmanager/bundles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
40 changes: 4 additions & 36 deletions pkg/pluginregistry/bundles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 3 additions & 5 deletions pkg/runner/base_test_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 7 additions & 7 deletions pkg/runner/job_runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
},
Expand Down Expand Up @@ -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),
}),
},
},
},
Expand Down Expand Up @@ -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),
}),
},
},
},
Expand Down Expand Up @@ -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),
}),
},
},
},
Expand Down Expand Up @@ -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),
}),
},
},
},
Expand Down
26 changes: 13 additions & 13 deletions pkg/runner/step_runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (s *StepRunnerSuite) TestRunningStep() {

inputResumeState := json.RawMessage("{\"some_input\": 42}")
resultChan, addTarget, 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,
Expand Down Expand Up @@ -144,7 +144,7 @@ func (s *StepRunnerSuite) TestAddSameTargetSequentiallyTimes() {
defer stepRunner.Stop()

resultChan, addTarget, 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,
Expand Down Expand Up @@ -200,7 +200,7 @@ func (s *StepRunnerSuite) TestAddTargetReturnsErrorIfFailsToInput() {
defer stepRunner.Stop()

resultChan, addTarget, 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,
Expand Down Expand Up @@ -246,7 +246,7 @@ func (s *StepRunnerSuite) TestStepPanics() {
defer stepRunner.Stop()

resultChan, addTarget, 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,
Expand Down Expand Up @@ -306,7 +306,7 @@ func (s *StepRunnerSuite) TestCornerCases() {
defer stepRunner.Stop()

resultChan, addTarget, 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,
Expand All @@ -326,7 +326,7 @@ func (s *StepRunnerSuite) TestCornerCases() {
defer stepRunner.Stop()

resultChan, _, 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,
Expand All @@ -336,7 +336,7 @@ func (s *StepRunnerSuite) TestCornerCases() {
require.NotNil(s.T(), resultChan)

resultChan2, _, 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,
Expand All @@ -352,7 +352,7 @@ func (s *StepRunnerSuite) TestCornerCases() {
defer stepRunner.Stop()

resultChan, _, 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,
Expand All @@ -373,7 +373,7 @@ func (s *StepRunnerSuite) TestCornerCases() {

stepRunner.Stop()
resultChan, _, 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,
Expand All @@ -387,20 +387,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,
Expand Down
2 changes: 1 addition & 1 deletion pkg/runner/test_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ func (tr *TestRunner) runStepIfNeeded(ss *stepState) error {
resumeState := ss.resumeState
ss.resumeState = nil

sva := newStepVariablesAccessor(ss.sb.TestStepLabel, ss.sb.VariablesMapping, tr.stepsVariables)
sva := newStepVariablesAccessor(ss.sb.TestStepLabel, tr.stepsVariables)
resultCh, addTarget, err := ss.stepRunner.Run(ss.ctx, ss.sb, sva, ss.ev, resumeState, ss.resumeStateTargets)
if err != nil {
return fmt.Errorf("failed to stert a step runner for '%s': %v", ss.sb.TestStepLabel, err)
Expand Down
54 changes: 15 additions & 39 deletions pkg/runner/test_runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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"),
},
)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down
Loading

0 comments on commit 51fa609

Please sign in to comment.