Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug in flagutil package and added tests #15046

Merged
merged 1 commit into from
Jan 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions go/flagutil/enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,12 @@ func newStringEnum(name string, initialValue string, choices []string, caseInsen
}

return &StringEnum{
name: name,
val: initialValue,
choices: choiceMap,
choiceNames: choiceNames,
choiceMapper: choiceMapper,
name: name,
val: initialValue,
choices: choiceMap,
choiceNames: choiceNames,
choiceMapper: choiceMapper,
caseInsensitive: caseInsensitive,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

}
}

Expand Down
79 changes: 79 additions & 0 deletions go/flagutil/enum_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
Copyright 2024 The Vitess Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package flagutil

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestStringEnum(t *testing.T) {
tests := []struct {
name string
initialValue string
choices []string
caseInsensitive bool
secondValue string
expectedErr string
}{
{
name: "valid set call",
initialValue: "mango",
choices: []string{"apple", "mango", "kiwi"},
caseInsensitive: true,
secondValue: "kiwi",
},
{
name: "invalid set call",
initialValue: "apple",
choices: []string{"apple", "mango", "kiwi"},
caseInsensitive: false,
secondValue: "banana",
expectedErr: "invalid choice for enum (valid choices: [apple kiwi mango])",
},
{
name: "invalid set call case insensitive",
initialValue: "apple",
choices: []string{"apple", "kiwi"},
caseInsensitive: true,
secondValue: "banana",
expectedErr: "invalid choice for enum (valid choices: [apple kiwi] [case insensitive])",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to capture the tt variable within the loop. See this for an example/explainer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, done.

var enum *StringEnum
if tt.caseInsensitive {
enum = NewCaseInsensitiveStringEnum(tt.name, tt.initialValue, tt.choices)
} else {
enum = NewStringEnum(tt.name, tt.initialValue, tt.choices)
}

require.Equal(t, "string", enum.Type())
err := enum.Set(tt.secondValue)
if tt.expectedErr == "" {
require.NoError(t, err)
require.Equal(t, tt.secondValue, enum.String())
} else {
require.ErrorContains(t, err, tt.expectedErr)
require.Equal(t, tt.initialValue, enum.String())
}
})
}
}
15 changes: 15 additions & 0 deletions go/flagutil/flagutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/spf13/pflag"
"github.com/stretchr/testify/require"
)

func TestStringList(t *testing.T) {
Expand Down Expand Up @@ -106,3 +107,17 @@ func TestStringMap(t *testing.T) {
}
}
}

func TestStringListValue(t *testing.T) {
strListVal := StringListValue{"temp", "val"}
require.Equal(t, []string([]string{"temp", "val"}), strListVal.Get())
require.Equal(t, "strings", strListVal.Type())
}

func TestStringMapValue(t *testing.T) {
strMapVal := StringMapValue{
"key": "val",
}
require.Equal(t, "StringMap", strMapVal.Type())
require.Equal(t, map[string]string(map[string]string{"key": "val"}), strMapVal.Get())
}
58 changes: 58 additions & 0 deletions go/flagutil/optional_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright 2024 The Vitess Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package flagutil

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestNewOptionalFloat64(t *testing.T) {
fl := NewOptionalFloat64(4.187)
require.NotEmpty(t, fl)
require.Equal(t, false, fl.IsSet())

require.Equal(t, "4.187", fl.String())
require.Equal(t, "float64", fl.Type())

err := fl.Set("invalid value")
require.ErrorContains(t, err, "parse error")

err = fl.Set("7.77")
require.NoError(t, err)
require.Equal(t, 7.77, fl.Get())
require.Equal(t, true, fl.IsSet())

err = fl.Set("1e1000")
require.ErrorContains(t, err, "value out of range")
}

func TestNewOptionalString(t *testing.T) {
optStr := NewOptionalString("4.187")
require.NotEmpty(t, optStr)
require.Equal(t, false, optStr.IsSet())

require.Equal(t, "4.187", optStr.String())
require.Equal(t, "string", optStr.Type())

err := optStr.Set("value")
require.NoError(t, err)

require.Equal(t, "value", optStr.Get())
require.Equal(t, true, optStr.IsSet())
}
54 changes: 54 additions & 0 deletions go/flagutil/sets_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
Copyright 2019 The Vitess Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package flagutil

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestStringSetFlag(t *testing.T) {
strSetFlag := StringSetFlag{}
set := strSetFlag.ToSet()
require.Empty(t, set)

set = set.Insert("mango", "apple", "mango")
strSetFlag.set = set

require.Equal(t, "StringSetFlag", strSetFlag.Type())
require.Equal(t, "apple, mango", strSetFlag.String())

err := strSetFlag.Set("guvava")
require.NoError(t, err)
require.Equal(t, "apple, guvava, mango", strSetFlag.String())

require.NotEmpty(t, strSetFlag.ToSet())
}

func TestStringSetFlagWithEmptySet(t *testing.T) {
strSetFlag := StringSetFlag{}
require.Equal(t, "", strSetFlag.String())

err := strSetFlag.Set("tmp")
require.NoError(t, err)
require.Empty(t, strSetFlag.ToSet())

err = strSetFlag.Set("guvava")
require.NoError(t, err)
require.Equal(t, "guvava", strSetFlag.String())
}
Loading