From 67679ebf5349d0055fcaf6fc3c30c2520c318201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Thu, 23 Dec 2021 22:01:00 +0100 Subject: [PATCH 1/7] Ensure the `environment.InitFlags()` is called to avoid confusion. Also, moving code to TestMain() to avoid side effects, while importing a package. --- README.md | 6 ++---- pkg/environment/magic.go | 9 +++++++-- pkg/feature/level.go | 10 ++++++++++ pkg/feature/states.go | 10 ++++++++++ test/e2e/main_test.go | 8 +++----- test/example/main_test.go | 8 +++----- 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 7a8dd2d0..5c9bf651 100644 --- a/README.md +++ b/README.md @@ -51,13 +51,11 @@ import ( // config as well as the parsing level and state flags. var global environment.GlobalEnvironment -func init() { +// TestMain is the first entry point for `go test`. +func TestMain(m *testing.M) { // environment.InitFlags registers state and level filter flags. environment.InitFlags(flag.CommandLine) -} -// TestMain is the first entry point for `go test`. -func TestMain(m *testing.M) { // We get a chance to parse flags to include the framework flags for the // framework as well as any additional flags included in the integration. flag.Parse() diff --git a/pkg/environment/magic.go b/pkg/environment/magic.go index aa7ffaf0..d7571891 100644 --- a/pkg/environment/magic.go +++ b/pkg/environment/magic.go @@ -19,7 +19,6 @@ package environment import ( "context" "errors" - "fmt" "regexp" "testing" "time" @@ -35,7 +34,13 @@ import ( ) func NewGlobalEnvironment(ctx context.Context) GlobalEnvironment { - fmt.Printf("level %s, state %s, feature %s\n\n", l, s, *f) + log := logging.FromContext(ctx) + if l.Valid() && s.Valid() { + log.Infof("Global environment settings: level %s, state %s, feature %#v", l, s, *f) + } else { + log.Fatal("Flags are not initialized properly. Ensure to call " + + "environment.InitFlags() func.") + } return &MagicGlobalEnvironment{ c: ctx, diff --git a/pkg/feature/level.go b/pkg/feature/level.go index ff3c26e4..7d53378d 100644 --- a/pkg/feature/level.go +++ b/pkg/feature/level.go @@ -71,6 +71,16 @@ func (l Levels) String() string { return b.String() } +func (l Levels) Valid() bool { + values := []Levels{Must, MustNot, Should, ShouldNot, May, All} + for _, value := range values { + if l == value { + return true + } + } + return false +} + var LevelMapping = map[Levels]string{ Must: "MUST", MustNot: "MUST NOT", diff --git a/pkg/feature/states.go b/pkg/feature/states.go index 899f0449..0efa787d 100644 --- a/pkg/feature/states.go +++ b/pkg/feature/states.go @@ -60,6 +60,16 @@ func (s States) String() string { return b.String() } +func (s States) Valid() bool { + values := []States{Alpha, Beta, Stable, Any} + for _, value := range values { + if s == value { + return true + } + } + return false +} + var StatesMapping = map[States]string{ Alpha: "Alpha", Beta: "Beta", diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index ca6876b8..4c94732a 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -38,13 +38,11 @@ import ( // config as well as the parsing level and state flags. var global environment.GlobalEnvironment -func init() { +// TestMain is the first entry point for `go test`. +func TestMain(m *testing.M) { // environment.InitFlags registers state, level and feature filter flags. environment.InitFlags(flag.CommandLine) -} -// TestMain is the first entry point for `go test`. -func TestMain(m *testing.M) { // We get a chance to parse flags to include the framework flags for the // framework as well as any additional flags included in the integration. flag.Parse() @@ -53,7 +51,7 @@ func TestMain(m *testing.M) { // testing framework for namespace management, and could be leveraged by // features to pull Kubernetes clients or the test environment out of the // context passed in the features. - ctx, startInformers := injection.EnableInjectionOrDie(nil, nil) //nolint + ctx, startInformers := injection.EnableInjectionOrDie(nil, nil) // nolint startInformers() // global is used to make instances of Environments, NewGlobalEnvironment diff --git a/test/example/main_test.go b/test/example/main_test.go index 396eeb58..7c10f823 100644 --- a/test/example/main_test.go +++ b/test/example/main_test.go @@ -38,13 +38,11 @@ import ( // config as well as the parsing level and state flags. var global environment.GlobalEnvironment -func init() { +// TestMain is the first entry point for `go test`. +func TestMain(m *testing.M) { // environment.InitFlags registers state and level filter flags. environment.InitFlags(flag.CommandLine) -} -// TestMain is the first entry point for `go test`. -func TestMain(m *testing.M) { // We get a chance to parse flags to include the framework flags for the // framework as well as any additional flags included in the integration. flag.Parse() @@ -53,7 +51,7 @@ func TestMain(m *testing.M) { // testing framework for namespace management, and could be leveraged by // features to pull Kubernetes clients or the test environment out of the // context passed in the features. - ctx, startInformers := injection.EnableInjectionOrDie(nil, nil) //nolint + ctx, startInformers := injection.EnableInjectionOrDie(nil, nil) // nolint startInformers() // global is used to make instances of Environments, NewGlobalEnvironment From 04b12185221c42e9c0784d7e088412da83c07f00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Tue, 4 Jan 2022 12:31:27 +0100 Subject: [PATCH 2/7] Bump Github CI From 01b3c2514a931156bef18f6d9364f4df9a03a97e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Tue, 11 Jan 2022 12:01:23 +0100 Subject: [PATCH 3/7] Bump Github CI From fb1418c216fa57e4ee18d16a4cc1e2969bdc4a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Tue, 11 Jan 2022 20:42:49 +0100 Subject: [PATCH 4/7] Work for all bit mask combinations --- pkg/feature/level.go | 16 ++++-- pkg/feature/level_test.go | 114 +++++++++++++++++++++++++++++++++++++ pkg/feature/states.go | 15 +++-- pkg/feature/states_test.go | 112 ++++++++++++++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 12 deletions(-) create mode 100644 pkg/feature/level_test.go create mode 100644 pkg/feature/states_test.go diff --git a/pkg/feature/level.go b/pkg/feature/level.go index 7d53378d..256b9723 100644 --- a/pkg/feature/level.go +++ b/pkg/feature/level.go @@ -17,6 +17,7 @@ limitations under the License. package feature import ( + "fmt" "strings" ) @@ -53,6 +54,9 @@ func (l Levels) String() string { if l == All { return "All" } + if !l.Valid() { + return fmt.Sprintf("Invalid(%d)", uint8(l)) + } var b strings.Builder @@ -71,14 +75,14 @@ func (l Levels) String() string { return b.String() } +// Valid checks if given level is valid (not empty combination of Levels values). func (l Levels) Valid() bool { - values := []Levels{Must, MustNot, Should, ShouldNot, May, All} - for _, value := range values { - if l == value { - return true - } + val := uint8(l) + if val == 0 { + return false } - return false + val &^= uint8(All) + return val == 0 } var LevelMapping = map[Levels]string{ diff --git a/pkg/feature/level_test.go b/pkg/feature/level_test.go new file mode 100644 index 00000000..1ce24576 --- /dev/null +++ b/pkg/feature/level_test.go @@ -0,0 +1,114 @@ +package feature_test + +/* +Copyright 2022 The Knative 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. +*/ + +import ( + "testing" + + "knative.dev/reconciler-test/pkg/feature" +) + +func TestLevel(t *testing.T) { + t.Parallel() + + for _, tc := range levelTestCases() { + tc := tc + t.Run(tc.name(), tc.run) + } +} + +func (tc levelTestCase) run(t *testing.T) { + got := tc.level.Valid() + if tc.invalid == got { + t.Errorf("want %t, got %t", !tc.invalid, got) + } +} + +func levelTestCases() []levelTestCase { + levels := []feature.Levels{ + feature.Must, + feature.MustNot, + feature.Should, + feature.ShouldNot, + feature.May, + } + levels = levelBitwiseOr(levelPowerset(levels)) + empty := asLevelTestCases(false, feature.Levels(0)) + valid := asLevelTestCases(true, levels...) + invalid := asLevelTestCases(false, + feature.Levels(126), feature.Levels(127), feature.Levels(128)) + + cases := make([]levelTestCase, 0, len(empty)+len(valid)+len(invalid)) + cases = append(cases, empty...) + cases = append(cases, valid...) + cases = append(cases, invalid...) + return cases +} + +// levelPowerset returns all combinations for a given array. +func levelPowerset(set []feature.Levels) (subsets [][]feature.Levels) { + length := uint(len(set)) + + // Go through all possible combinations of objects + // from 1 (only first object in subset) to 2^length (all objects in subset) + for subsetBits := 1; subsetBits < (1 << length); subsetBits++ { + var subset []feature.Levels + + for object := uint(0); object < length; object++ { + // checks if object is contained in subset + // by checking if bit 'object' is set in subsetBits + if (subsetBits>>object)&1 == 1 { + // add object to subset + subset = append(subset, set[object]) + } + } + // add subset to subsets + subsets = append(subsets, subset) + } + return subsets +} + +func levelBitwiseOr(subsets [][]feature.Levels) []feature.Levels { + levels := make([]feature.Levels, len(subsets)) + for i, subset := range subsets { + val := 0 + for _, level := range subset { + val |= int(level) + } + levels[i] = feature.Levels(val) + } + return levels +} + +func asLevelTestCases(valid bool, levels ...feature.Levels) []levelTestCase { + cases := make([]levelTestCase, len(levels)) + for i, level := range levels { + cases[i] = levelTestCase{ + level: level, invalid: !valid, + } + } + return cases +} + +type levelTestCase struct { + level feature.Levels + invalid bool +} + +func (tc levelTestCase) name() string { + return tc.level.String() +} diff --git a/pkg/feature/states.go b/pkg/feature/states.go index 0efa787d..a58e758d 100644 --- a/pkg/feature/states.go +++ b/pkg/feature/states.go @@ -17,6 +17,7 @@ limitations under the License. package feature import ( + "fmt" "strings" ) @@ -43,6 +44,9 @@ func (s States) String() string { if s == Any { return "Any" } + if !s.Valid() { + return fmt.Sprintf("Invalid(%d)", uint8(s)) + } var b strings.Builder @@ -61,13 +65,12 @@ func (s States) String() string { } func (s States) Valid() bool { - values := []States{Alpha, Beta, Stable, Any} - for _, value := range values { - if s == value { - return true - } + val := uint8(s) + if val == 0 { + return false } - return false + val &^= uint8(All) + return val == 0 } var StatesMapping = map[States]string{ diff --git a/pkg/feature/states_test.go b/pkg/feature/states_test.go new file mode 100644 index 00000000..3c84c6f2 --- /dev/null +++ b/pkg/feature/states_test.go @@ -0,0 +1,112 @@ +package feature_test + +import ( + "testing" + + "knative.dev/reconciler-test/pkg/feature" +) + +/* +Copyright 2022 The Knative 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. +*/ + +func TestState(t *testing.T) { + t.Parallel() + + for _, tc := range stateTestCases() { + tc := tc + t.Run(tc.name(), tc.run) + } +} + +func (tc stateTestCase) run(t *testing.T) { + got := tc.state.Valid() + if tc.invalid == got { + t.Errorf("want %t, got %t", !tc.invalid, got) + } +} + +func stateTestCases() []stateTestCase { + states := []feature.States{ + feature.Alpha, + feature.Beta, + feature.Stable, + } + states = stateBitwiseOr(statePowerset(states)) + empty := asStateTestCases(false, feature.States(0)) + valid := asStateTestCases(true, states...) + invalid := asStateTestCases(false, + feature.States(126), feature.States(127), feature.States(128)) + + cases := make([]stateTestCase, 0, len(empty)+len(valid)+len(invalid)) + cases = append(cases, empty...) + cases = append(cases, valid...) + cases = append(cases, invalid...) + return cases +} + +// statePowerset returns all combinations for a given array. +func statePowerset(set []feature.States) (subsets [][]feature.States) { + length := uint(len(set)) + + // Go through all possible combinations of objects + // from 1 (only first object in subset) to 2^length (all objects in subset) + for subsetBits := 1; subsetBits < (1 << length); subsetBits++ { + var subset []feature.States + + for object := uint(0); object < length; object++ { + // checks if object is contained in subset + // by checking if bit 'object' is set in subsetBits + if (subsetBits>>object)&1 == 1 { + // add object to subset + subset = append(subset, set[object]) + } + } + // add subset to subsets + subsets = append(subsets, subset) + } + return subsets +} + +func stateBitwiseOr(subsets [][]feature.States) []feature.States { + levels := make([]feature.States, len(subsets)) + for i, subset := range subsets { + val := 0 + for _, state := range subset { + val |= int(state) + } + levels[i] = feature.States(val) + } + return levels +} + +func asStateTestCases(valid bool, states ...feature.States) []stateTestCase { + cases := make([]stateTestCase, len(states)) + for i, state := range states { + cases[i] = stateTestCase{ + state: state, invalid: !valid, + } + } + return cases +} + +type stateTestCase struct { + state feature.States + invalid bool +} + +func (tc stateTestCase) name() string { + return tc.state.String() +} From c19911844a93855b27a749dd0c39c76033a7027d Mon Sep 17 00:00:00 2001 From: Chris Suszynski Date: Thu, 2 Mar 2023 11:24:21 +0100 Subject: [PATCH 5/7] include current vals in failure Co-authored-by: Pierangelo Di Pilato --- pkg/environment/magic.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/environment/magic.go b/pkg/environment/magic.go index 4adae142..f7179c2e 100644 --- a/pkg/environment/magic.go +++ b/pkg/environment/magic.go @@ -194,7 +194,7 @@ func (mr *MagicGlobalEnvironment) Environment(opts ...EnvOpts) (context.Context, env.l, env.s, env.featureMatch) } else { log.Fatal("Flags are not initialized properly. Ensure to call " + - "environment.InitFlags() func.") + "environment.InitFlags() func, level %s, state %s", env.l.String(), env.s.String()) } mr.initializersOnce.Do(func() { From 7bd94c50adc57fe32382d96e41551379071d9154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Thu, 2 Mar 2023 12:32:36 +0100 Subject: [PATCH 6/7] Explain the test code --- pkg/feature/level_test.go | 4 ++++ pkg/feature/states_test.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/pkg/feature/level_test.go b/pkg/feature/level_test.go index 1ce24576..f2aa8b36 100644 --- a/pkg/feature/level_test.go +++ b/pkg/feature/level_test.go @@ -32,12 +32,16 @@ func TestLevel(t *testing.T) { } func (tc levelTestCase) run(t *testing.T) { + t.Parallel() got := tc.level.Valid() if tc.invalid == got { t.Errorf("want %t, got %t", !tc.invalid, got) } } +// levelTestCases returns all possible bitwise OR combinations of +// feature.Levels: valid ones, invalid, and empty. For example: `Must|Should`, +// `Must|MustNot|Should|ShouldNot|May`, or `Invalid(128)` etc. func levelTestCases() []levelTestCase { levels := []feature.Levels{ feature.Must, diff --git a/pkg/feature/states_test.go b/pkg/feature/states_test.go index 3c84c6f2..13dd5e8c 100644 --- a/pkg/feature/states_test.go +++ b/pkg/feature/states_test.go @@ -32,12 +32,16 @@ func TestState(t *testing.T) { } func (tc stateTestCase) run(t *testing.T) { + t.Parallel() got := tc.state.Valid() if tc.invalid == got { t.Errorf("want %t, got %t", !tc.invalid, got) } } +// stateTestCases returns all possible bitwise OR combinations of +// feature.States: valid ones, invalid, and empty. For example: `Alpha|Stable`, +// `Alpha|Beta|Stable`, etc. func stateTestCases() []stateTestCase { states := []feature.States{ feature.Alpha, From 2242340334a7ba2472481c1201b384e9b65ca52e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Thu, 2 Mar 2023 13:06:15 +0100 Subject: [PATCH 7/7] Reformat --- pkg/environment/magic.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/environment/magic.go b/pkg/environment/magic.go index 0863da04..4888950d 100644 --- a/pkg/environment/magic.go +++ b/pkg/environment/magic.go @@ -208,8 +208,9 @@ func (mr *MagicGlobalEnvironment) Environment(opts ...EnvOpts) (context.Context, log.Infof("Environment settings: level %s, state %s, feature %q", env.l, env.s, env.featureMatch) } else { - log.Fatal("Flags are not initialized properly. Ensure to call " + - "environment.InitFlags() func, level %s, state %s", env.l.String(), env.s.String()) + log.Fatal("Flags are not initialized properly. Ensure to call "+ + "environment.InitFlags() func, level %s, state %s", + env.l.String(), env.s.String()) } mr.initializersOnce.Do(func() {