Skip to content

Commit

Permalink
Fix unit test failure
Browse files Browse the repository at this point in the history
Signed-off-by: Parthvi Vala <[email protected]>
  • Loading branch information
valaparthvi committed Jun 20, 2022
1 parent 91c83ef commit 51cae65
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 59 deletions.
6 changes: 3 additions & 3 deletions pkg/preference/implem.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,21 +256,21 @@ func (c *preferenceInfo) SetConfiguration(parameter string, value string) error
case "timeout":
typedval, err := time.ParseDuration(value)
if err != nil || typedval < minimumDurationValue {
return fmt.Errorf("unable to set %q to %q, value must be a positive Duration (e.g. 4s, 5m, 1h)", parameter, value)
return fmt.Errorf("unable to set %q to %q, value must be a positive Duration (e.g. 4s, 5m, 1h); minimum value: 1s", parameter, value)
}
c.OdoSettings.Timeout = &typedval

case "pushtimeout":
typedval, err := time.ParseDuration(value)
if err != nil || typedval < minimumDurationValue {
return fmt.Errorf("unable to set %q to %q, value must be Duration (e.g. 4s, 5m, 1h)", parameter, value)
return fmt.Errorf("unable to set %q to %q, value must be a positive Duration (e.g. 4s, 5m, 1h); minimum value: 1s", parameter, value)
}
c.OdoSettings.PushTimeout = &typedval

case "registrycachetime":
typedval, err := time.ParseDuration(value)
if err != nil || typedval < minimumDurationValue {
return fmt.Errorf("unable to set %q to %q, value must be Duration (e.g. 4s, 5m, 1h)", parameter, value)
return fmt.Errorf("unable to set %q to %q, value must be a positive Duration (e.g. 4s, 5m, 1h); minimum value: 1s", parameter, value)
}
c.OdoSettings.RegistryCacheTime = &typedval

Expand Down
107 changes: 51 additions & 56 deletions pkg/preference/implem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,31 +78,21 @@ func TestGetPushTimeout(t *testing.T) {
}
defer tempConfigFile.Close()
os.Setenv(GlobalConfigEnvName, tempConfigFile.Name())
zeroValue := 0 * time.Second

nonzeroValue := 5 * time.Second

tests := []struct {
name string
existingConfig Preference
want time.Duration
}{
{
name: "Case 1: Validating default value from test case",
name: "Validating default value from test case",
existingConfig: Preference{},
want: 240,
},

{
name: "Case 2: Validating value 0 from configuration",
existingConfig: Preference{
OdoSettings: odoSettings{
PushTimeout: &zeroValue,
},
},
want: 0,
},

{
name: "Case 3: Validating value 5 from configuration",
name: "Validating value 5 from configuration",
existingConfig: Preference{
OdoSettings: odoSettings{
PushTimeout: &nonzeroValue,
Expand Down Expand Up @@ -142,13 +132,13 @@ func TestGetTimeout(t *testing.T) {
want time.Duration
}{
{
name: "Case 1: validating value 1 from config in default case",
name: "validating value 1 from config in default case",
existingConfig: Preference{},
want: 1,
},

{
name: "Case 2: validating value 0 from config",
name: "validating value 0 from config",
existingConfig: Preference{
OdoSettings: odoSettings{
Timeout: &zeroValue,
Expand All @@ -158,7 +148,7 @@ func TestGetTimeout(t *testing.T) {
},

{
name: "Case 3: validating value 5 from config",
name: "validating value 5 from config",
existingConfig: Preference{
OdoSettings: odoSettings{
Timeout: &nonzeroValue,
Expand Down Expand Up @@ -192,7 +182,7 @@ func TestSetConfiguration(t *testing.T) {
os.Setenv(GlobalConfigEnvName, tempConfigFile.Name())
trueValue := true
falseValue := false
zeroValue := 0 * time.Second
minValue := minimumDurationValue

tests := []struct {
name string
Expand All @@ -204,15 +194,15 @@ func TestSetConfiguration(t *testing.T) {
}{
// update notification
{
name: fmt.Sprintf("Case 1: %s set nil to true", UpdateNotificationSetting),
name: fmt.Sprintf("%s set nil to true", UpdateNotificationSetting),
parameter: UpdateNotificationSetting,
value: "true",
existingConfig: Preference{},
want: true,
wantErr: false,
},
{
name: fmt.Sprintf("Case 2: %s set true to false", UpdateNotificationSetting),
name: fmt.Sprintf("%s set true to false", UpdateNotificationSetting),
parameter: UpdateNotificationSetting,
value: "false",
existingConfig: Preference{
Expand All @@ -224,7 +214,7 @@ func TestSetConfiguration(t *testing.T) {
wantErr: false,
},
{
name: fmt.Sprintf("Case 3: %s set false to true", UpdateNotificationSetting),
name: fmt.Sprintf("%s set false to true", UpdateNotificationSetting),
parameter: UpdateNotificationSetting,
value: "true",
existingConfig: Preference{
Expand All @@ -237,50 +227,49 @@ func TestSetConfiguration(t *testing.T) {
},

{
name: fmt.Sprintf("Case 4: %s invalid value", UpdateNotificationSetting),
name: fmt.Sprintf("%s invalid value", UpdateNotificationSetting),
parameter: UpdateNotificationSetting,
value: "invalid_value",
existingConfig: Preference{},
wantErr: true,
},
// time out
{
name: fmt.Sprintf("Case 5: %s set to 5 from 0", TimeoutSetting),
name: fmt.Sprintf("%s set to 5 from 0", TimeoutSetting),
parameter: TimeoutSetting,
value: "5s",
existingConfig: Preference{
OdoSettings: odoSettings{
Timeout: &zeroValue,
Timeout: &minValue,
},
},
want: 5 * time.Second,
wantErr: false,
},
{
name: fmt.Sprintf("Case 6: %s set to 300", TimeoutSetting),
name: fmt.Sprintf("%s set to 300", TimeoutSetting),
parameter: TimeoutSetting,
value: "5m",
existingConfig: Preference{},
want: 5 * time.Second,
wantErr: false,
},
{
name: fmt.Sprintf("Case 7: %s set to 0", TimeoutSetting),
name: fmt.Sprintf("%s set to 0", TimeoutSetting),
parameter: TimeoutSetting,
value: "0s",
existingConfig: Preference{},
want: 0 * time.Second,
wantErr: false,
wantErr: true,
},
{
name: fmt.Sprintf("Case 9: %s invalid value", TimeoutSetting),
name: fmt.Sprintf("%s invalid value", TimeoutSetting),
parameter: TimeoutSetting,
value: "this",
existingConfig: Preference{},
wantErr: true,
},
{
name: fmt.Sprintf("Case 10: %s set to 300 with mixed case in parameter name", TimeoutSetting),
name: fmt.Sprintf("%s set to 300 with mixed case in parameter name", TimeoutSetting),
parameter: "TimeOut",
value: "5m",
existingConfig: Preference{},
Expand All @@ -289,66 +278,72 @@ func TestSetConfiguration(t *testing.T) {
},
// invalid parameter
{
name: "Case 11: invalid parameter",
name: "invalid parameter",
parameter: "invalid_parameter",
existingConfig: Preference{},
wantErr: true,
},
{
name: fmt.Sprintf("Case 12: %s set to 0", TimeoutSetting),
name: fmt.Sprintf("%s set to 0", TimeoutSetting),
parameter: TimeoutSetting,
value: "0s",
existingConfig: Preference{},
want: 0 * time.Second,
wantErr: false,
wantErr: true,
},
{
name: fmt.Sprintf("Case 13: %s invalid value", TimeoutSetting),
name: fmt.Sprintf("%s invalid value", TimeoutSetting),
parameter: TimeoutSetting,
value: "invalid",
existingConfig: Preference{},
wantErr: true,
},
{
name: fmt.Sprintf("Case 14: %s set to 99 with mixed case in parameter name", TimeoutSetting),
name: fmt.Sprintf("%s negative value", TimeoutSetting),
parameter: TimeoutSetting,
value: "-5s",
existingConfig: Preference{},
wantErr: true,
},
{
name: fmt.Sprintf("%s set to 99 with mixed case in parameter name", TimeoutSetting),
parameter: "PushTimeout",
value: "99s",
existingConfig: Preference{},
want: 99 * time.Second,
wantErr: false,
},
{
name: "Case 15: set RegistryCacheTime to 1 minutes",
name: "set RegistryCacheTime to 1 minutes",
parameter: "RegistryCacheTime",
value: "1m",
existingConfig: Preference{},
want: 1 * time.Minute,
wantErr: false,
},
{
name: "Case 16: set RegistryCacheTime to non int value",
name: "set RegistryCacheTime to non int value",
parameter: "RegistryCacheTime",
value: "a",
existingConfig: Preference{},
wantErr: true,
},
{
name: fmt.Sprintf("Case 17: set %s to non bool value", ConsentTelemetrySetting),
name: fmt.Sprintf("set %s to non bool value", ConsentTelemetrySetting),
parameter: ConsentTelemetrySetting,
value: "123",
existingConfig: Preference{},
wantErr: true,
},
{
name: fmt.Sprintf("Case 18: set %s from nil to true", ConsentTelemetrySetting),
name: fmt.Sprintf("set %s from nil to true", ConsentTelemetrySetting),
parameter: ConsentTelemetrySetting,
value: "true",
existingConfig: Preference{},
wantErr: false,
want: true,
},
{
name: fmt.Sprintf("Case 19: set %s from true to false", ConsentTelemetrySetting),
name: fmt.Sprintf("set %s from true to false", ConsentTelemetrySetting),
parameter: ConsentTelemetrySetting,
value: "false",
existingConfig: Preference{
Expand Down Expand Up @@ -422,12 +417,12 @@ func TestConsentTelemetry(t *testing.T) {
want bool
}{
{
name: fmt.Sprintf("Case 1: %s nil", ConsentTelemetrySetting),
name: fmt.Sprintf("%s nil", ConsentTelemetrySetting),
existingConfig: Preference{},
want: false,
},
{
name: fmt.Sprintf("Case 2: %s true", ConsentTelemetrySetting),
name: fmt.Sprintf("%s true", ConsentTelemetrySetting),
existingConfig: Preference{
OdoSettings: odoSettings{
ConsentTelemetry: &trueValue,
Expand All @@ -436,7 +431,7 @@ func TestConsentTelemetry(t *testing.T) {
want: true,
},
{
name: fmt.Sprintf("Case 3: %s false", ConsentTelemetrySetting),
name: fmt.Sprintf("%s false", ConsentTelemetrySetting),
existingConfig: Preference{
OdoSettings: odoSettings{
ConsentTelemetry: &falseValue,
Expand Down Expand Up @@ -478,12 +473,12 @@ func TestGetupdateNotification(t *testing.T) {
want bool
}{
{
name: fmt.Sprintf("Case 1: %s nil", UpdateNotificationSetting),
name: fmt.Sprintf("%s nil", UpdateNotificationSetting),
existingConfig: Preference{},
want: true,
},
{
name: fmt.Sprintf("Case 2: %s true", UpdateNotificationSetting),
name: fmt.Sprintf("%s true", UpdateNotificationSetting),
existingConfig: Preference{
OdoSettings: odoSettings{
UpdateNotification: &trueValue,
Expand All @@ -492,7 +487,7 @@ func TestGetupdateNotification(t *testing.T) {
want: true,
},
{
name: fmt.Sprintf("Case 3: %s false", UpdateNotificationSetting),
name: fmt.Sprintf("%s false", UpdateNotificationSetting),
existingConfig: Preference{
OdoSettings: odoSettings{
UpdateNotification: &falseValue,
Expand Down Expand Up @@ -603,7 +598,7 @@ func TestHandleWithoutRegistryExist(t *testing.T) {
want []Registry
}{
{
name: "Case 1: Add registry",
name: "Add registry",
registryList: []Registry{},
operation: "add",
registryName: "testName",
Expand All @@ -616,15 +611,15 @@ func TestHandleWithoutRegistryExist(t *testing.T) {
},
},
{
name: "Case 2: Update registry",
name: "Update registry",
registryList: []Registry{},
operation: "update",
registryName: "testName",
registryURL: "testURL",
want: nil,
},
{
name: "Case 3: Delete registry",
name: "Delete registry",
registryList: []Registry{},
operation: "delete",
registryName: "testName",
Expand Down Expand Up @@ -659,7 +654,7 @@ func TestHandleWithRegistryExist(t *testing.T) {
want []Registry
}{
{
name: "Case 1: Add registry",
name: "Add registry",
index: 0,
registryList: []Registry{
{
Expand All @@ -674,7 +669,7 @@ func TestHandleWithRegistryExist(t *testing.T) {
want: nil,
},
{
name: "Case 2: update registry",
name: "update registry",
index: 0,
registryList: []Registry{
{
Expand All @@ -694,7 +689,7 @@ func TestHandleWithRegistryExist(t *testing.T) {
},
},
{
name: "Case 3: Delete registry",
name: "Delete registry",
index: 0,
registryList: []Registry{
{
Expand Down Expand Up @@ -737,12 +732,12 @@ func TestGetConsentTelemetry(t *testing.T) {
existingConfig Preference
want bool
}{{
name: fmt.Sprintf("Case 1: %s nil", ConsentTelemetrySetting),
name: fmt.Sprintf("%s nil", ConsentTelemetrySetting),
existingConfig: Preference{},
want: false,
},
{
name: fmt.Sprintf("Case 2: %s true", ConsentTelemetrySetting),
name: fmt.Sprintf("%s true", ConsentTelemetrySetting),
existingConfig: Preference{
OdoSettings: odoSettings{
ConsentTelemetry: &trueValue,
Expand All @@ -751,7 +746,7 @@ func TestGetConsentTelemetry(t *testing.T) {
want: true,
},
{
name: fmt.Sprintf("Case 3: %s false", ConsentTelemetrySetting),
name: fmt.Sprintf("%s false", ConsentTelemetrySetting),
existingConfig: Preference{
OdoSettings: odoSettings{
ConsentTelemetry: &falseValue,
Expand Down

0 comments on commit 51cae65

Please sign in to comment.