Add unit tests for PluginRegistry.NewTestStepBundle

Add check that variables refer to existing test steps Signed-off-by: Ilya <[email protected]>
linuxboot · Jun 27, 2022 · 00915fa · 00915fa
1 parent c7fb520
commit 00915fa
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 2 deletions.
diff --git a/pkg/jobmanager/bundles.go b/pkg/jobmanager/bundles.go
@@ -98,5 +98,17 @@ func newStepBundles(ctx xcontext.Context, descriptors test.TestStepsDescriptors,
 		}
 		labels[bundle.TestStepLabel] = true
 	}
+	// verify that all variables mappings refer to existing labels
+	for _, bundle := range testStepBundles {
+		if bundle.VariablesMapping == nil {
+			continue
+		}
+		for _, sv := range bundle.VariablesMapping {
+			if _, ok := labels[sv.StepLabel]; !ok {
+				return nil, fmt.Errorf("variable '%s.%s' of step '%s' refers to unexisting step",
+					sv.StepLabel, sv.VariableName, bundle.TestStepLabel)
+			}
+		}
+	}
 	return testStepBundles, nil
 }
diff --git a/pkg/jobmanager/job_test.go b/pkg/jobmanager/job_test.go
@@ -112,6 +112,122 @@ func TestNewJobNoTests(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestVariablesReferToExistingStepLabels(t *testing.T) {
+	pr := pluginregistry.NewPluginRegistry(xcontext.Background())
+	require.NoError(t, pr.RegisterTestStep(echo.Load()))
+	require.NoError(t, pr.RegisterTargetManager(targetlist.Load()))
+	require.NoError(t, pr.RegisterTestFetcher(literal.Load()))
+	require.NoError(t, pr.RegisterReporter(noop.Load()))
+
+	targetManagerAcquireParameters := `{
+		"Targets": [
+			{
+				"ID": "id1",
+				"FQDN": "some-host.fna1.dummy-facebook.com"
+			}
+		]
+	}`
+
+	t.Run("correct_label", func(t *testing.T) {
+		testParams := `{
+			"TestName": "TestVariables",
+			"Steps": [
+				{
+					"name": "echo",
+					"label": "echo1",
+					"parameters": {
+						"text": ["Some text1"]
+					}
+				},
+				{
+					"name": "echo",
+					"label": "echo2",
+					"parameters": {
+						"text": ["Some text1"]
+					},
+					"variablesmapping": {
+						"output_var": "echo1.message"
+					}
+				}
+			]
+		}`
+
+		testDescriptors := []*test.TestDescriptor{
+			{
+				TargetManagerName:              "targetList",
+				TargetManagerAcquireParameters: []byte(targetManagerAcquireParameters),
+				TargetManagerReleaseParameters: []byte("{}"),
+				TestFetcherName:                "literal",
+				TestFetcherFetchParameters:     []byte(testParams),
+			},
+		}
+
+		jd := job.Descriptor{
+			TestDescriptors: testDescriptors,
+			JobName:         "Test",
+			Reporting: job.Reporting{
+				RunReporters: []job.ReporterConfig{
+					{Name: "noop"},
+				},
+			},
+		}
+
+		result, err := NewJobFromDescriptor(xcontext.Background(), pr, &jd)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+		require.Len(t, result.Tests, 1)
+		require.Equal(t, "TestVariables", result.Tests[0].Name)
+	})
+
+	t.Run("unexisting_label", func(t *testing.T) {
+		testParams := `{
+			"TestName": "TestVariables",
+			"Steps": [
+				{
+					"name": "echo",
+					"label": "echo1",
+					"parameters": {
+						"text": ["Some text1"]
+					}
+				},
+				{
+					"name": "echo",
+					"label": "echo2",
+					"parameters": {
+						"text": ["Some text1"]
+					},
+					"variablesmapping": {
+						"output_var": "noecho.message"
+					}
+				}
+			]
+		}`
+
+		testDescriptors := []*test.TestDescriptor{
+			{
+				TargetManagerName:              "targetList",
+				TargetManagerAcquireParameters: []byte(targetManagerAcquireParameters),
+				TargetManagerReleaseParameters: []byte("{}"),
+				TestFetcherName:                "literal",
+				TestFetcherFetchParameters:     []byte(testParams),
+			},
+		}
+
+		jd := job.Descriptor{
+			TestDescriptors: testDescriptors,
+			JobName:         "Test",
+			Reporting: job.Reporting{
+				RunReporters: []job.ReporterConfig{
+					{Name: "noop"},
+				},
+			},
+		}
+
+		_, err := NewJobFromDescriptor(xcontext.Background(), pr, &jd)
+		require.Error(t, err)
+	})
+}
+
 func TestNewJobNoTestSteps(t *testing.T) {
 	pr := pluginregistry.NewPluginRegistry(xcontext.Background())
 	// require.NoError(t, pr.RegisterTestStep(echo.Load()))

diff --git a/pkg/pluginregistry/bundles.go b/pkg/pluginregistry/bundles.go
@@ -59,12 +59,11 @@ func (r *PluginRegistry) NewTestStepBundle(ctx xcontext.Context, testStepDescrip
 			}
 
 			variablesMapping[internalName] = test.StepVariable{
-				StepName:     parts[0],
+				StepLabel:    parts[0],
 				VariableName: parts[1],
 			}
 		}
 	}
-	// TODO: check that all testStep labels from variable mappings exist
 	testStepBundle := test.TestStepBundle{
 		TestStep:         testStep,
 		TestStepLabel:    label,

diff --git a/pkg/pluginregistry/pluginregistry_test.go b/pkg/pluginregistry/pluginregistry_test.go
@@ -64,3 +64,37 @@ func TestRegisterTestStepDoesNotValidate(t *testing.T) {
 	err := pr.RegisterTestStep("AStep", NewAStep, []event.Name{"Event which does not validate"})
 	require.Error(t, err)
 }
+
+func TestNewTestStepBundle(t *testing.T) {
+	t.Run("valid_bundle", func(t *testing.T) {
+		ctx := logrusctx.NewContext(logger.LevelDebug)
+		pr := NewPluginRegistry(ctx)
+		err := pr.RegisterTestStep("AStep", NewAStep, []event.Name{"AStepEventName"})
+		require.NoError(t, err)
+
+		_, err = pr.NewTestStepBundle(ctx, test.TestStepDescriptor{
+			Name:  "AStep",
+			Label: "Dummy",
+			VariablesMapping: map[string]string{
+				"variable": "step_label.var",
+			},
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("invalid_variable_name", func(t *testing.T) {
+		ctx := logrusctx.NewContext(logger.LevelDebug)
+		pr := NewPluginRegistry(ctx)
+		err := pr.RegisterTestStep("AStep", NewAStep, []event.Name{"AStepEventName"})
+		require.NoError(t, err)
+
+		_, err = pr.NewTestStepBundle(ctx, test.TestStepDescriptor{
+			Name:  "AStep",
+			Label: "Dummy",
+			VariablesMapping: map[string]string{
+				"variable   ": "step_label.var",
+			},
+		})
+		require.Error(t, err)
+	})
+}