Reject test options in the fake pipelines service as the service does

Author: tharunbhuk-db

libs/testserver/pipelines.go

@@ -181,6 +181,14 @@ func (s *FakeWorkspace) PipelineStartUpdate(pipelineId string, body []byte) Resp
 			Body:       map[string]string{"error_code": "INVALID_PARAMETER_VALUE", "message": "test_options requires test_only to be true."},
 		}
 	}
+	if test.TestOptions != nil {
+		if message := pipelineTestOptionsError(test.TestOptions.PytestArgs, test.TestOptions.TestSelection); message != "" {
+			return Response{
+				StatusCode: 400,
+				Body:       map[string]string{"error_code": "INVALID_PARAMETER_VALUE", "message": message},
+			}
+		}
+	}
 
 	updateId := nextUUID()
 	update := &fakePipelineUpdate{

libs/testserver/pipelines_testing.go

@@ -105,6 +105,31 @@ type pipelineTestRequest struct {
 
 var pipelineTestFunctionRegexp = regexp.MustCompile(`(?m)^def (test_\w+)\(`)
 
+// pipelineTestOptionsError returns the service's message for test options that it rejects, or "". Like
+// PipelineTestSelection in the service, pytest_args holds -k and -m, each followed by a value that is not
+// empty and does not start with "-", and each test selection is an absolute workspace path.
+func pipelineTestOptionsError(pytestArgs, selection []string) string {
+	for i := 0; i < len(pytestArgs); i += 2 {
+		option := pytestArgs[i]
+		switch {
+		case option != "-k" && option != "-m":
+			return fmt.Sprintf("pytest_args accepts only -k and -m, each followed by its value. Got '%s'.", option)
+		case i+1 == len(pytestArgs):
+			return fmt.Sprintf("The pytest_args option '%s' must be followed by its value.", option)
+		case strings.TrimSpace(pytestArgs[i+1]) == "":
+			return fmt.Sprintf("The value of the pytest_args option '%s' must not be empty.", option)
+		case strings.HasPrefix(pytestArgs[i+1], "-"):
+			return fmt.Sprintf("The value of the pytest_args option '%s' must not start with '-'. Got '%s'.", option, pytestArgs[i+1])
+		}
+	}
+	for _, entry := range selection {
+		if p, _, _ := strings.Cut(strings.TrimSpace(entry), "::"); !strings.HasPrefix(p, "/") {
+			return fmt.Sprintf("The test selection '%s' must be an absolute workspace path, such as /Workspace/Users/someone@example.com/orders/tests/test_orders.py.", entry)
+		}
+	}
+	return ""
+}
+
 // collectPipelineTests stands in for pytest collection: it finds the top-level "def test_*"
 // functions in the workspace test_*.py files under each selection, or under rootPath without one.
 // Like origin.test_case_id, a test ID is the absolute workspace path of its file followed by the

libs/testserver/pipelines_testing_test.go

@@ -166,6 +166,27 @@ func TestPipelineStartUpdateRejectsTestOptionsWithoutTestOnly(t *testing.T) {
 	assert.Equal(t, 400, response.StatusCode)
 }
 
+func TestPipelineStartUpdateRejectsInvalidTestOptions(t *testing.T) {
+	workspace := NewFakeWorkspace("http://test", "dbapi123")
+	pipelineID := createTestPipelineWithTests(t, workspace)
+
+	// The messages are the service's, from PipelineTestSelection.
+	for options, message := range map[string]string{
+		`{"pytest_args": ["-korders"]}`:              "pytest_args accepts only -k and -m, each followed by its value. Got '-korders'.",
+		`{"pytest_args": ["-x"]}`:                    "pytest_args accepts only -k and -m, each followed by its value. Got '-x'.",
+		`{"pytest_args": ["-m"]}`:                    "The pytest_args option '-m' must be followed by its value.",
+		`{"pytest_args": ["-k", " "]}`:               "The value of the pytest_args option '-k' must not be empty.",
+		`{"pytest_args": ["-k", "-m"]}`:              "The value of the pytest_args option '-k' must not start with '-'. Got '-m'.",
+		`{"test_selection": ["tests/test_a.py::t"]}`: "The test selection 'tests/test_a.py::t' must be an absolute workspace path, such as /Workspace/Users/someone@example.com/orders/tests/test_orders.py.",
+	} {
+		response := workspace.PipelineStartUpdate(pipelineID, []byte(`{"test_only": true, "test_options": `+options+`}`))
+		assert.Equal(t, 400, response.StatusCode, options)
+		assert.Equal(t, map[string]string{"error_code": "INVALID_PARAMETER_VALUE", "message": message}, response.Body, options)
+	}
+
+	startTestUpdate(t, workspace, pipelineID, `{"test_only": true, "test_options": {"pytest_args": ["-k", "orders", "-m", "smoke"], "test_selection": ["`+testRootPath+`/tests"]}}`)
+}
+
 func TestPipelineFiniteUpdateIsActiveUntilItEnds(t *testing.T) {
 	workspace := NewFakeWorkspace("http://test", "dbapi123")
 	pipelineID := createTestPipelineWithTests(t, workspace)