From 55fe4ff5b04c919903c635e347345fab5202db9f Mon Sep 17 00:00:00 2001 From: Derek Nola Date: Tue, 13 Jul 2021 10:44:11 -0700 Subject: [PATCH] Convert existing unit tests to standard layout (#3621) * Converted parser_test.go, scrypt_test.go, types_test.go, nodeconfig_test.go Signed-off-by: dereknola --- pkg/authenticator/hash/scrypt_test.go | 53 ++-- pkg/configfilearg/parser_test.go | 377 +++++++++++++++++--------- pkg/daemons/config/types_test.go | 73 +++-- pkg/nodeconfig/nodeconfig_test.go | 114 +++++--- 4 files changed, 399 insertions(+), 218 deletions(-) diff --git a/pkg/authenticator/hash/scrypt_test.go b/pkg/authenticator/hash/scrypt_test.go index 7b76d852e8..634c6c78b2 100644 --- a/pkg/authenticator/hash/scrypt_test.go +++ b/pkg/authenticator/hash/scrypt_test.go @@ -3,28 +3,37 @@ package hash import ( "strings" "testing" - - "github.com/stretchr/testify/assert" ) -var hasher = NewSCrypt() - -func TestBasicHash(t *testing.T) { - secretKey := "hello world" - hash, err := hasher.CreateHash(secretKey) - assert.Nil(t, err) - assert.NotNil(t, hash) - - assert.Nil(t, hasher.VerifyHash(hash, secretKey)) - assert.NotNil(t, hasher.VerifyHash(hash, "goodbye")) -} - -func TestLongKey(t *testing.T) { - secretKey := strings.Repeat("A", 720) - hash, err := hasher.CreateHash(secretKey) - assert.Nil(t, err) - assert.NotNil(t, hash) - - assert.Nil(t, hasher.VerifyHash(hash, secretKey)) - assert.NotNil(t, hasher.VerifyHash(hash, secretKey+":wrong!")) +func TestSCrypt_VerifyHash(t *testing.T) { + type args struct { + secretKey string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "Basic Hash Test", + args: args{ + secretKey: "hello world", + }, + }, + { + name: "Long Hash Test", + args: args{ + secretKey: strings.Repeat("A", 720), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hasher := NewSCrypt() + hash, _ := hasher.CreateHash(tt.args.secretKey) + if err := hasher.VerifyHash(hash, tt.args.secretKey); (err != nil) != tt.wantErr { + t.Errorf("SCrypt.VerifyHash() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } } diff --git a/pkg/configfilearg/parser_test.go b/pkg/configfilearg/parser_test.go index 55d0439740..0321ba0e14 100644 --- a/pkg/configfilearg/parser_test.go +++ b/pkg/configfilearg/parser_test.go @@ -2,153 +2,181 @@ package configfilearg import ( "os" + "reflect" "testing" - - "github.com/stretchr/testify/assert" ) -func TestFindStart(t *testing.T) { - testCases := []struct { - input []string +func TestParser_findStart(t *testing.T) { + tests := []struct { + name string + args []string prefix []string suffix []string found bool - what string }{ { - input: nil, + name: "default case", + args: nil, prefix: nil, suffix: nil, found: false, - what: "default case", }, { - input: []string{"server"}, + name: "simple case", + args: []string{"server"}, prefix: []string{"server"}, suffix: []string{}, found: true, - what: "simple case", }, { - input: []string{"server", "foo"}, + name: "also simple case", + args: []string{"server", "foo"}, prefix: []string{"server"}, suffix: []string{"foo"}, found: true, - what: "also simple case", }, { - input: []string{"server", "foo", "bar"}, + name: "longer simple case", + args: []string{"server", "foo", "bar"}, prefix: []string{"server"}, suffix: []string{"foo", "bar"}, found: true, - what: "longer simple case", }, { - input: []string{"not-server", "foo", "bar"}, + name: "not found", + args: []string{"not-server", "foo", "bar"}, prefix: []string{"not-server", "foo", "bar"}, found: false, - what: "not found", }, } - - p := Parser{ - After: []string{"server", "agent"}, - } - - for _, testCase := range testCases { - prefix, suffix, found := p.findStart(testCase.input) - assert.Equal(t, testCase.prefix, prefix) - assert.Equal(t, testCase.suffix, suffix) - assert.Equal(t, testCase.found, found) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := Parser{ + After: []string{"server", "agent"}, + } + prefix, suffix, found := p.findStart(tt.args) + if !reflect.DeepEqual(prefix, tt.prefix) { + t.Errorf("Parser.findStart() prefix = %+v\nWant = %+v", prefix, tt.prefix) + } + if !reflect.DeepEqual(suffix, tt.suffix) { + t.Errorf("Parser.findStart() suffix = %+v\nWant = %+v", suffix, tt.suffix) + } + if found != tt.found { + t.Errorf("Parser.findStart() found = %+v\nWant = %+v", found, tt.found) + } + }) } } -func TestConfigFile(t *testing.T) { - testCases := []struct { - input []string - env string - def string - configFile string - found bool - what string +func TestParser_findConfigFileFlag(t *testing.T) { + type fields struct { + DefaultConfig string + env string + } + tests := []struct { + name string + fields fields + arg []string + want string + found bool }{ { - input: nil, + name: "default case", + arg: nil, found: false, - what: "default case", }, { - input: []string{"asdf", "-c", "value"}, - configFile: "value", - found: true, - what: "simple case", + name: "simple case", + arg: []string{"asdf", "-c", "value"}, + want: "value", + found: true, }, { - input: []string{"-c"}, + name: "invalid args string", + arg: []string{"-c"}, found: false, - what: "invalid args string", }, { - input: []string{"-c="}, + name: "empty arg value", + arg: []string{"-c="}, found: true, - what: "empty arg value", }, { - def: "def", - input: []string{"-c="}, + name: "empty arg value override default", + fields: fields{ + DefaultConfig: "def", + }, + arg: []string{"-c="}, found: true, - what: "empty arg value override default", }, { - def: "def", - input: []string{"-c"}, + fields: fields{ + DefaultConfig: "def", + }, + arg: []string{"-c"}, found: false, - what: "invalid args always return no value", + name: "invalid args always return no value", }, { - def: "def", - input: []string{"-c", "value"}, - configFile: "value", - found: true, - what: "value override default", + fields: fields{ + DefaultConfig: "def", + }, + arg: []string{"-c", "value"}, + want: "value", + found: true, + name: "value override default", }, { - def: "def", - configFile: "def", - found: false, - what: "default gets used when nothing is passed", + fields: fields{ + DefaultConfig: "def", + }, + want: "def", + found: false, + name: "default gets used when nothing is passed", }, { - def: "def", - input: []string{"-c", "value"}, - env: "env", - configFile: "env", - found: true, - what: "env override args", + name: "env override args", + fields: fields{ + DefaultConfig: "def", + env: "env", + }, + arg: []string{"-c", "value"}, + want: "env", + found: true, }, { - def: "def", - input: []string{"before", "-c", "value", "after"}, - configFile: "value", - found: true, - what: "garbage in start and end", + name: "garbage in start and end", + fields: fields{ + DefaultConfig: "def", + }, + arg: []string{"before", "-c", "value", "after"}, + want: "value", + found: true, }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := Parser{ + FlagNames: []string{"--config", "-c"}, + EnvName: "_TEST_FLAG_ENV", + DefaultConfig: tt.fields.DefaultConfig, + } + // Setup + defer os.Unsetenv(tt.fields.env) + os.Setenv(p.EnvName, tt.fields.env) - for _, testCase := range testCases { - p := Parser{ - FlagNames: []string{"--config", "-c"}, - EnvName: "_TEST_FLAG_ENV", - DefaultConfig: testCase.def, - } - os.Setenv(p.EnvName, testCase.env) - configFile, found := p.findConfigFileFlag(testCase.input) - assert.Equal(t, testCase.configFile, configFile, testCase.what) - assert.Equal(t, testCase.found, found, testCase.what) + got, found := p.findConfigFileFlag(tt.arg) + if got != tt.want { + t.Errorf("Parser.findConfigFileFlag() got = %+v\nWant = %+v", got, tt.want) + } + if found != tt.found { + t.Errorf("Parser.findConfigFileFlag() found = %+v\nWant = %+v", found, tt.found) + } + }) } } -func TestParse(t *testing.T) { +func TestParser_Parse(t *testing.T) { testDataOutput := []string{ "--foo-bar=bar-foo", "--a-slice=1", @@ -170,71 +198,78 @@ func TestParse(t *testing.T) { "--e-slice=one", "--e-slice=two", } - - defParser := Parser{ - After: []string{"server", "agent"}, - FlagNames: []string{"-c", "--config"}, - EnvName: "_TEST_ENV", - DefaultConfig: "./testdata/data.yaml", + type fields struct { + After []string + FlagNames []string + EnvName string + DefaultConfig string } - - testCases := []struct { - parser Parser - env string - input []string - output []string - err string - what string + tests := []struct { + name string + fields fields + arg []string + want []string + wantErr bool }{ { - parser: defParser, - what: "default case", + name: "default case", + fields: fields{ + After: []string{"server", "agent"}, + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/data.yaml", + }, }, { - parser: defParser, - input: []string{"server"}, - output: append([]string{"server"}, testDataOutput...), - what: "read config file when not specified", + name: "read config file when not specified", + fields: fields{ + After: []string{"server", "agent"}, + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/data.yaml", + }, + arg: []string{"server"}, + want: append([]string{"server"}, testDataOutput...), }, { - parser: Parser{ + name: "ignore missing config when not set", + fields: fields{ After: []string{"server", "agent"}, FlagNames: []string{"-c", "--config"}, DefaultConfig: "missing", }, - input: []string{"server"}, - output: []string{"server"}, - what: "ignore missing config when not set", + arg: []string{"server"}, + want: []string{"server"}, }, { - parser: Parser{ + name: "fail when missing config", + fields: fields{ After: []string{"server", "agent"}, FlagNames: []string{"-c", "--config"}, DefaultConfig: "missing", }, - input: []string{"server", "-c=missing"}, - output: []string{"server", "-c=missing"}, - what: "fail when missing config", - err: "stat missing: no such file or directory", + arg: []string{"server", "-c=missing"}, + wantErr: true, }, { - parser: Parser{ + name: "read config file", + fields: fields{ After: []string{"server", "agent"}, FlagNames: []string{"-c", "--config"}, DefaultConfig: "missing", }, - input: []string{"before", "server", "before", "-c", "./testdata/data.yaml", "after"}, - output: append(append([]string{"before", "server"}, testDataOutput...), "before", "-c", "./testdata/data.yaml", "after"), - what: "read config file", + arg: []string{"before", "server", "before", "-c", "./testdata/data.yaml", "after"}, + want: append(append([]string{"before", "server"}, testDataOutput...), "before", "-c", "./testdata/data.yaml", "after"), }, { - parser: Parser{ + name: "read single config file", + fields: fields{ After: []string{"server", "agent"}, FlagNames: []string{"-c", "--config"}, DefaultConfig: "missing", }, - input: []string{"before", "server", "before", "-c", "./testdata/data.yaml.d/02-data.yaml", "after"}, - output: []string{"before", "server", + arg: []string{"before", "server", "before", "-c", "./testdata/data.yaml.d/02-data.yaml", "after"}, + want: []string{"before", "server", "--foo-bar=bar-foo", "--b-string=two", "--c-slice=three", @@ -243,20 +278,104 @@ func TestParse(t *testing.T) { "--e-slice=one", "--e-slice=two", "before", "-c", "./testdata/data.yaml.d/02-data.yaml", "after"}, - what: "read single config file", }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &Parser{ + After: tt.fields.After, + FlagNames: tt.fields.FlagNames, + EnvName: tt.fields.EnvName, + DefaultConfig: tt.fields.DefaultConfig, + } - for _, testCase := range testCases { - os.Setenv(testCase.parser.EnvName, testCase.env) - output, err := testCase.parser.Parse(testCase.input) - if err == nil { - assert.Equal(t, testCase.err, "", testCase.what) - } else { - assert.Equal(t, testCase.err, err.Error(), testCase.what) - } - if testCase.err == "" { - assert.Equal(t, testCase.output, output, testCase.what) - } + got, err := p.Parse(tt.arg) + if (err != nil) != tt.wantErr { + t.Errorf("Parser.Parse() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Parser.Parse() = %+v\nWant = %+v", got, tt.want) + } + }) + } +} + +func TestParser_FindString(t *testing.T) { + type fields struct { + After []string + FlagNames []string + EnvName string + DefaultConfig string + } + type args struct { + osArgs []string + target string + } + tests := []struct { + name string + fields fields + args args + want string + wantErr bool + }{ + { + name: "Default config does not exist", + fields: fields{ + After: []string{"server", "agent"}, + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "missing", + }, + args: args{ + osArgs: []string{}, + target: "", + }, + want: "", + }, + { + name: "A custom config yaml exists, target exists", + fields: fields{ + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/data.yaml", + }, + args: args{ + osArgs: []string{"-c", "./testdata/data.yaml"}, + target: "foo-bar", + }, + want: "baz", + }, + { + name: "A custom config yaml exists, target does not exist", + fields: fields{ + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/data.yaml", + }, + args: args{ + osArgs: []string{"-c", "./testdata/data.yaml"}, + target: "tls", + }, + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &Parser{ + After: tt.fields.After, + FlagNames: tt.fields.FlagNames, + EnvName: tt.fields.EnvName, + DefaultConfig: tt.fields.DefaultConfig, + } + got, err := p.FindString(tt.args.osArgs, tt.args.target) + if (err != nil) != tt.wantErr { + t.Errorf("Parser.FindString() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("Parser.FindString() = %+v\nWant = %+v", got, tt.want) + } + }) } } diff --git a/pkg/daemons/config/types_test.go b/pkg/daemons/config/types_test.go index 15780b090f..71fa8f7a73 100644 --- a/pkg/daemons/config/types_test.go +++ b/pkg/daemons/config/types_test.go @@ -6,34 +6,53 @@ import ( ) func TestGetArgsList(t *testing.T) { - argsMap := map[string]string{ - "aaa": "A", - "bbb": "B", - "ccc": "C", - "ddd": "d", - "eee": "e", - "fff": "f", - "ggg": "g", - "hhh": "h", + type args struct { + argsMap map[string]string + extraArgs []string } - extraArgs := []string{ - "bbb=BB", - "ddd=DD", - "iii=II", - } - expected := []string{ - "--aaa=A", - "--bbb=BB", - "--ccc=C", - "--ddd=DD", - "--eee=e", - "--fff=f", - "--ggg=g", - "--hhh=h", - "--iii=II", + tests := []struct { + name string + args args + want []string + }{ + { + name: "Default Test", + args: args{ + argsMap: map[string]string{ + "aaa": "A", + "bbb": "B", + "ccc": "C", + "ddd": "d", + "eee": "e", + "fff": "f", + "ggg": "g", + "hhh": "h", + }, + extraArgs: []string{ + "bbb=BB", + "ddd=DD", + "iii=II", + }, + }, + + want: []string{ + "--aaa=A", + "--bbb=BB", + "--ccc=C", + "--ddd=DD", + "--eee=e", + "--fff=f", + "--ggg=g", + "--hhh=h", + "--iii=II", + }, + }, } - actual := GetArgsList(argsMap, extraArgs) - if !reflect.DeepEqual(actual, expected) { - t.Errorf("got %v\nwant %v", actual, expected) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetArgsList(tt.args.argsMap, tt.args.extraArgs); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetArgsList() = %+v\nWant = %+v", got, tt.want) + } + }) } } diff --git a/pkg/nodeconfig/nodeconfig_test.go b/pkg/nodeconfig/nodeconfig_test.go index 823ee08cf7..af6935ec16 100644 --- a/pkg/nodeconfig/nodeconfig_test.go +++ b/pkg/nodeconfig/nodeconfig_test.go @@ -19,6 +19,7 @@ var FakeNodeWithNoAnnotation = &corev1.Node{ }, } +var TestEnvName = version.ProgramUpper + "_NODE_NAME" var FakeNodeWithAnnotation = &corev1.Node{ TypeMeta: metav1.TypeMeta{ Kind: "Node", @@ -28,40 +29,12 @@ var FakeNodeWithAnnotation = &corev1.Node{ Name: "fakeNode-with-annotation", Annotations: map[string]string{ NodeArgsAnnotation: `["server","--no-flannel"]`, - NodeEnvAnnotation: `{"` + version.ProgramUpper + `_NODE_NAME":"fakeNode-with-annotation"}`, + NodeEnvAnnotation: `{"` + TestEnvName + `":"fakeNode-with-annotation"}`, NodeConfigHashAnnotation: "LNQOAOIMOQIBRMEMACW7LYHXUNPZADF6RFGOSPIHJCOS47UVUJAA====", }, }, } -func assertEqual(t *testing.T, a interface{}, b interface{}) { - if a != b { - t.Fatalf("[ %v != %v ]", a, b) - } -} - -func TestSetEmptyNodeConfigAnnotations(t *testing.T) { - os.Args = []string{version.Program, "server", "--no-flannel"} - os.Setenv(version.ProgramUpper+"_NODE_NAME", "fakeNode-no-annotation") - nodeUpdated, err := SetNodeConfigAnnotations(FakeNodeWithNoAnnotation) - if err != nil { - t.Fatalf("Failed to set node config annotation: %v", err) - } - assertEqual(t, true, nodeUpdated) - - expectedArgs := `["server","--no-flannel"]` - actualArgs := FakeNodeWithNoAnnotation.Annotations[NodeArgsAnnotation] - assertEqual(t, expectedArgs, actualArgs) - - expectedEnv := `{"` + version.ProgramUpper + `_NODE_NAME":"fakeNode-no-annotation"}` - actualEnv := FakeNodeWithNoAnnotation.Annotations[NodeEnvAnnotation] - assertEqual(t, expectedEnv, actualEnv) - - expectedHash := "MROOIJGRXUZ53BM74K76TZLRXQOLNNBNJBJOY7JJ22EAEUIBW7YA====" - actualHash := FakeNodeWithNoAnnotation.Annotations[NodeConfigHashAnnotation] - assertEqual(t, expectedHash, actualHash) -} - func TestSetExistingNodeConfigAnnotations(t *testing.T) { // adding same config os.Args = []string{version.Program, "server", "--no-flannel"} @@ -70,18 +43,79 @@ func TestSetExistingNodeConfigAnnotations(t *testing.T) { if err != nil { t.Fatalf("Failed to set node config annotation: %v", err) } - assertEqual(t, false, nodeUpdated) + if nodeUpdated { + t.Errorf("TestSetExistingNodeConfigAnnotations() expected false") + } } -func TestSetArgsWithEqual(t *testing.T) { - os.Args = []string{version.Program, "server", "--no-flannel", "--write-kubeconfig-mode=777"} - os.Setenv("K3S_NODE_NAME", "fakeNode-with-no-annotation") - nodeUpdated, err := SetNodeConfigAnnotations(FakeNodeWithNoAnnotation) - if err != nil { - t.Fatalf("Failed to set node config annotation: %v", err) +func Test_SetNodeConfigAnnotations(t *testing.T) { + type args struct { + node *corev1.Node + osArgs []string + } + setup := func(osArgs []string) error { + os.Args = osArgs + return os.Setenv(TestEnvName, "fakeNode-with-no-annotation") + } + teardown := func() error { + return os.Unsetenv(TestEnvName) + } + tests := []struct { + name string + args args + want bool + wantErr bool + wantNodeArgs string + wantNodeEnv string + wantNodeConfigHash string + }{ + { + name: "Set empty NodeConfigAnnotations", + args: args{ + node: FakeNodeWithAnnotation, + osArgs: []string{version.Program, "server", "--no-flannel"}, + }, + want: true, + wantNodeArgs: `["server","--no-flannel"]`, + wantNodeEnv: `{"` + TestEnvName + `":"fakeNode-with-no-annotation"}`, + wantNodeConfigHash: "FBV4UQYLF2N7NH7EK42GKOTU5YA24TXB4WAYZHA5ZOFNGZHC4ZPA====", + }, + { + name: "Set args with equal", + args: args{ + node: FakeNodeWithNoAnnotation, + osArgs: []string{version.Program, "server", "--no-flannel", "--write-kubeconfig-mode=777"}, + }, + want: true, + wantNodeArgs: `["server","--no-flannel","--write-kubeconfig-mode","777"]`, + wantNodeEnv: `{"` + TestEnvName + `":"fakeNode-with-no-annotation"}`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer teardown() + if err := setup(tt.args.osArgs); err != nil { + t.Errorf("Setup for SetNodeConfigAnnotations() failed = %v", err) + return + } + got, err := SetNodeConfigAnnotations(tt.args.node) + if (err != nil) != tt.wantErr { + t.Errorf("SetNodeConfigAnnotations() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("SetNodeConfigAnnotations() = %+v\nWantRes = %+v", got, tt.want) + } + nodeAnn := tt.args.node.Annotations + if nodeAnn[NodeArgsAnnotation] != tt.wantNodeArgs { + t.Errorf("SetNodeConfigAnnotations() = %+v\nWantAnn.nodeArgs = %+v", nodeAnn[NodeArgsAnnotation], tt.wantNodeArgs) + } + if nodeAnn[NodeEnvAnnotation] != tt.wantNodeEnv { + t.Errorf("SetNodeConfigAnnotations() = %+v\nWantAnn.nodeEnv = %+v", nodeAnn[NodeEnvAnnotation], tt.wantNodeEnv) + } + if tt.wantNodeConfigHash != "" && nodeAnn[NodeConfigHashAnnotation] != tt.wantNodeConfigHash { + t.Errorf("SetNodeConfigAnnotations() = %+v\nWantAnn.nodeConfigHash = %+v", nodeAnn[NodeConfigHashAnnotation], tt.wantNodeConfigHash) + } + }) } - assertEqual(t, true, nodeUpdated) - expectedArgs := `["server","--no-flannel","--write-kubeconfig-mode","777"]` - actualArgs := FakeNodeWithNoAnnotation.Annotations[NodeArgsAnnotation] - assertEqual(t, expectedArgs, actualArgs) }