Code-style improvements

Signed-off-by: Ilya <[email protected]>

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{

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
 }

pkg/runner/test_runner.go

@@ -44,7 +44,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.
@@ -171,14 +171,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
 		}
@@ -258,7 +258,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
@@ -545,7 +545,7 @@ func (tr *TestRunner) runStepIfNeeded(ss *stepState) error {
 	resumeState := ss.resumeState
 	ss.resumeState = nil
 
-	sva := newStepVariablesAccessor(ss.sb.TestStepLabel, tr.stepsVariables)
+	sva := newStepVariablesAccessor(ss.sb.TestStepLabel, tr.stepOutputs)
 	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)

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 (

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
 }

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"))
 }