From 5978bc5b7f58ea3a3dd8c06d5271ee45cc15f739 Mon Sep 17 00:00:00 2001 From: LandonTClipp Date: Sun, 5 Nov 2023 20:24:16 -0600 Subject: [PATCH] Fix test with config initialization The test wasn't properly initializing the viper object so the config wasn't getting decoded into the struct properly. Also adding more comments to various places to better explain what the code was doing, as even I, the person who wrote it, was confused as to what I was doing. How can I expect other people to understand it if I can't?! --- pkg/config/config.go | 28 +++++++++++++++++++++------- pkg/config/config_test.go | 23 ++++++++++++++--------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 7899fd9c..1682231f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -218,7 +218,7 @@ func (c *Config) getPackageConfigMap(ctx context.Context, packageName string) (m } func (c *Config) GetPackageConfig(ctx context.Context, packageName string) (*Config, error) { - log := zerolog.Ctx(ctx) + log := zerolog.Ctx(ctx).With().Str("package-path", packageName).Logger() if c.pkgConfigCache == nil { log.Debug().Msg("package cache is nil") @@ -228,11 +228,13 @@ func (c *Config) GetPackageConfig(ctx context.Context, packageName string) (*Con return pkgConf, nil } - pkgConfig := reflect.New(reflect.ValueOf(c).Elem().Type()).Interface() + //pkgConfig := reflect.New(reflect.ValueOf(c).Elem().Type()).Interface() + pkgConfig := &Config{} if err := copier.Copy(pkgConfig, c); err != nil { return nil, fmt.Errorf("failed to copy config: %w", err) } - pkgConfigTyped := pkgConfig.(*Config) + //pkgConfigTyped := pkgConfig.(*Config) + pkgConfigTyped := pkgConfig configMap, err := c.getPackageConfigMap(ctx, packageName) if err != nil { @@ -242,6 +244,7 @@ func (c *Config) GetPackageConfig(ctx context.Context, packageName string) (*Con configSection, ok := configMap["config"] if !ok { log.Debug().Msg("config section not provided for package") + configMap["config"] = deepCopyConfigMap(c._cfgAsMap) return pkgConfigTyped, nil } @@ -444,13 +447,13 @@ func (c *Config) addSubPkgConfig(ctx context.Context, subPkgPath string, parentP } log.Debug().Msg("getting config") - cfg, err := c.CfgAsMap(ctx) + topLevelConfig, err := c.CfgAsMap(ctx) if err != nil { return fmt.Errorf("failed to get configuration map: %w", err) } log.Debug().Msg("getting packages section") - packagesSection := cfg["packages"].(map[string]any) + packagesSection := topLevelConfig["packages"].(map[string]any) // Don't overwrite any config that already exists _, pkgExists := packagesSection[subPkgPath] @@ -600,6 +603,7 @@ func (c *Config) subPackages( // recursive and recurses the file tree to find all sub-packages. func (c *Config) discoverRecursivePackages(ctx context.Context) error { log := zerolog.Ctx(ctx) + log.Trace().Msg("discovering recursive packages") recursivePackages := map[string]*Config{} packageList, err := c.GetPackages(ctx) if err != nil { @@ -607,11 +611,17 @@ func (c *Config) discoverRecursivePackages(ctx context.Context) error { } for _, pkg := range packageList { pkgConfig, err := c.GetPackageConfig(ctx, pkg) + pkgLog := log.With().Str("package", pkg).Logger() + pkgLog.Trace().Msg("iterating over package") if err != nil { return fmt.Errorf("failed to get package config: %w", err) } if pkgConfig.Recursive { + pkgLog.Trace().Msg("package marked as recursive") recursivePackages[pkg] = pkgConfig + } else { + pkgLog.Trace().Msg("package not marked as recursive") + pkgLog.Trace().Msg(fmt.Sprintf("%+v", pkgConfig)) } } if len(recursivePackages) == 0 { @@ -703,8 +713,12 @@ func (c *Config) mergeInConfig(ctx context.Context) error { configSectionUntyped, configExists := packageConfig["config"] if !configExists { - pkgLog.Trace().Msg("config section doesn't exist, setting it to a deepcopy of the top-level config") - packageConfig["config"] = deepCopyConfigMap(defaultCfg) + // The reason why this should never happen is because getPackageConfigMap + // should be populating the config section with the top-level config if it + // wasn't defined in the yaml. + msg := "config section does not exist for package, this should never happen" + pkgLog.Error().Msg(msg) + return fmt.Errorf(msg) } pkgLog.Trace().Msg("got config section for package") diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 5a7b654e..bf8edd8e 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1182,22 +1182,24 @@ dir: foobar recursive: True all: True packages: - github.com/vektra/mockery/v2/pkg/fixtures/example_project/pkg_with_subpkgs: + github.com/vektra/mockery/v2/pkg/fixtures/example_project/pkg_with_subpkgs/subpkg2: `, - wantCfgMap: `dir: foobar + wantCfgMap: `all: true +dir: foobar packages: github.com/vektra/mockery/v2/pkg/fixtures/example_project/pkg_with_subpkgs/subpkg2: config: all: true dir: foobar recursive: true - with-expecter: true + with-expecter: false github.com/vektra/mockery/v2/pkg/fixtures/example_project/pkg_with_subpkgs/subpkg2/subpkg3: config: all: true dir: foobar recursive: true - with-expecter: true + with-expecter: false +recursive: true with-expecter: false `, }, @@ -1209,19 +1211,22 @@ with-expecter: false cfg := tmpdir.Join("config.yaml") require.NoError(t, cfg.WriteFile([]byte(tt.cfgYaml))) - c := &Config{ - Config: cfg.String(), - } + viperObj := viper.New() + viperObj.SetConfigFile(cfg.String()) + require.NoError(t, viperObj.ReadInConfig()) + c, err := NewConfigFromViper(viperObj) + require.NoError(t, err) + log, err := logging.GetLogger("TRACE") require.NoError(t, err) - if err := c.Initialize(log.WithContext(context.Background())); !errors.Is(err, tt.wantErr) { + if err := c.Initialize(log.WithContext(ctx)); !errors.Is(err, tt.wantErr) { t.Errorf("Config.Initialize() error = %v, wantErr %v", err, tt.wantErr) } cfgAsMap, err := c.CfgAsMap(ctx) require.NoError(t, err) - + cfgAsStr, err := yaml.Marshal(cfgAsMap) require.NoError(t, err)