rewrite TestParseRestartPolicy to use sub-tests

cli/command/container/opts_test.go

@@ -700,34 +700,75 @@ func TestRunFlagsParseShmSize(t *testing.T) {
 }
 
 func TestParseRestartPolicy(t *testing.T) {
-	invalids := map[string]string{
-		"always:2:3":         "invalid restart policy format: maximum retry count must be an integer",
-		"on-failure:invalid": "invalid restart policy format: maximum retry count must be an integer",
-	}
-	valids := map[string]container.RestartPolicy{
-		"": {},
-		"always": {
-			Name:              "always",
-			MaximumRetryCount: 0,
+	tests := []struct {
+		input       string
+		expected    container.RestartPolicy
+		expectedErr string
+	}{
+		{
+			input: "",
 		},
-		"on-failure:1": {
-			Name:              "on-failure",
-			MaximumRetryCount: 1,
+		{
+			input: "no",
+			expected: container.RestartPolicy{
+				Name: "no",
+			},
+		},
+		{
+			input:       ":1",
+			expectedErr: "invalid restart policy format: no policy provided before colon",
+		},
+		{
+			input: "always",
+			expected: container.RestartPolicy{
+				Name: "always",
+			},
+		},
+		{
+			input: "always:1",
+			expected: container.RestartPolicy{
+				Name:              "always",
+				MaximumRetryCount: 1,
+			},
+		},
+		{
+			input:       "always:2:3",
+			expectedErr: "invalid restart policy format: maximum retry count must be an integer",
+		},
+		{
+			input: "on-failure:1",
+			expected: container.RestartPolicy{
+				Name:              "on-failure",
+				MaximumRetryCount: 1,
+			},
+		},
+		{
+			input:       "on-failure:invalid",
+			expectedErr: "invalid restart policy format: maximum retry count must be an integer",
+		},
+		{
+			input: "unless-stopped",
+			expected: container.RestartPolicy{
+				Name: "unless-stopped",
+			},
+		},
+		{
+			input:       "unless-stopped:invalid",
+			expectedErr: "invalid restart policy format: maximum retry count must be an integer",
 		},
 	}
-	for restart, expectedError := range invalids {
-		if _, _, _, err := parseRun([]string{fmt.Sprintf("--restart=%s", restart), "img", "cmd"}); err == nil || err.Error() != expectedError {
-			t.Fatalf("Expected an error with message '%v' for %v, got %v", expectedError, restart, err)
-		}
-	}
-	for restart, expected := range valids {
-		_, hostconfig, _, err := parseRun([]string{fmt.Sprintf("--restart=%v", restart), "img", "cmd"})
-		if err != nil {
-			t.Fatal(err)
-		}
-		if hostconfig.RestartPolicy != expected {
-			t.Fatalf("Expected %v, got %v", expected, hostconfig.RestartPolicy)
-		}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.input, func(t *testing.T) {
+			_, hostConfig, _, err := parseRun([]string{"--restart=" + tc.input, "img", "cmd"})
+			if tc.expectedErr != "" {
+				assert.Check(t, is.Error(err, tc.expectedErr))
+				assert.Check(t, is.Nil(hostConfig))
+			} else {
+				assert.NilError(t, err)
+				assert.Check(t, is.DeepEqual(hostConfig.RestartPolicy, tc.expected))
+			}
+		})
 	}
 }
 

opts/parse.go

@@ -71,17 +71,22 @@ func ConvertKVStringsToMapWithNil(values []string) map[string]*string {
 
 // ParseRestartPolicy returns the parsed policy or an error indicating what is incorrect
 func ParseRestartPolicy(policy string) (container.RestartPolicy, error) {
-	p := container.RestartPolicy{}
-
 	if policy == "" {
-		return p, nil
+		// for backward-compatibility, we don't set the default ("no")
+		// policy here, because older versions of the engine may not
+		// support it.
+		return container.RestartPolicy{}, nil
 	}
 
-	k, v, _ := strings.Cut(policy, ":")
+	p := container.RestartPolicy{}
+	k, v, ok := strings.Cut(policy, ":")
+	if ok && k == "" {
+		return container.RestartPolicy{}, fmt.Errorf("invalid restart policy format: no policy provided before colon")
+	}
 	if v != "" {
 		count, err := strconv.Atoi(v)
 		if err != nil {
-			return p, fmt.Errorf("invalid restart policy format: maximum retry count must be an integer")
+			return container.RestartPolicy{}, fmt.Errorf("invalid restart policy format: maximum retry count must be an integer")
 		}
 		p.MaximumRetryCount = count
 	}