Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ var (
UseVersion string
WorkspaceTmpDir bool
OnlyOutTestToml bool
Subset bool
)

// In order to debug CLI running under acceptance test, search for TestInprocessMode and update
Expand Down Expand Up @@ -78,6 +79,7 @@ func init() {
// to simulate an identical environment.
flag.BoolVar(&WorkspaceTmpDir, "workspace-tmp-dir", false, "Run tests on the workspace file system (For DBR testing).")
flag.BoolVar(&OnlyOutTestToml, "only-out-test-toml", false, "Only regenerate out.test.toml files without running tests")
flag.BoolVar(&Subset, "subset", false, "Select a subset of EnvMatrix variants that cover all output files. Auto-enabled on -update.")
}

const (
Expand Down Expand Up @@ -156,7 +158,32 @@ func setReplsForTestEnvVars(t *testing.T, repls *testdiff.ReplacementsContext) {
}
}

// helperScriptUsesEngineCache caches whether a _script helper in a given directory
// (or any of its ancestors) references $DATABRICKS_BUNDLE_ENGINE.
// Since _script helpers are shared across many tests, caching avoids redundant reads.
var helperScriptUsesEngineCache sync.Map

// anyHelperScriptUsesEngine returns true if any _script helper in dir or its ancestors
// contains $DATABRICKS_BUNDLE_ENGINE.
func anyHelperScriptUsesEngine(dir string) bool {
if dir == "" || dir == "." {
return false
}
if v, ok := helperScriptUsesEngineCache.Load(dir); ok {
return v.(bool)
}
content, err := os.ReadFile(filepath.Join(dir, "_script"))
result := (err == nil && strings.Contains(string(content), "$DATABRICKS_BUNDLE_ENGINE")) ||
anyHelperScriptUsesEngine(filepath.Dir(dir))
helperScriptUsesEngineCache.Store(dir, result)
return result
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SUGGESTION] anyHelperScriptUsesEngine() and the Subset wiring at line 395-400 are new harness control flow, but the unit tests only exercise SubsetExpanded() in isolation. A regression in the ancestor _script scan or the -update auto-enable path would not be caught. Consider adding a selftest acceptance case whose output varies by engine only through an ancestor _script, run under -subset/-update so the harness behavior is pinned end-to-end.

Copy link
Contributor Author

@denik denik Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to add self-test as it also serves as documentation. 22304e1

}

func testAccept(t *testing.T, inprocessMode bool, singleTest string) int {
if testdiff.OverwriteMode {
Subset = true
}

repls := testdiff.ReplacementsContext{}
cwd, err := os.Getwd()
require.NoError(t, err)
Expand Down Expand Up @@ -366,6 +393,12 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int {
}

expanded := internal.ExpandEnvMatrix(config.EnvMatrix, config.EnvMatrixExclude, extraVars)
if Subset {
scriptContent, _ := os.ReadFile(filepath.Join(dir, EntryPointScript))
scriptUsesEngine := strings.Contains(string(scriptContent), "$DATABRICKS_BUNDLE_ENGINE") ||
anyHelperScriptUsesEngine(dir)
expanded = internal.SubsetExpanded(expanded, dir, scriptUsesEngine)
}

for ind, envset := range expanded {
envname := strings.Join(envset, "/")
Expand Down
66 changes: 58 additions & 8 deletions acceptance/internal/config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package internal

import (
"hash/fnv"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -427,17 +428,66 @@ func filterExcludedEnvSets(envSets [][]string, exclude map[string][]string) [][]
return filtered
}

// SubsetExpanded selects one variant per DATABRICKS_BUNDLE_ENGINE value (if scriptUsesEngine)
// or one variant total from an already-expanded and exclusion-filtered list.
// DATABRICKS_BUNDLE_ENGINE=direct has weight 10; all other variants have weight 1.
func SubsetExpanded(expanded [][]string, testDir string, scriptUsesEngine bool) [][]string {
if len(expanded) <= 1 {
return expanded
}
if scriptUsesEngine {
// Collect candidates per engine key, preserving first-seen order.
// keyToIdx maps engine value -> index in result/groups slices.
var result [][]string
var groups [][][]string
keyToIdx := make(map[string]int)
for _, envset := range expanded {
engine := ""
for _, kv := range envset {
if v, ok := strings.CutPrefix(kv, "DATABRICKS_BUNDLE_ENGINE="); ok {
engine = v
break
}
}
idx, ok := keyToIdx[engine]
if !ok {
idx = len(result)
keyToIdx[engine] = idx
result = append(result, nil)
groups = append(groups, nil)
}
groups[idx] = append(groups[idx], envset)
}
for i, group := range groups {
result[i] = weightedSelect(group, testDir)
}
return result
}
return [][]string{weightedSelect(expanded, testDir)}
}

// weightedSelect picks one envset using weighted consistent hashing.
// DATABRICKS_BUNDLE_ENGINE=direct has weight 10; all other envsets have weight 1.
func weightedSelect(envsets [][]string, testDir string) []string {
var weighted [][]string
for _, envset := range envsets {
weight := 1
if slices.Contains(envset, "DATABRICKS_BUNDLE_ENGINE=direct") {
weight = 10
}
for range weight {
weighted = append(weighted, envset)
}
}
h := fnv.New64a()
h.Write([]byte(testDir))
return weighted[h.Sum64()%uint64(len(weighted))]
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SUGGESTION] weightedSelect will panic on empty input (division by zero). Currently unreachable since SubsetExpanded returns early for len(expanded) <= 1, but a precondition comment would help future maintainers:

// Precondition: envsets must be non-empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

panic on unsupported input is okay

// matchesExclusionRule returns true if envSet contains all KEY=value pairs from excludeRule.
func matchesExclusionRule(envSet, excludeRule []string) bool {
for _, excludePair := range excludeRule {
found := false
for _, envPair := range envSet {
if envPair == excludePair {
found = true
break
}
}
if !found {
if !slices.Contains(envSet, excludePair) {
return false
}
}
Expand Down
41 changes: 41 additions & 0 deletions acceptance/internal/config_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package internal

import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -201,6 +203,45 @@ func TestExpandEnvMatrix(t *testing.T) {
}
}

func TestSubsetExpanded_DirectBias(t *testing.T) {
// Across many test dirs, DATABRICKS_BUNDLE_ENGINE=direct should be selected ~10/11 of the time.
expanded := [][]string{
{"DATABRICKS_BUNDLE_ENGINE=terraform"},
{"DATABRICKS_BUNDLE_ENGINE=direct"},
}
directCount := 0
total := 1000
for i := range total {
r := SubsetExpanded(expanded, fmt.Sprintf("test/dir%d", i), false)
if r[0][0] == "DATABRICKS_BUNDLE_ENGINE=direct" {
directCount++
}
}
ratio := float64(directCount) / float64(total)
assert.InDelta(t, float64(10)/11, ratio, 0.05, "expected ~10/11 direct, got %.1f%%", ratio*100)
}

func TestSubsetExpanded_ScriptUsesEngine(t *testing.T) {
// When script uses $DATABRICKS_BUNDLE_ENGINE, one combo per engine value is returned.
expanded := [][]string{
{"DATABRICKS_BUNDLE_ENGINE=terraform", "READPLAN="},
{"DATABRICKS_BUNDLE_ENGINE=direct", "READPLAN="},
{"DATABRICKS_BUNDLE_ENGINE=direct", "READPLAN=1"},
}
result := SubsetExpanded(expanded, "test/dir", true)
require.Len(t, result, 2)
engines := make(map[string]bool)
for _, envset := range result {
for _, kv := range envset {
if strings.HasPrefix(kv, "DATABRICKS_BUNDLE_ENGINE=") {
engines[kv] = true
}
}
}
assert.True(t, engines["DATABRICKS_BUNDLE_ENGINE=terraform"])
assert.True(t, engines["DATABRICKS_BUNDLE_ENGINE=direct"])
}

func TestLoadConfigPhaseIsNotInherited(t *testing.T) {
tests := []struct {
name string
Expand Down
1 change: 1 addition & 0 deletions acceptance/selftest/subset_ancestor_engine/_script
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
echo "engine=$DATABRICKS_BUNDLE_ENGINE" > "out.$DATABRICKS_BUNDLE_ENGINE.txt"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
engine=direct
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
engine=terraform

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Empty file.
4 changes: 4 additions & 0 deletions acceptance/selftest/subset_ancestor_engine/child/script
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# This script does not directly reference $DATABRICKS_BUNDLE_ENGINE.
# Engine detection happens via the ancestor _script helper,
# exercising the anyHelperScriptUsesEngine() path in subset selection.
source "$TESTDIR/../_script"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
EnvVaryOutput = "DATABRICKS_BUNDLE_ENGINE"
Loading