From ff4a5c22698dd31445524672c70f05d05031d4f3 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 7 Feb 2025 17:38:27 +0100 Subject: [PATCH] acc: Implement config merge (#2294) ## Changes Instead of using leaf-most config, all configs from root at acceptance/test.toml to all intermediate ones to leaf config are merged into one. Maps are merged, slices are appended, other values are overridden. I had to disable caching, because it is tricky when merging is involved - deep copy is needed. There is performance impact but currently it is tiny, about 1%. Also, remove empty root config. ## Tests Manually checked that inheritance of LocalOnly setting worked for these tests: Before - integration tests showed: ``` PASS acceptance.TestAccept/bundle/templates/wrong-url (0.70s) PASS acceptance.TestAccept/bundle/templates/wrong-path (0.44s) ``` After: ``` SKIP acceptance.TestAccept/bundle/templates/wrong-url (0.00s) SKIP acceptance.TestAccept/bundle/templates/wrong-path (0.00s) acceptance_test.go:216: Disabled via LocalOnly setting in bundle/templates/test.toml, bundle/templates/wrong-path/test.toml (CLOUD_ENV=***) ``` --- NOTICE | 5 ++++ acceptance/config_test.go | 56 +++++++++++++++++---------------------- acceptance/test.toml | 2 -- go.mod | 1 + go.sum | 4 +-- 5 files changed, 33 insertions(+), 35 deletions(-) delete mode 100644 acceptance/test.toml diff --git a/NOTICE b/NOTICE index ed22084cf..4331a2a32 100644 --- a/NOTICE +++ b/NOTICE @@ -109,3 +109,8 @@ License - https://github.com/hexops/gotextdiff/blob/main/LICENSE https://github.com/BurntSushi/toml Copyright (c) 2013 TOML authors https://github.com/BurntSushi/toml/blob/master/COPYING + +dario.cat/mergo +Copyright (c) 2013 Dario Castañé. All rights reserved. +Copyright (c) 2012 The Go Authors. All rights reserved. +https://github.com/darccio/mergo/blob/master/LICENSE diff --git a/acceptance/config_test.go b/acceptance/config_test.go index e24a683e7..920e713a1 100644 --- a/acceptance/config_test.go +++ b/acceptance/config_test.go @@ -3,9 +3,11 @@ package acceptance_test import ( "os" "path/filepath" - "sync" + "slices" + "strings" "testing" + "dario.cat/mergo" "github.com/BurntSushi/toml" "github.com/databricks/cli/libs/testdiff" "github.com/stretchr/testify/require" @@ -13,11 +15,6 @@ import ( const configFilename = "test.toml" -var ( - configCache map[string]TestConfig - configMutex sync.Mutex -) - type TestConfig struct { // Place to describe what's wrong with this test. Does not affect how the test is run. Badness string @@ -65,58 +62,55 @@ type ServerStub struct { } } -// FindConfig finds the closest config file. -func FindConfig(t *testing.T, dir string) (string, bool) { - shared := false +// FindConfigs finds all the config relevant for this test, +// ordered from the most outermost (at acceptance/) to current test directory (identified by dir). +// Argument dir must be a relative path from the root of acceptance tests (/acceptance/). +func FindConfigs(t *testing.T, dir string) []string { + configs := []string{} for { path := filepath.Join(dir, configFilename) _, err := os.Stat(path) if err == nil { - return path, shared + configs = append(configs, path) } - shared = true - if dir == "" || dir == "." { break } - if os.IsNotExist(err) { - dir = filepath.Dir(dir) + dir = filepath.Dir(dir) + + if err == nil || os.IsNotExist(err) { continue } t.Fatalf("Error while reading %s: %s", path, err) } - t.Fatal("Config not found: " + configFilename) - return "", shared + slices.Reverse(configs) + return configs } // LoadConfig loads the config file. Non-leaf configs are cached. func LoadConfig(t *testing.T, dir string) (TestConfig, string) { - path, leafConfig := FindConfig(t, dir) + configs := FindConfigs(t, dir) - if leafConfig { - return DoLoadConfig(t, path), path + if len(configs) == 0 { + return TestConfig{}, "(no config)" } - configMutex.Lock() - defer configMutex.Unlock() + result := DoLoadConfig(t, configs[0]) - if configCache == nil { - configCache = make(map[string]TestConfig) + for _, cfgName := range configs[1:] { + cfg := DoLoadConfig(t, cfgName) + err := mergo.Merge(&result, cfg, mergo.WithOverride, mergo.WithAppendSlice) + if err != nil { + t.Fatalf("Error during config merge: %s: %s", cfgName, err) + } } - result, ok := configCache[path] - if ok { - return result, path - } - - result = DoLoadConfig(t, path) - configCache[path] = result - return result, path + return result, strings.Join(configs, ", ") } func DoLoadConfig(t *testing.T, path string) TestConfig { diff --git a/acceptance/test.toml b/acceptance/test.toml deleted file mode 100644 index eee94d0ea..000000000 --- a/acceptance/test.toml +++ /dev/null @@ -1,2 +0,0 @@ -# If test directory nor any of its parents do not have test.toml then this file serves as fallback configuration. -# The configurations are not merged across parents; the closest one is used fully. diff --git a/go.mod b/go.mod index b3f11e918..662fcd40b 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.23 toolchain go1.23.4 require ( + dario.cat/mergo v1.0.1 // BSD 3-Clause github.com/BurntSushi/toml v1.4.0 // MIT github.com/Masterminds/semver/v3 v3.3.1 // MIT github.com/briandowns/spinner v1.23.1 // Apache 2.0 diff --git a/go.sum b/go.sum index 4e295a82d..bffc3b53d 100644 --- a/go.sum +++ b/go.sum @@ -5,8 +5,8 @@ cloud.google.com/go/auth/oauth2adapt v0.2.2 h1:+TTV8aXpjeChS9M+aTtN/TjdQnzJvmzKF cloud.google.com/go/auth/oauth2adapt v0.2.2/go.mod h1:wcYjgpZI9+Yu7LyYBg4pqSiaRkfEK3GQcpb7C/uyF1Q= cloud.google.com/go/compute/metadata v0.3.0 h1:Tz+eQXMEqDIKRsmY3cHTL6FVaynIjX2QxYC4trgAKZc= cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= -dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk= -dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= +dario.cat/mergo v1.0.1 h1:Ra4+bf83h2ztPIQYNP99R6m+Y7KfnARDfID+a+vLl4s= +dario.cat/mergo v1.0.1/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v1.4.0 h1:kuoIxZQy2WRRk1pttg9asf+WVv6tWQuBNVmK8+nqPr0= github.com/BurntSushi/toml v1.4.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho=