Skip to content

switches refactoring #37

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 18, 2022
Merged

switches refactoring #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1c688d
switches refactoring
GrigoriyMikhalkin Jan 14, 2022
9355dfc
refactoring pt2
GrigoriyMikhalkin Jan 18, 2022
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
86 changes: 64 additions & 22 deletions cmdutils/switches/switches.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ package switches
import (
"encoding/csv"
"fmt"
"sort"
"strings"

"sigs.k8s.io/kustomize/kyaml/sets"
"k8s.io/apimachinery/pkg/util/sets"
)

const (
Expand All @@ -31,15 +32,25 @@ const (
)

type Switches struct {
defaults map[string]bool
settings map[string]bool
}

// New creates an instance of Switches
func New(settings []string) *Switches {
s := &Switches{
// New creates an instance of Switches and returns the pointer to it
func New(settings ...string) *Switches {
s := Make(settings...)
return &s
}

// Make creates an instance of Switches
// Same as New but returns copy of a struct, not a pointer
func Make(settings ...string) Switches {
s := Switches{
defaults: make(map[string]bool),
settings: make(map[string]bool),
}
s.setSettings(settings)

s.defaults = s.prepareSettings(settings)
return s
}

Expand All @@ -49,7 +60,27 @@ func Disable(name string) string {
}

func (s *Switches) String() string {
return fmt.Sprintf("%v", s.settings)
var res string

vals := make([]string, 0, len(s.defaults))
for v := range s.defaults {
vals = append(vals, v)
}

sort.Strings(vals)
for _, v := range vals {
if res != "" {
res += ","
}

if s.settings[v] {
res += v
} else {
res += "-" + v
}
}

return res
}

func (s *Switches) Set(val string) error {
Expand All @@ -70,15 +101,15 @@ func (s *Switches) Set(val string) error {
// Validate that all specified controllers are known
for _, v := range settings {
trimmed := strings.TrimPrefix(v, disablePrefix)
if _, ok := s.settings[trimmed]; trimmed != All && !ok {
if _, ok := s.defaults[trimmed]; trimmed != All && !ok {
return fmt.Errorf("unknown item: %s", trimmed)
}
}
} else {
settings = []string{""}
}

s.setSettings(settings)
s.settings = s.prepareSettings(settings)
return nil
}

Expand All @@ -89,18 +120,30 @@ func (s *Switches) Enabled(name string) bool {

// All returns names of all items set in settings
func (s *Switches) All() sets.String {
names := make(sets.String, len(s.settings))
names := sets.NewString()
for k := range s.settings {
names.Insert(k)
}

return names
}

// EnabledByDefault returns names of all enabled items
func (s *Switches) EnabledByDefault() sets.String {
names := sets.NewString()
for k, enabled := range s.defaults {
if enabled {
names.Insert(k)
}
}

return names
}

// DisabledByDefault returns names of all disabled items
func (s *Switches) DisabledByDefault() sets.String {
names := make(sets.String)
for k, enabled := range s.settings {
names := sets.NewString()
for k, enabled := range s.defaults {
if !enabled {
names.Insert(k)
}
Expand All @@ -110,32 +153,31 @@ func (s *Switches) DisabledByDefault() sets.String {
}

func (s *Switches) Type() string {
return "Switches"
return "strings"
}

func (s *Switches) setSettings(settings []string) {
func (s *Switches) prepareSettings(settings []string) (res map[string]bool) {
res = make(map[string]bool)

if len(settings) == 1 && settings[0] == "" {
return
}

var isDefault bool
for _, v := range settings {
if v == All {
isDefault = true
for k, v := range s.defaults {
res[k] = v
}
break
}
}

if !isDefault {
for k := range s.settings {
s.settings[k] = false
}
}

for _, v := range settings {
if v == All {
continue
}
s.settings[strings.TrimPrefix(v, disablePrefix)] = !strings.HasPrefix(v, disablePrefix)
res[strings.TrimPrefix(v, disablePrefix)] = !strings.HasPrefix(v, disablePrefix)
}

return
}
56 changes: 31 additions & 25 deletions cmdutils/switches/switches_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,51 +19,57 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/spf13/pflag"
"sigs.k8s.io/kustomize/kyaml/sets"
"k8s.io/apimachinery/pkg/util/sets"
)

var _ = Describe("CMD Switches", func() {
Context("Testing Switches interface", func() {
It("should disable runner", func() {
s := New([]string{"runner-a", Disable("runner-b")})
s := New("runner-a", "runner-b")
Expect(s.Set("*,-runner-b")).ToNot(HaveOccurred())
Expect(s.Enabled("runner-a")).To(BeTrue())
Expect(s.Enabled("runner-b")).To(BeFalse())
})
It("should return all items", func() {
s := New([]string{"runner-a", Disable("runner-b")})
s := New("runner-a", "runner-b")
Expect(s.Set("*,-runner-b")).ToNot(HaveOccurred())

expected := make(sets.String)
expected.Insert("runner-a", "runner-b")
expected := sets.NewString("runner-a", "runner-b")
Expect(s.All()).To(Equal(expected))
})
It("should return all enabled items", func() {
s := New("runner-a", Disable("runner-b"))

expected := sets.NewString("runner-a")
Expect(s.EnabledByDefault()).To(Equal(expected))
})
It("should return all disabled items", func() {
s := New([]string{"runner-a", Disable("runner-b")})
s := New("runner-a", Disable("runner-b"))

expected := make(sets.String)
expected.Insert("runner-b")
expected := sets.NewString("runner-b")
Expect(s.DisabledByDefault()).To(Equal(expected))
})
It("should return string", func() {
s := New([]string{"runner-a", Disable("runner-b")})

Expect(s.String()).To(Equal("map[runner-a:true runner-b:false]"))
s := New("runner-a", "runner-b")
Expect(s.Set("*,-runner-b")).ToNot(HaveOccurred())
Expect(s.String()).To(Equal("runner-a,-runner-b"))
})
})

Context("Testing flag package behavior", func() {
It("should keep default settings when no flag is passed", func() {
It("should disable all controllers when no flag is passed", func() {
fs := flag.NewFlagSet("", flag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{})).NotTo(HaveOccurred())
Expect(controllers.Enabled("runner-a")).To(BeTrue())
Expect(controllers.Enabled("runner-a")).To(BeFalse())
Expect(controllers.Enabled("runner-b")).To(BeFalse())
Expect(controllers.Enabled("runner-c")).To(BeTrue())
Expect(controllers.Enabled("runner-c")).To(BeFalse())
})
It("should keep default settings when * is passed", func() {
fs := flag.NewFlagSet("", flag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{"--controllers=*"})).NotTo(HaveOccurred())
Expand All @@ -73,7 +79,7 @@ var _ = Describe("CMD Switches", func() {
})
It("should override default settings", func() {
fs := flag.NewFlagSet("", flag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{"--controllers=runner-a,-runner-c"})).NotTo(HaveOccurred())
Expand All @@ -83,7 +89,7 @@ var _ = Describe("CMD Switches", func() {
})
It("should override some of default settings", func() {
fs := flag.NewFlagSet("", flag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{"--controllers=*,-runner-a"})).NotTo(HaveOccurred())
Expand All @@ -94,19 +100,19 @@ var _ = Describe("CMD Switches", func() {
})

Context("Testing pflag package behavior", func() {
It("should keep default settings when no flag is passed", func() {
It("should disable all controllers when no flag is passed", func() {
fs := pflag.NewFlagSet("", pflag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{})).NotTo(HaveOccurred())
Expect(controllers.Enabled("runner-a")).To(BeTrue())
Expect(controllers.Enabled("runner-a")).To(BeFalse())
Expect(controllers.Enabled("runner-b")).To(BeFalse())
Expect(controllers.Enabled("runner-c")).To(BeTrue())
Expect(controllers.Enabled("runner-c")).To(BeFalse())
})
It("should keep default settings when * is passed", func() {
fs := pflag.NewFlagSet("", pflag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{"--controllers=*"})).NotTo(HaveOccurred())
Expand All @@ -116,7 +122,7 @@ var _ = Describe("CMD Switches", func() {
})
It("should override default settings", func() {
fs := pflag.NewFlagSet("", pflag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{"--controllers=runner-a,-runner-c"})).NotTo(HaveOccurred())
Expand All @@ -126,7 +132,7 @@ var _ = Describe("CMD Switches", func() {
})
It("should override some of default settings", func() {
fs := pflag.NewFlagSet("", pflag.ExitOnError)
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
controllers := New("runner-a", Disable("runner-b"), "runner-c")
fs.Var(controllers, "controllers", "")

Expect(fs.Parse([]string{"--controllers=*,-runner-a"})).NotTo(HaveOccurred())
Expand Down