Skip to content

Commit

Permalink
fix spelling and formating issues, add addtional test case, and remov…
Browse files Browse the repository at this point in the history
…e todo comments
  • Loading branch information
ElijahQuinones committed Aug 5, 2024
1 parent bfebb68 commit 50aea58
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 13 deletions.
6 changes: 3 additions & 3 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,21 @@ func TestNewCloud(t *testing.T) {
batchingEnabled bool
}{
{
name: "sucess with debug,userAgentExtra and batching",
name: "success: with debug, userAgentExtra, and batching",
region: "us-east-1",
awsSdkDebugLog: true,
userAgentExtra: "example_user_agent_extra",
batchingEnabled: true,
},
{
name: "sucess with only debug and userAgenrExtra",
name: "success: with only debug, and userAgentExtra",
region: "us-east-1",
awsSdkDebugLog: true,
userAgentExtra: "example_user_agent_extra",
batchingEnabled: false,
},
{
name: "sucess with only region",
name: "success: with only region",
region: "us-east-1",
awsSdkDebugLog: false,
userAgentExtra: "",
Expand Down
4 changes: 0 additions & 4 deletions pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,6 @@ func (d *ControllerService) CreateVolume(ctx context.Context, req *csi.CreateVol

for key, value := range req.GetParameters() {
switch strings.ToLower(key) {
// we tell user here that they should use csi.storage.k8s.io/fstype instead but there is no switch cases to handle that
// Suggestion to add case FSTypeKey that will set the fstype and in the case "fstype" we print the warning and still set
// fstype for them

case "fstype":
klog.InfoS("\"fstype\" is deprecated, please use \"csi.storage.k8s.io/fstype\" instead")
case VolumeTypeKey:
Expand Down
18 changes: 18 additions & 0 deletions pkg/driver/controller_modify_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ func TestMergeModifyVolumeRequest(t *testing.T) {
expectedModifyVolumeRequest modifyVolumeRequest
expectError bool
}{
{
name: "Valid merge of size and iops",
input: modifyVolumeRequest{
newSize: 5,
},
existing: modifyVolumeRequest{
modifyDiskOptions: cloud.ModifyDiskOptions{
IOPS: validIopsInt,
},
},
expectedModifyVolumeRequest: modifyVolumeRequest{
newSize: 5,
modifyDiskOptions: cloud.ModifyDiskOptions{
IOPS: validIopsInt,
},
},
expectError: false,
},
{
name: "Different size requested by a previous request",
input: modifyVolumeRequest{
Expand Down
4 changes: 0 additions & 4 deletions pkg/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ func NewDriver(c cloud.Cloud, o *Options, m mounter.Mounter, md metadata.Metadat
case AllMode:
driver.controller = NewControllerService(c, o)
driver.node = NewNodeService(o, md, m, k)
/*TODO is the default case even possible to occur if we have a node mode that is not controller driver or all it will return
an error on line 71 as ValidateDriverOptions calls to ValidateMode which only allows the contoller node and all modes or it will
return an error
*/
default:
return nil, fmt.Errorf("unknown mode: %s", o.Mode)
}
Expand Down
1 change: 0 additions & 1 deletion pkg/driver/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ func TestNewDriver(t *testing.T) {
}
if tc.hasController && driver.controller == nil {
t.Fatalf("Expected driver to have controller but driver does not have controller")

}
if tc.expectError {
require.Error(t, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/driver/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestValidateExtraTags(t *testing.T) {
{
name: "invaid tag: key is empty",
tags: map[string]string{
randomString(cloud.MinTagKeyLength - 1): "extra-tag-value",
"": "extra-tag-value",
},
expErr: fmt.Errorf("Tag key cannot be empty (min: 1)"),
},
Expand Down

0 comments on commit 50aea58

Please sign in to comment.