From b97d8239fcf5eea24885bbfd1d39e53e1865e41a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 27 Oct 2026 12:13:51 +0100 Subject: [PATCH] Replace env var with acceptance SuppressLines; suppress retry warnings by default Instead of disabling the retry warning in the CLI, the acceptance runner moves lines matching SuppressLines patterns from the compared output to the test log. Tests that assert the warning disable the named pattern with an empty value. Co-authored-by: Isaac --- acceptance/acceptance_test.go | 38 +++++++++++++++++-- .../bundle/deploy/files/upload-504/test.toml | 2 +- .../permissions/pipelines/504/test.toml | 2 +- acceptance/bundle/sync/upload-504/test.toml | 2 +- .../upload/internal_server_error/test.toml | 2 +- acceptance/cmd/sync/get-status-504/test.toml | 2 +- acceptance/cmd/sync/upload-504/test.toml | 2 +- acceptance/internal/config.go | 7 ++++ .../selftest/suppresslines/keep/out.test.toml | 1 + .../selftest/suppresslines/keep/output.txt | 4 ++ acceptance/selftest/suppresslines/keep/script | 4 ++ .../selftest/suppresslines/keep/test.toml | 3 ++ .../selftest/suppresslines/out.test.toml | 1 + acceptance/selftest/suppresslines/output.txt | 2 + acceptance/selftest/suppresslines/script | 4 ++ acceptance/selftest/suppresslines/test.toml | 4 ++ acceptance/test.toml | 9 +++-- libs/retry/retry.go | 8 +--- libs/testdiff/removelines.go | 21 ++++++++++ libs/testdiff/removelines_test.go | 20 ++++++++++ 20 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 acceptance/selftest/suppresslines/keep/out.test.toml create mode 100644 acceptance/selftest/suppresslines/keep/output.txt create mode 100644 acceptance/selftest/suppresslines/keep/script create mode 100644 acceptance/selftest/suppresslines/keep/test.toml create mode 100644 acceptance/selftest/suppresslines/out.test.toml create mode 100644 acceptance/selftest/suppresslines/output.txt create mode 100644 acceptance/selftest/suppresslines/script create mode 100644 acceptance/selftest/suppresslines/test.toml create mode 100644 libs/testdiff/removelines.go create mode 100644 libs/testdiff/removelines_test.go diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index f7509623b81..29f1ccde3a3 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1072,13 +1072,14 @@ func runTest(t *testing.T, pathFilter := preparePathFilter(config, customEnv) sortLines := compileSortLines(t, config) + suppressLines := compileSuppressLines(t, config) // output.txt lives in outputDir, not tmpDir, so the bundle sync never uploads it; // compare it from there. Every run produces it, so compare it explicitly rather // than relying on it turning up in the tmpDir scan below. out.requests.txt also // stays in outputDir and is never compared: tests assert on recorded requests // through print_requests.py, not by committing the raw recording. - doComparison(t, repls, sortLines, dir, outputDir, "output.txt", &printedRepls) + doComparison(t, repls, sortLines, suppressLines, dir, outputDir, "output.txt", &printedRepls) // Compare expected outputs for relPath := range outputs { @@ -1090,7 +1091,7 @@ func runTest(t *testing.T, continue } - doComparison(t, repls, sortLines, dir, tmpDir, relPath, &printedRepls) + doComparison(t, repls, sortLines, suppressLines, dir, tmpDir, relPath, &printedRepls) } // Make sure there are not unaccounted for new files @@ -1122,7 +1123,7 @@ func runTest(t *testing.T, if strings.HasPrefix(relPath, "out") { // We have a new file starting with "out" // Show the contents & support overwrite mode for it: - doComparison(t, repls, sortLines, dir, tmpDir, relPath, &printedRepls) + doComparison(t, repls, sortLines, suppressLines, dir, tmpDir, relPath, &printedRepls) } } @@ -1239,7 +1240,25 @@ func compileSortLines(t *testing.T, config internal.TestConfig) []*regexp.Regexp return result } -func doComparison(t *testing.T, repls testdiff.ReplacementsContext, sortLines []*regexp.Regexp, dirRef, dirNew, relPath string, printedRepls *bool) { +// compileSuppressLines compiles the non-empty SuppressLines patterns from the test config. +// Patterns are returned in name order so the result does not depend on map iteration. +func compileSuppressLines(t *testing.T, config internal.TestConfig) []*regexp.Regexp { + result := make([]*regexp.Regexp, 0, len(config.SuppressLines)) + for _, name := range slices.Sorted(maps.Keys(config.SuppressLines)) { + pattern := config.SuppressLines[name] + if pattern == "" { + continue + } + re, err := regexp.Compile(pattern) + if err != nil { + t.Fatalf("Invalid SuppressLines pattern %s = %#v: %s", name, pattern, err) + } + result = append(result, re) + } + return result +} + +func doComparison(t *testing.T, repls testdiff.ReplacementsContext, sortLines, suppressLines []*regexp.Regexp, dirRef, dirNew, relPath string, printedRepls *bool) { pathRef := filepath.Join(dirRef, relPath) pathNew := filepath.Join(dirNew, relPath) bufRef, okRef := tryReading(t, pathRef) @@ -1258,6 +1277,17 @@ func doComparison(t *testing.T, repls testdiff.ReplacementsContext, sortLines [] valueNew = repls.Replace(valueNew) } + // Move lines the test does not pin (e.g. retry warnings) from the compared output to the log. + // Applied to the reference too, so a golden that still contains them compares the same way. + for _, re := range suppressLines { + var removed []string + valueNew, removed = testdiff.RemoveLines(valueNew, re) + valueRef, _ = testdiff.RemoveLines(valueRef, re) + if len(removed) > 0 { + t.Logf("Suppressed from %s:\n%s", relPath, strings.Join(removed, "\n")) + } + } + // Canonicalize line runs whose order the command does not guarantee. Applied to // the reference too so a hand-edited golden compares the same way; sorting is // idempotent, so a stored reference is unaffected. diff --git a/acceptance/bundle/deploy/files/upload-504/test.toml b/acceptance/bundle/deploy/files/upload-504/test.toml index 7d496b2f139..711ede517b9 100644 --- a/acceptance/bundle/deploy/files/upload-504/test.toml +++ b/acceptance/bundle/deploy/files/upload-504/test.toml @@ -1,3 +1,3 @@ RecordRequests = true Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100" -Env.DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING = "" +SuppressLines.warn_retry = "" diff --git a/acceptance/bundle/resources/permissions/pipelines/504/test.toml b/acceptance/bundle/resources/permissions/pipelines/504/test.toml index 99ccafaf8e8..b26f818ab24 100644 --- a/acceptance/bundle/resources/permissions/pipelines/504/test.toml +++ b/acceptance/bundle/resources/permissions/pipelines/504/test.toml @@ -2,4 +2,4 @@ Env.DATABRICKS_BUNDLE_ENGINE = "direct" EnvMatrix.READPLAN = [] RecordRequests = true Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100" -Env.DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING = "" +SuppressLines.warn_retry = "" diff --git a/acceptance/bundle/sync/upload-504/test.toml b/acceptance/bundle/sync/upload-504/test.toml index 7d496b2f139..711ede517b9 100644 --- a/acceptance/bundle/sync/upload-504/test.toml +++ b/acceptance/bundle/sync/upload-504/test.toml @@ -1,3 +1,3 @@ RecordRequests = true Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100" -Env.DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING = "" +SuppressLines.warn_retry = "" diff --git a/acceptance/bundle/upload/internal_server_error/test.toml b/acceptance/bundle/upload/internal_server_error/test.toml index 838bbee034f..89b52a06240 100644 --- a/acceptance/bundle/upload/internal_server_error/test.toml +++ b/acceptance/bundle/upload/internal_server_error/test.toml @@ -1,2 +1,2 @@ Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100" -Env.DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING = "" +SuppressLines.warn_retry = "" diff --git a/acceptance/cmd/sync/get-status-504/test.toml b/acceptance/cmd/sync/get-status-504/test.toml index 838bbee034f..89b52a06240 100644 --- a/acceptance/cmd/sync/get-status-504/test.toml +++ b/acceptance/cmd/sync/get-status-504/test.toml @@ -1,2 +1,2 @@ Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100" -Env.DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING = "" +SuppressLines.warn_retry = "" diff --git a/acceptance/cmd/sync/upload-504/test.toml b/acceptance/cmd/sync/upload-504/test.toml index 7d496b2f139..711ede517b9 100644 --- a/acceptance/cmd/sync/upload-504/test.toml +++ b/acceptance/cmd/sync/upload-504/test.toml @@ -1,3 +1,3 @@ RecordRequests = true Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100" -Env.DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING = "" +SuppressLines.warn_retry = "" diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index a6541d062de..6ea79a9578a 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -96,6 +96,13 @@ type TestConfig struct { // to true; set it to false to drop an inherited pattern for this test. SortLinesOn map[string]bool + // Map of name -> regexp. Output lines matching an enabled regexp are removed from the + // compared output and written to the test log instead. Use for lines that depend on + // timing or fault injection, e.g. retry warnings. Entries are keyed by name so an inner + // test.toml can replace an inherited pattern by reusing its name, or disable it by + // setting it to "". + SuppressLines map[string]string + // List of gitignore patterns to ignore when checking output files Ignore []string diff --git a/acceptance/selftest/suppresslines/keep/out.test.toml b/acceptance/selftest/suppresslines/keep/out.test.toml new file mode 100644 index 00000000000..18b1a88417e --- /dev/null +++ b/acceptance/selftest/suppresslines/keep/out.test.toml @@ -0,0 +1 @@ +Cloud = false diff --git a/acceptance/selftest/suppresslines/keep/output.txt b/acceptance/selftest/suppresslines/keep/output.txt new file mode 100644 index 00000000000..f0f63e21e56 --- /dev/null +++ b/acceptance/selftest/suppresslines/keep/output.txt @@ -0,0 +1,4 @@ +Signal: before +Noise: 1 +Signal: after +Warn: deploying resources.jobs.foo: retrying after 500 Internal Server Error from GET /api diff --git a/acceptance/selftest/suppresslines/keep/script b/acceptance/selftest/suppresslines/keep/script new file mode 100644 index 00000000000..bcc5b790f43 --- /dev/null +++ b/acceptance/selftest/suppresslines/keep/script @@ -0,0 +1,4 @@ +echo "Signal: before" +echo "Noise: 1" +echo "Signal: after" +echo "Warn: deploying resources.jobs.foo: retrying after 500 Internal Server Error from GET /api" diff --git a/acceptance/selftest/suppresslines/keep/test.toml b/acceptance/selftest/suppresslines/keep/test.toml new file mode 100644 index 00000000000..a8d77b0c82c --- /dev/null +++ b/acceptance/selftest/suppresslines/keep/test.toml @@ -0,0 +1,3 @@ +# An empty pattern disables the inherited one, so these lines are compared again. +SuppressLines.noise = "" +SuppressLines.warn_retry = "" diff --git a/acceptance/selftest/suppresslines/out.test.toml b/acceptance/selftest/suppresslines/out.test.toml new file mode 100644 index 00000000000..18b1a88417e --- /dev/null +++ b/acceptance/selftest/suppresslines/out.test.toml @@ -0,0 +1 @@ +Cloud = false diff --git a/acceptance/selftest/suppresslines/output.txt b/acceptance/selftest/suppresslines/output.txt new file mode 100644 index 00000000000..0b56a035232 --- /dev/null +++ b/acceptance/selftest/suppresslines/output.txt @@ -0,0 +1,2 @@ +Signal: before +Signal: after diff --git a/acceptance/selftest/suppresslines/script b/acceptance/selftest/suppresslines/script new file mode 100644 index 00000000000..bcc5b790f43 --- /dev/null +++ b/acceptance/selftest/suppresslines/script @@ -0,0 +1,4 @@ +echo "Signal: before" +echo "Noise: 1" +echo "Signal: after" +echo "Warn: deploying resources.jobs.foo: retrying after 500 Internal Server Error from GET /api" diff --git a/acceptance/selftest/suppresslines/test.toml b/acceptance/selftest/suppresslines/test.toml new file mode 100644 index 00000000000..81de16753e3 --- /dev/null +++ b/acceptance/selftest/suppresslines/test.toml @@ -0,0 +1,4 @@ +# Runs once: suppression is independent of the deployment engine. + +# Lines matching the pattern are logged instead of being compared. +SuppressLines.noise = '^Noise: ' diff --git a/acceptance/test.toml b/acceptance/test.toml index 714b05e5242..6e18a40e78a 100644 --- a/acceptance/test.toml +++ b/acceptance/test.toml @@ -19,16 +19,17 @@ Env.PYTHONDONTWRITEBYTECODE = "1" Env.PYTHONUNBUFFERED = "1" Env.PYTHONUTF8 = "1" -# Retries are timing dependent, so their warnings would make output flaky. -# Tests that inject errors deterministically and assert the warning set this to "". -Env.DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING = "1" - # The direct engine reports resources as soon as each one is applied, and resources are # applied in parallel, so the order of these lines is not deterministic. Sort each run of # them before comparing, so tests pin which resources were applied, not the order. # A test that needs the raw order sets SortLinesOn.deploy_actions = false. SortLines.deploy_actions = '^(Created|Updated|Deleted|Recreated|Resized) \S+$' +# Retries are timing dependent, so their warnings would make output flaky. They go to the +# test log instead. A test that injects errors deterministically and asserts the warning +# sets SuppressLines.warn_retry = "". +SuppressLines.warn_retry = '^Warn: .*retrying after ' + # >>> datetime.datetime.fromtimestamp(18000000000) # datetime.datetime(2027, 1, 15, 9, 0) # >>> datetime.datetime.fromtimestamp(1900000000) diff --git a/libs/retry/retry.go b/libs/retry/retry.go index 9a8a5e75db4..86dd9ec232c 100644 --- a/libs/retry/retry.go +++ b/libs/retry/retry.go @@ -23,10 +23,6 @@ const MaxRetries = 2 // IntervalMsVariable names the environment variable that overrides the pause between attempts. const IntervalMsVariable = "DATABRICKS_BUNDLE_RETRY_INTERVAL_MS" -// DisableWarningVariable names the environment variable that, when set to a non-empty value, -// suppresses the warning logged before each retry. -const DisableWarningVariable = "DATABRICKS_BUNDLE_RETRY_DISABLE_WARNING" - const defaultInterval = 15 * time.Second // interval returns the pause between attempts: the override in milliseconds if set and valid, 15s otherwise. @@ -89,9 +85,7 @@ func With[T any](ctx context.Context, check func(context.Context, error) bool, f if err == nil || attempt >= MaxRetries || !check(ctx, err) { return result, err } - if env.Get(ctx, DisableWarningVariable) == "" { - log.Warnf(ctx, "%s", retryMessage(err)) - } + log.Warnf(ctx, "%s", retryMessage(err)) select { case <-ctx.Done(): var zero T diff --git a/libs/testdiff/removelines.go b/libs/testdiff/removelines.go new file mode 100644 index 00000000000..f8d280a3eee --- /dev/null +++ b/libs/testdiff/removelines.go @@ -0,0 +1,21 @@ +package testdiff + +import ( + "regexp" + "strings" +) + +// RemoveLines drops every line matching pattern from input and returns the remaining text +// along with the dropped lines. Use it for output lines that depend on timing or fault +// injection, so that a test does not have to pin them. +func RemoveLines(input string, pattern *regexp.Regexp) (string, []string) { + var kept, removed []string + for line := range strings.SplitSeq(input, "\n") { + if pattern.MatchString(line) { + removed = append(removed, line) + } else { + kept = append(kept, line) + } + } + return strings.Join(kept, "\n"), removed +} diff --git a/libs/testdiff/removelines_test.go b/libs/testdiff/removelines_test.go new file mode 100644 index 00000000000..35d4bf03df2 --- /dev/null +++ b/libs/testdiff/removelines_test.go @@ -0,0 +1,20 @@ +package testdiff + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRemoveLines(t *testing.T) { + kept, removed := RemoveLines("a\nNoise: 1\nb\nNoise: 2\n", regexp.MustCompile(`^Noise: `)) + assert.Equal(t, "a\nb\n", kept) + assert.Equal(t, []string{"Noise: 1", "Noise: 2"}, removed) +} + +func TestRemoveLinesNoMatch(t *testing.T) { + kept, removed := RemoveLines("a\nb", regexp.MustCompile(`^Noise: `)) + assert.Equal(t, "a\nb", kept) + assert.Empty(t, removed) +}