From ccdbb1aebaa4ede35984b5c708d4c2b35de69398 Mon Sep 17 00:00:00 2001
From: Denis Bilenko <denis.bilenko@databricks.com>
Date: Tue, 25 Feb 2025 09:53:46 +0100
Subject: [PATCH] acc: fix override of settings with null values (#2369)

## Changes
Previously, one could not set `LocalOnly=true` in parent directory and
then override it with `LocalOnly=false` in child directory. This is
because, `false` is considered empty value by mergo.

In order to distinguish between 'explicitly set to false' and 'not set',
I've changed all simple variables in config to be pointers. Now, one can
always override those, because non-nil pointers are not null (with
mergo.WithoutDereference option).

## Tests
Manually:

```
~/work/cli/acceptance/bundle/templates/default-python % cat test.toml   # add this new file
LocalOnly = false

~/work/cli/acceptance/bundle/templates/default-python % CLOUD_ENV=aws go test ../../.. -run ^TestAccept$/^bundle$/^templates$/^default-python$ -v
(the test is no longer skipped)
```
---
 acceptance/acceptance_test.go | 10 +++++++---
 acceptance/config_test.go     |  8 ++++----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go
index c0fa960b6..465df5ec7 100644
--- a/acceptance/acceptance_test.go
+++ b/acceptance/acceptance_test.go
@@ -217,7 +217,7 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont
 	}
 
 	cloudEnv := os.Getenv("CLOUD_ENV")
-	if config.LocalOnly && cloudEnv != "" {
+	if isTruePtr(config.LocalOnly) && cloudEnv != "" {
 		t.Skipf("Disabled via LocalOnly setting in %s (CLOUD_ENV=%s)", configPath, cloudEnv)
 	}
 
@@ -263,9 +263,9 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont
 
 		databricksLocalHost := os.Getenv("DATABRICKS_DEFAULT_HOST")
 
-		if len(config.Server) > 0 || config.RecordRequests {
+		if len(config.Server) > 0 || isTruePtr(config.RecordRequests) {
 			server = testserver.New(t)
-			if config.RecordRequests {
+			if isTruePtr(config.RecordRequests) {
 				requestsPath := filepath.Join(tmpDir, "out.requests.txt")
 				server.RecordRequestsCallback = func(request *testserver.Request) {
 					req := getLoggedRequest(request, config.IncludeRequestHeaders)
@@ -703,3 +703,7 @@ func filterHeaders(h http.Header, includedHeaders []string) http.Header {
 	}
 	return headers
 }
+
+func isTruePtr(value *bool) bool {
+	return value != nil && *value
+}
diff --git a/acceptance/config_test.go b/acceptance/config_test.go
index ec0d1baee..a3589b2e5 100644
--- a/acceptance/config_test.go
+++ b/acceptance/config_test.go
@@ -18,14 +18,14 @@ const configFilename = "test.toml"
 
 type TestConfig struct {
 	// Place to describe what's wrong with this test. Does not affect how the test is run.
-	Badness string
+	Badness *string
 
 	// Which OSes the test is enabled on. Each string is compared against runtime.GOOS.
 	// If absent, default to true.
 	GOOS map[string]bool
 
 	// If true, do not run this test against cloud environment
-	LocalOnly bool
+	LocalOnly *bool
 
 	// List of additional replacements to apply on this test.
 	// Old is a regexp, New is a replacement expression.
@@ -44,7 +44,7 @@ type TestConfig struct {
 
 	// Record the requests made to the server and write them as output to
 	// out.requests.txt
-	RecordRequests bool
+	RecordRequests *bool
 
 	// List of request headers to include when recording requests.
 	IncludeRequestHeaders []string
@@ -102,7 +102,7 @@ func LoadConfig(t *testing.T, dir string) (TestConfig, string) {
 
 	for _, cfgName := range configs[1:] {
 		cfg := DoLoadConfig(t, cfgName)
-		err := mergo.Merge(&result, cfg, mergo.WithOverride, mergo.WithAppendSlice)
+		err := mergo.Merge(&result, cfg, mergo.WithOverride, mergo.WithoutDereference, mergo.WithAppendSlice)
 		if err != nil {
 			t.Fatalf("Error during config merge: %s: %s", cfgName, err)
 		}