Refactor TestCreateVolumeWithFormattingParameters tests

pkg/driver/controller_test.go

@@ -1634,87 +1634,148 @@ func TestCreateVolume(t *testing.T) {
 				checkExpectedErrorCode(t, err, codes.AlreadyExists)
 			},
 		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, tc.testFunc)
+	}
+}
+
+func TestCreateVolumeWithFormattingParameters(t *testing.T) {
+	stdVolCap := []*csi.VolumeCapability{
 		{
-			name: "success with block size",
-			testFunc: func(t *testing.T) {
-				testSuccessWithParameter(t, BlockSizeKey, "4096", stdCapRange, stdVolCap, stdVolSize)
+			AccessType: &csi.VolumeCapability_Mount{
+				Mount: &csi.VolumeCapability_MountVolume{},
+			},
+			AccessMode: &csi.VolumeCapability_AccessMode{
+				Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
 			},
 		},
+	}
+	stdVolSize := int64(5 * 1024 * 1024 * 1024)
+	stdCapRange := &csi.CapacityRange{RequiredBytes: stdVolSize}
+
+	testCases := []struct {
+		name                 string
+		formattingParamKey   string
+		formattingParamValue string
+		errExpected          bool
+	}{
 		{
-			name: "success with inode size",
-			testFunc: func(t *testing.T) {
-				testSuccessWithParameter(t, INodeSizeKey, "256", stdCapRange, stdVolCap, stdVolSize)
-			},
+			name:                 "success with block size",
+			formattingParamKey:   BlockSizeKey,
+			formattingParamValue: "4096",
+			errExpected:          false,
 		},
 		{
-			name: "success with bytes-per-inode",
-			testFunc: func(t *testing.T) {
-				testSuccessWithParameter(t, BytesPerINodeKey, "8192", stdCapRange, stdVolCap, stdVolSize)
-			},
+			name:                 "success with inode size",
+			formattingParamKey:   INodeSizeKey,
+			formattingParamValue: "256",
+			errExpected:          false,
 		},
 		{
-			name: "success with number-of-inodes",
-			testFunc: func(t *testing.T) {
-				testSuccessWithParameter(t, NumberOfINodesKey, "13107200", stdCapRange, stdVolCap, stdVolSize)
-			},
+			name:                 "success with bytes-per-inode",
+			formattingParamKey:   BytesPerINodeKey,
+			formattingParamValue: "8192",
+			errExpected:          false,
 		},
 		{
-			name: "success with bigalloc cluster size",
-			testFunc: func(t *testing.T) {
-				testSuccessWithParameter(t, BigAllocClusterSizeKey, "16384", stdCapRange, stdVolCap, stdVolSize)
-			},
+			name:                 "success with number-of-inodes",
+			formattingParamKey:   NumberOfINodesKey,
+			formattingParamValue: "13107200",
+			errExpected:          false,
+		},
+		{
+			name:                 "success with bigalloc cluster size",
+			formattingParamKey:   BigAllocClusterSizeKey,
+			formattingParamValue: "16384",
+			errExpected:          false,
+		},
+		// NOTE: We do only test non-integer input failures because we rely on mkfs failing during block device formatting as source of truth for valid parameter values
+		{
+			name:                 "failure with block size",
+			formattingParamKey:   BlockSizeKey,
+			formattingParamValue: "wrong_value",
+			errExpected:          true,
+		},
+		{
+			name:                 "failure with inode size",
+			formattingParamKey:   INodeSizeKey,
+			formattingParamValue: "wrong_value",
+			errExpected:          true,
+		},
+		{
+			name:                 "failure with bytes-per-inode",
+			formattingParamKey:   BytesPerINodeKey,
+			formattingParamValue: "wrong_value",
+			errExpected:          true,
+		},
+		{
+			name:                 "failure with number-of-inodes",
+			formattingParamKey:   NumberOfINodesKey,
+			formattingParamValue: "wrong_value",
+			errExpected:          true,
+		},
+		{
+			name:                 "failure with bigalloc cluster size",
+			formattingParamKey:   BigAllocClusterSizeKey,
+			formattingParamValue: "wrong_value",
+			errExpected:          true,
 		},
 	}
-
 	for _, tc := range testCases {
-		t.Run(tc.name, tc.testFunc)
-	}
-}
+		t.Run(tc.name, func(t *testing.T) {
+			assert := assert.New(t)
 
-func testSuccessWithParameter(t *testing.T, key, value string, capRange *csi.CapacityRange, volCap []*csi.VolumeCapability, volSize int64) {
-	req := &csi.CreateVolumeRequest{
-		Name:               "random-vol-name",
-		CapacityRange:      capRange,
-		VolumeCapabilities: volCap,
-		Parameters:         map[string]string{key: value},
-	}
+			req := &csi.CreateVolumeRequest{
+				Name:               "random-vol-name",
+				CapacityRange:      stdCapRange,
+				VolumeCapabilities: stdVolCap,
+				Parameters:         map[string]string{tc.formattingParamKey: tc.formattingParamValue},
+			}
 
-	ctx := context.Background()
+			ctx := context.Background()
 
-	mockDisk := &cloud.Disk{
-		VolumeID:         req.Name,
-		AvailabilityZone: expZone,
-		CapacityGiB:      util.BytesToGiB(volSize),
-	}
+			mockDisk := &cloud.Disk{
+				VolumeID:         req.Name,
+				AvailabilityZone: expZone,
+				CapacityGiB:      util.BytesToGiB(stdVolSize),
+			}
 
-	mockCtl := gomock.NewController(t)
-	defer mockCtl.Finish()
+			mockCtl := gomock.NewController(t)
 
-	mockCloud := cloud.NewMockCloud(mockCtl)
-	mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Any()).Return(mockDisk, nil)
+			mockCloud := cloud.NewMockCloud(mockCtl)
 
-	awsDriver := controllerService{
-		cloud:         mockCloud,
-		inFlight:      internal.NewInFlight(),
-		driverOptions: &DriverOptions{},
-	}
+			// CreateDisk not called on Unhappy Case
+			if !tc.errExpected {
+				mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Any()).Return(mockDisk, nil)
+				defer mockCtl.Finish()
+			}
 
-	response, err := awsDriver.CreateVolume(ctx, req)
-	if err != nil {
-		srvErr, ok := status.FromError(err)
-		if !ok {
-			t.Fatalf("Could not get error status code from error: %v", srvErr)
-		}
-		t.Fatalf("Unexpected error: %v", srvErr.Code())
-	}
+			awsDriver := controllerService{
+				cloud:         mockCloud,
+				inFlight:      internal.NewInFlight(),
+				driverOptions: &DriverOptions{},
+			}
 
-	volCtx := response.Volume.VolumeContext
-	if sizeValue, ok := volCtx[key]; ok {
-		if sizeValue != value {
-			t.Fatalf("Invalid %s in VolumeContext (got %s expected %s)", key, sizeValue, value)
-		}
-	} else {
-		t.Fatalf("Missing key %s in VolumeContext", key)
+			response, err := awsDriver.CreateVolume(ctx, req)
+
+			// Splits happy case tests from unhappy case tests
+			if !tc.errExpected {
+				assert.Nilf(err, "Unexpected error: %w", err)
+
+				volCtx := response.Volume.VolumeContext
+
+				createdFormattingParamValue, ok := volCtx[tc.formattingParamKey]
+				assert.Truef(ok, "Missing key %s in VolumeContext", tc.formattingParamKey)
+
+				assert.Equalf(createdFormattingParamValue, tc.formattingParamValue, "Invalid %s in VolumeContext", tc.formattingParamKey)
+			} else {
+				assert.NotNilf(err, "CreateVolume did not return an error")
+
+				checkExpectedErrorCode(t, err, codes.InvalidArgument)
+			}
+		})
 	}
 }