From 6a6dc69b6e1e764061bad8ec52c17aa4387aff25 Mon Sep 17 00:00:00 2001 From: chankyin Date: Tue, 17 Dec 2024 18:01:53 +0800 Subject: [PATCH] test(util/component): add unit tests for component deduplication --- util/cmd/cmd.go | 6 +- util/cmd/cmd_test.go | 173 ++++++++++++++++++++++++++++++++++++ util/component/component.go | 77 ++++++++-------- 3 files changed, 218 insertions(+), 38 deletions(-) create mode 100644 util/cmd/cmd_test.go diff --git a/util/cmd/cmd.go b/util/cmd/cmd.go index 5447690..a819e3a 100644 --- a/util/cmd/cmd.go +++ b/util/cmd/cmd.go @@ -120,12 +120,16 @@ func tryRun(requests []func(*component.DepRequests)) error { // and does not block until shutdown. // Returns as soon as startup completes to allow the caller to orchestrate integration tests. func MockStartup(ctx context.Context, requests []func(*component.DepRequests)) component.ApiMap { + return MockStartupWithCliArgs(ctx, requests, []string{}) +} + +func MockStartupWithCliArgs(ctx context.Context, requests []func(*component.DepRequests), cliArgs []string) component.ApiMap { components := component.ResolveList(requests) fs := new(pflag.FlagSet) setupFlags(components, fs) - if err := fs.Parse([]string{}); err != nil { + if err := fs.Parse(cliArgs); err != nil { panic(err) } diff --git a/util/cmd/cmd_test.go b/util/cmd/cmd_test.go new file mode 100644 index 0000000..6b421a4 --- /dev/null +++ b/util/cmd/cmd_test.go @@ -0,0 +1,173 @@ +// Copyright 2024 The Podseidon 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 cmd_test + +import ( + "context" + "flag" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/utils/ptr" + + "github.com/kubewharf/podseidon/util/cmd" + "github.com/kubewharf/podseidon/util/component" + "github.com/kubewharf/podseidon/util/util" +) + +var TestingT = component.Declare( + func(_ *testing.T) string { return "testing" }, + func(_ *testing.T, _ *flag.FlagSet) util.Empty { return util.Empty{} }, + func(_ *testing.T, _ *component.DepRequests) util.Empty { return util.Empty{} }, + func(_ context.Context, _ *testing.T, _ util.Empty, _ util.Empty) (*util.Empty, error) { + return &util.Empty{}, nil + }, + component.Lifecycle[*testing.T, util.Empty, util.Empty, util.Empty]{ + Start: nil, + Join: nil, + HealthChecks: nil, + }, + func(d *component.Data[*testing.T, util.Empty, util.Empty, util.Empty]) *testing.T { return d.Args }, +) + +var Counter = component.Declare[CounterArgs, CounterOptions, CounterDeps, CounterState, CounterApi]( + func(args CounterArgs) string { return args.Name }, + func(args CounterArgs, fs *flag.FlagSet) CounterOptions { + return CounterOptions{ + Multiplier: fs.Int("multiplier", 10, ""), + ExpectMutableField: fs.String("expect", "", ""), + } + }, + func(_ CounterArgs, reqs *component.DepRequests) CounterDeps { + return CounterDeps{Testing: component.DepPtr(reqs, TestingT(nil))} + }, + func(_ context.Context, args CounterArgs, options CounterOptions, deps CounterDeps) (*CounterState, error) { + return &CounterState{}, nil + }, + component.Lifecycle[CounterArgs, CounterOptions, CounterDeps, CounterState]{ + Start: func(_ context.Context, args *CounterArgs, options *CounterOptions, deps *CounterDeps, state *CounterState) error { + assert.Equalf( + deps.Testing.Get(), + *options.ExpectMutableField, + strings.Join(state.mutableField, ","), + "start is called strictly after all init", + ) + return nil + }, + Join: nil, + HealthChecks: nil, + }, + func(d *component.Data[CounterArgs, CounterOptions, CounterDeps, CounterState]) CounterApi { + return CounterApi{multiplier: *d.Options.Multiplier, state: d.State} + }, +) + +type CounterArgs struct { + Name string +} + +type CounterOptions struct { + Multiplier *int + ExpectMutableField *string +} + +type CounterDeps struct { + Testing component.Dep[*testing.T] +} + +type CounterState struct { + mutableField []string +} + +type CounterApi struct { + multiplier int + state *CounterState +} + +func (api CounterApi) Update(value int) { + api.state.mutableField = append(api.state.mutableField, fmt.Sprint(api.multiplier*value)) +} + +// the pointer is intentionally added to trigger panic when it is unused despite expected to be used. +func Mutator(mutatorArgs MutatorArgs, counterDepName *string, mutatorDepNames *[]string) component.Declared[util.Empty] { + return component.Declare( + func(args MutatorArgs) string { return args.Name }, + func(_ MutatorArgs, fs *flag.FlagSet) MutatorOptions { + return MutatorOptions{Value: fs.Int("value", 0, "")} + }, + func(_ MutatorArgs, reqs *component.DepRequests) MutatorDeps { + counterDep := component.DepPtr(reqs, Counter(CounterArgs{Name: *counterDepName})) + for _, mutatorDepName := range *mutatorDepNames { + component.DepPtr(reqs, Mutator(MutatorArgs{Name: mutatorDepName}, nil, nil)) + } + + return MutatorDeps{ + Counter: counterDep, + } + }, + func(_ context.Context, _ MutatorArgs, options MutatorOptions, deps MutatorDeps) (*util.Empty, error) { + deps.Counter.Get().Update(*options.Value) + return &util.Empty{}, nil + }, + component.Lifecycle[MutatorArgs, MutatorOptions, MutatorDeps, util.Empty]{ + Start: nil, + Join: nil, + HealthChecks: nil, + }, + func(d *component.Data[MutatorArgs, MutatorOptions, MutatorDeps, util.Empty]) util.Empty { + return util.Empty{} + }, + )(mutatorArgs) +} + +type MutatorArgs struct { + Name string +} + +type MutatorOptions struct { + Value *int +} + +type MutatorDeps struct { + Counter component.Dep[CounterApi] +} + +func TestDeduplication(t *testing.T) { + cmd.MockStartupWithCliArgs( + context.Background(), + []func(*component.DepRequests){ + component.RequireDep(TestingT(t).SkipFutureMerges()), + component.RequireDep(Mutator(MutatorArgs{ + Name: "mutator1", + }, ptr.To("counter1"), ptr.To[[]string](nil))), + component.RequireDep(Mutator(MutatorArgs{ + Name: "mutator2", + }, ptr.To("counter1"), ptr.To([]string{"mutator1"}))), + component.RequireDep(Mutator(MutatorArgs{ + Name: "mutator3", + }, ptr.To("counter2"), ptr.To([]string{"mutator1"}))), + }, + []string{ + "--counter1-multiplier=100", + "--counter1-expect=100,200", + "--counter2-expect=30", + "--mutator1-value=1", + "--mutator2-value=2", + "--mutator3-value=3", + }, + ) +} diff --git a/util/component/component.go b/util/component/component.go index c052d22..a5e5c05 100644 --- a/util/component/component.go +++ b/util/component/component.go @@ -83,8 +83,10 @@ var ErrRecursiveDependencies = errors.TagErrorf( // // This interface is only useful for lifecycle orchestration and should not be implemented by other packages. type Component interface { + getName() string + // Describes this component for dependency resolution. - manifest() manifest + depRequests() []*depRequest canBeMergedInto() bool // Updates the other component upon name duplication. @@ -106,11 +108,6 @@ type Component interface { RegisterHealthChecks(handler *healthz.Handler, onFail func(name string, err error)) } -type manifest struct { - Name string - Dependencies []*depRequest -} - // A registry of dependencies requested by components. type DepRequests struct { requests []*depRequest @@ -164,7 +161,7 @@ func DepPtr[Api any](requests *DepRequests, base Declared[Api]) Dep[Api] { if !ok { panic(fmt.Sprintf( "Components of types %T and %T declared the same name %q with incompatible APIs %T and %v", - comp, base, comp.manifest().Name, util.Type[Api]().Out(0), reflect.TypeOf(api).Out(0), + comp, base, comp.getName(), util.Type[Api]().Out(0), reflect.TypeOf(api).Out(0), )) } @@ -258,10 +255,10 @@ func resolveRequest( request *depRequest, ) (string, Component, any) { requestComp, requestApi := request.getNew() - manifest := requestComp.manifest() + name := requestComp.getName() // already exists, return previous value - if prev, hasPrev := componentMap[manifest.Name]; hasPrev { + if prev, hasPrev := componentMap[name]; hasPrev { deps := requestComp.mergeInto(prev.comp) // resolve incremental dependencies @@ -271,25 +268,25 @@ func resolveRequest( prev.deps.Insert(depName) } - return manifest.Name, prev.comp, prev.apiGetter + return name, prev.comp, prev.apiGetter } requestDeps := sets.New[string]() // new component; resolve dependencies, init and return the instance we got - for _, dep := range manifest.Dependencies { + for _, dep := range requestComp.depRequests() { depName, depComp, depApi := resolveRequest(componentMap, dep) dep.set(depComp, depApi) requestDeps.Insert(depName) } - componentMap[manifest.Name] = &componentMapEntry{ + componentMap[name] = &componentMapEntry{ comp: requestComp, apiGetter: requestApi, deps: requestDeps, } - return manifest.Name, requestComp, requestApi + return name, requestComp, requestApi } // Accessor to interact with components by name. @@ -391,18 +388,20 @@ func strictMergeIntoFn[Args any, Options any, Deps any, State any]( return []*depRequest{} } - if other, isValidType := other.(*componentImpl[Args, Options, Deps, State]); isValidType { - reqs := DepRequests{requests: nil} - onMerge(&other.Args, &other.Deps, &reqs) - return reqs.requests - } else { + otherTyped, isValidType := other.(*componentImpl[Args, Options, Deps, State]) + if !isValidType { panic(fmt.Sprintf( "cannot merge %q [%v, %v, %v, %v] into incompatible Component type %T", compName, util.Type[Args](), util.Type[Options](), util.Type[Deps](), util.Type[State](), - other, + otherTyped, )) } + + reqs := DepRequests{requests: nil} + onMerge(&otherTyped.Args, &otherTyped.Deps, &reqs) + + return reqs.requests } } @@ -437,13 +436,14 @@ func Declare[Args any, Options any, Deps any, State any, Api any]( Deps: util.Zero[Deps](), State: nil, }, - name: name, - optionsFn: newOptions, - depsFn: newDeps, - init: init, - lifecycle: lifecycle, - mergeIntoFn: strictMergeIntoFn[Args, Options, Deps, State](name, func(*Args, *Deps, *DepRequests) {}), - phase: nil, + name: name, + optionsFn: newOptions, + depsFn: newDeps, + init: init, + lifecycle: lifecycle, + mergeIntoFn: strictMergeIntoFn[Args, Options, Deps, State](name, func(*Args, *Deps, *DepRequests) {}), + skipFutureMerges: false, + phase: nil, } if start := impl.lifecycle.Start; start != nil { @@ -484,6 +484,7 @@ func (ctor DeclaredCtor[Args, Deps, Api]) SkipFutureMerges() DeclaredCtor[Args, return func(args Args) Declared[Api] { decl := ctor(args) decl.SkipFutureMerges() + return decl } } @@ -588,14 +589,15 @@ type Lifecycle[Args any, Options any, Deps any, State any] struct { // and returns a non-nil error for unready status. type HealthChecks map[string]func() error -func (impl *componentImpl[Args, Options, Deps, State]) manifest() manifest { +func (impl *componentImpl[Args, Options, Deps, State]) getName() string { + return impl.name +} + +func (impl *componentImpl[Args, Options, Deps, State]) depRequests() []*depRequest { deps := DepRequests{requests: []*depRequest{}} impl.Deps = impl.depsFn(impl.Args, &deps) - return manifest{ - Name: impl.name, - Dependencies: deps.requests, - } + return deps.requests } func (impl *componentImpl[Args, Options, Deps, State]) canBeMergedInto() bool { @@ -673,14 +675,15 @@ type emptyComponent struct { name string } -func (comp emptyComponent) manifest() manifest { - return manifest{ - Name: comp.name, - Dependencies: []*depRequest{}, - } +func (comp emptyComponent) getName() string { + return comp.name +} + +func (emptyComponent) depRequests() []*depRequest { + return nil } -func (comp emptyComponent) canBeMergedInto() bool { +func (emptyComponent) canBeMergedInto() bool { // do not merge into empty components since the implementation is exclusively determined by the test case return false }