From 727dc793c7ee2a052c638e29412b7e6cd52b25e3 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 16 Dec 2024 22:35:06 +0100 Subject: [PATCH 01/20] Add integration test for default Python bundle initialization --- go.mod | 9 +- go.sum | 16 ++ .../bundle/init_default_python_test.go | 106 +++++++ .../testdata/default_python/bundle_deploy.txt | 6 + .../testdata/default_python/bundle_init.txt | 8 + .../default_python/bundle_summary.txt | 185 ++++++++++++ .../default_python/bundle_validate.txt | 8 + internal/testcli/golden.go | 266 ++++++++++++++++++ internal/testcli/golden_test.go | 20 ++ internal/testutil/diff.go | 14 + 10 files changed, 637 insertions(+), 1 deletion(-) create mode 100644 integration/bundle/init_default_python_test.go create mode 100644 integration/bundle/testdata/default_python/bundle_deploy.txt create mode 100644 integration/bundle/testdata/default_python/bundle_init.txt create mode 100644 integration/bundle/testdata/default_python/bundle_summary.txt create mode 100644 integration/bundle/testdata/default_python/bundle_validate.txt create mode 100644 internal/testcli/golden.go create mode 100644 internal/testcli/golden_test.go create mode 100644 internal/testutil/diff.go diff --git a/go.mod b/go.mod index cf21a49dd2..49f11668d5 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/databricks/cli -go 1.23 +go 1.23.0 toolchain go1.23.4 @@ -8,12 +8,14 @@ require ( github.com/Masterminds/semver/v3 v3.3.1 // MIT github.com/briandowns/spinner v1.23.1 // Apache 2.0 github.com/databricks/databricks-sdk-go v0.54.0 // Apache 2.0 + github.com/elliotchance/orderedmap/v3 v3.0.0 // MIT github.com/fatih/color v1.18.0 // MIT github.com/google/uuid v1.6.0 // BSD-3-Clause github.com/hashicorp/go-version v1.7.0 // MPL 2.0 github.com/hashicorp/hc-install v0.9.0 // MPL 2.0 github.com/hashicorp/terraform-exec v0.21.0 // MPL 2.0 github.com/hashicorp/terraform-json v0.23.0 // MPL 2.0 + github.com/hexops/gotextdiff v1.0.3 // BSD 3-Clause "New" or "Revised" License github.com/manifoldco/promptui v0.9.0 // BSD-3-Clause github.com/mattn/go-isatty v0.0.20 // MIT github.com/nwidger/jsoncolor v0.3.2 // MIT @@ -22,6 +24,7 @@ require ( github.com/spf13/cobra v1.8.1 // Apache 2.0 github.com/spf13/pflag v1.0.5 // BSD-3-Clause github.com/stretchr/testify v1.10.0 // MIT + github.com/wI2L/jsondiff v0.6.1 // MIT golang.org/x/exp v0.0.0-20240222234643-814bf88cf225 golang.org/x/mod v0.22.0 golang.org/x/oauth2 v0.24.0 @@ -55,6 +58,10 @@ require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.2 // indirect + github.com/tidwall/gjson v1.18.0 // indirect + github.com/tidwall/match v1.1.1 // indirect + github.com/tidwall/pretty v1.2.1 // indirect + github.com/tidwall/sjson v1.2.5 // indirect github.com/zclconf/go-cty v1.15.0 // indirect go.opencensus.io v0.24.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect diff --git a/go.sum b/go.sum index 79775f512a..3959a1c1e8 100644 --- a/go.sum +++ b/go.sum @@ -37,6 +37,8 @@ github.com/databricks/databricks-sdk-go v0.54.0/go.mod h1:ds+zbv5mlQG7nFEU5ojLtg github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/elliotchance/orderedmap/v3 v3.0.0 h1:Yay/tDjX+vzza+Drcoo8VEbuBnOYGpgenCXWcpQSFDg= +github.com/elliotchance/orderedmap/v3 v3.0.0/go.mod h1:G+Hc2RwaZvJMcS4JpGCOyViCnGeKf0bTYCGTO4uhjSo= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= @@ -109,6 +111,8 @@ github.com/hashicorp/terraform-exec v0.21.0 h1:uNkLAe95ey5Uux6KJdua6+cv8asgILFVW github.com/hashicorp/terraform-exec v0.21.0/go.mod h1:1PPeMYou+KDUSSeRE9szMZ/oHf4fYUmB923Wzbq1ICg= github.com/hashicorp/terraform-json v0.23.0 h1:sniCkExU4iKtTADReHzACkk8fnpQXrdD2xoR+lppBkI= github.com/hashicorp/terraform-json v0.23.0/go.mod h1:MHdXbBAbSg0GvzuWazEGKAn/cyNfIB7mN6y7KJN6y2c= +github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= +github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= @@ -156,6 +160,18 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= +github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= +github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= +github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= +github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= +github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= +github.com/wI2L/jsondiff v0.6.1 h1:ISZb9oNWbP64LHnu4AUhsMF5W0FIj5Ok3Krip9Shqpw= +github.com/wI2L/jsondiff v0.6.1/go.mod h1:KAEIojdQq66oJiHhDyQez2x+sRit0vIzC9KeK0yizxM= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI1Bc68Uw= github.com/zclconf/go-cty v1.15.0 h1:tTCRWxsexYUmtt/wVxgDClUe+uQusuI443uL6e+5sXQ= diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go new file mode 100644 index 0000000000..b17ea687c3 --- /dev/null +++ b/integration/bundle/init_default_python_test.go @@ -0,0 +1,106 @@ +package bundle_test + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/integration/internal/acc" + "github.com/databricks/cli/internal/testcli" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/python/pythontest" + "github.com/stretchr/testify/require" +) + +var pythonVersions = []string{ + "3.8", + "3.9", + "3.10", + "3.11", + "3.12", + "3.13", +} + +var pythonVersionsShort = []string{ + "3.9", + "3.12", +} + +var extraInstalls = map[string][]string{ + "3.12": {"setuptools"}, + "3.13": {"setuptools"}, +} + +func TestDefaultPython(t *testing.T) { + versions := pythonVersions + if testing.Short() { + versions = pythonVersionsShort + } + + for _, pythonVersion := range versions { + t.Run(pythonVersion, func(t *testing.T) { + testDefaultPython(t, pythonVersion) + }) + } +} + +func testDefaultPython(t *testing.T, pythonVersion string) { + ctx, wt := acc.WorkspaceTest(t) + + uniqueProjectId := testutil.RandomName("") + ctx, replacements := testcli.WithReplacementsMap(ctx) + replacements.Set(uniqueProjectId, "$UNIQUE_PRJ") + + testcli.PrepareReplacements(t, replacements, wt.W) + + user, err := wt.W.CurrentUser.Me(ctx) + require.NoError(t, err) + if user != nil { + testcli.PrepareReplacementsUser(t, replacements, *user) + } + + tmpDir1 := pythontest.RequirePythonVENV(t, ctx, pythonVersion, true) + extras, ok := extraInstalls[pythonVersion] + if ok { + args := append([]string{"pip", "install"}, extras...) + testutil.RunCommand(t, "uv", args...) + } + + projectName := "project_name_" + uniqueProjectId + + initConfig := map[string]string{ + "project_name": projectName, + "include_notebook": "yes", + "include_python": "yes", + "include_dlt": "yes", + } + b, err := json.Marshal(initConfig) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(tmpDir1, "config.json"), b, 0o644) + require.NoError(t, err) + + testcli.RequireOutput(t, ctx, []string{"bundle", "init", "default-python", "--config-file", "config.json"}, "testdata/default_python/bundle_init.txt") + testutil.Chdir(t, projectName) + + testcli.RequireOutput(t, ctx, []string{"bundle", "validate"}, "testdata/default_python/bundle_validate.txt") + + testcli.RequireOutput(t, ctx, []string{"bundle", "deploy"}, "testdata/default_python/bundle_deploy.txt") + t.Cleanup(func() { + // Delete the stack + testcli.RequireSuccessfulRun(t, ctx, "bundle", "destroy", "--auto-approve") + }) + + ignoredFields := []string{ + "/resources/jobs/project_name_$UNIQUE_PRJ_job/email_notifications", + "/resources/jobs/project_name_$UNIQUE_PRJ_job/job_clusters/0/new_cluster/node_type_id", + "/resources/jobs/project_name_$UNIQUE_PRJ_job/url", + "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/catalog", + "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/url", + "/workspace/current_user/externalId", + "/workspace/current_user/groups", + "/workspace/current_user/name/familyName", + } + + testcli.RequireOutputJQ(t, ctx, []string{"bundle", "summary", "--output", "json"}, "testdata/default_python/bundle_summary.txt", ignoredFields) +} diff --git a/integration/bundle/testdata/default_python/bundle_deploy.txt b/integration/bundle/testdata/default_python/bundle_deploy.txt new file mode 100644 index 0000000000..eef0b79b32 --- /dev/null +++ b/integration/bundle/testdata/default_python/bundle_deploy.txt @@ -0,0 +1,6 @@ +Building project_name_$UNIQUE_PRJ... +Uploading project_name_$UNIQUE_PRJ-0.0.1+.-py3-none-any.whl... +Uploading bundle files to /Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/files... +Deploying resources... +Updating deployment state... +Deployment complete! diff --git a/integration/bundle/testdata/default_python/bundle_init.txt b/integration/bundle/testdata/default_python/bundle_init.txt new file mode 100644 index 0000000000..6cfc32f98c --- /dev/null +++ b/integration/bundle/testdata/default_python/bundle_init.txt @@ -0,0 +1,8 @@ + +Welcome to the default Python template for Databricks Asset Bundles! +Workspace to use (auto-detected, edit in 'project_name_$UNIQUE_PRJ/databricks.yml'): https://$DATABRICKS_HOST + +✨ Your new project has been created in the 'project_name_$UNIQUE_PRJ' directory! + +Please refer to the README.md file for "getting started" instructions. +See also the documentation at https://docs.databricks.com/dev-tools/bundles/index.html. diff --git a/integration/bundle/testdata/default_python/bundle_summary.txt b/integration/bundle/testdata/default_python/bundle_summary.txt new file mode 100644 index 0000000000..3143d729c2 --- /dev/null +++ b/integration/bundle/testdata/default_python/bundle_summary.txt @@ -0,0 +1,185 @@ +{ + "bundle": { + "name": "project_name_$UNIQUE_PRJ", + "target": "dev", + "environment": "dev", + "terraform": { + "exec_path": "/tmp/.../terraform" + }, + "git": { + "bundle_root_path": ".", + "inferred": true + }, + "mode": "development", + "deployment": { + "lock": { + "enabled": false + } + } + }, + "include": [ + "resources/project_name_$UNIQUE_PRJ.job.yml", + "resources/project_name_$UNIQUE_PRJ.pipeline.yml" + ], + "workspace": { + "host": "https://$DATABRICKS_HOST", + "current_user": { + "active": true, + "displayName": "$USERNAME", + "emails": [ + { + "primary": true, + "type": "work", + "value": "$USERNAME" + } + ], + "groups": [ + { + "$ref": "Groups/$USER.Groups[0]", + "display": "team.engineering", + "type": "direct", + "value": "$USER.Groups[0]" + } + ], + "id": "$USER.Id", + "name": { + "familyName": "$USERNAME", + "givenName": "$USERNAME" + }, + "schemas": [ + "urn:ietf:params:scim:schemas:core:2.0:User", + "urn:ietf:params:scim:schemas:extension:workspace:2.0:User" + ], + "short_name": "$USERNAME", + "userName": "$USERNAME" + }, + "root_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev", + "file_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/files", + "resource_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/resources", + "artifact_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/artifacts", + "state_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/state" + }, + "resources": { + "jobs": { + "project_name_$UNIQUE_PRJ_job": { + "deployment": { + "kind": "BUNDLE", + "metadata_file_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/state/metadata.json" + }, + "edit_mode": "UI_LOCKED", + "email_notifications": { + "on_failure": [ + "$USERNAME" + ] + }, + "format": "MULTI_TASK", + "id": "", + "job_clusters": [ + { + "job_cluster_key": "job_cluster", + "new_cluster": { + "autoscale": { + "max_workers": 4, + "min_workers": 1 + }, + "node_type_id": "i3.xlarge", + "spark_version": "15.4.x-scala2.12" + } + } + ], + "max_concurrent_runs": 4, + "name": "[dev $USERNAME] project_name_$UNIQUE_PRJ_job", + "queue": { + "enabled": true + }, + "tags": { + "dev": "$USERNAME" + }, + "tasks": [ + { + "job_cluster_key": "job_cluster", + "notebook_task": { + "notebook_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/files/src/notebook" + }, + "task_key": "notebook_task" + }, + { + "depends_on": [ + { + "task_key": "notebook_task" + } + ], + "pipeline_task": { + "pipeline_id": "${resources.pipelines.project_name_$UNIQUE_PRJ_pipeline.id}" + }, + "task_key": "refresh_pipeline" + }, + { + "depends_on": [ + { + "task_key": "refresh_pipeline" + } + ], + "job_cluster_key": "job_cluster", + "libraries": [ + { + "whl": "dist/*.whl" + } + ], + "python_wheel_task": { + "entry_point": "main", + "package_name": "project_name_$UNIQUE_PRJ" + }, + "task_key": "main_task" + } + ], + "trigger": { + "pause_status": "PAUSED", + "periodic": { + "interval": 1, + "unit": "DAYS" + } + }, + "url": "https://$DATABRICKS_HOST/jobs/?o=" + } + }, + "pipelines": { + "project_name_$UNIQUE_PRJ_pipeline": { + "catalog": "main", + "configuration": { + "bundle.sourcePath": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/files/src" + }, + "deployment": { + "kind": "BUNDLE", + "metadata_file_path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/state/metadata.json" + }, + "development": true, + "id": "", + "libraries": [ + { + "notebook": { + "path": "/Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev/files/src/dlt_pipeline" + } + } + ], + "name": "[dev $USERNAME] project_name_$UNIQUE_PRJ_pipeline", + "target": "project_name_$UNIQUE_PRJ_dev", + "url": "https://$DATABRICKS_HOST/pipelines/?o=" + } + } + }, + "sync": { + "paths": [ + "." + ] + }, + "presets": { + "name_prefix": "[dev $USERNAME] ", + "pipelines_development": true, + "trigger_pause_status": "PAUSED", + "jobs_max_concurrent_runs": 4, + "tags": { + "dev": "$USERNAME" + } + } +} \ No newline at end of file diff --git a/integration/bundle/testdata/default_python/bundle_validate.txt b/integration/bundle/testdata/default_python/bundle_validate.txt new file mode 100644 index 0000000000..88a5fdd181 --- /dev/null +++ b/integration/bundle/testdata/default_python/bundle_validate.txt @@ -0,0 +1,8 @@ +Name: project_name_$UNIQUE_PRJ +Target: dev +Workspace: + Host: https://$DATABRICKS_HOST + User: $USERNAME + Path: /Workspace/Users/$USERNAME/.bundle/project_name_$UNIQUE_PRJ/dev + +Validation OK! diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go new file mode 100644 index 0000000000..5186005071 --- /dev/null +++ b/internal/testcli/golden.go @@ -0,0 +1,266 @@ +package testcli + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "runtime" + "slices" + "strings" + + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/iamutil" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/iam" + "github.com/elliotchance/orderedmap/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wI2L/jsondiff" +) + +func ReadFile(t testutil.TestingT, ctx context.Context, filename string) string { + data, err := os.ReadFile(filename) + if os.IsNotExist(err) { + return "" + } + require.NoError(t, err) + return string(data) +} + +func WriteFile(t testutil.TestingT, ctx context.Context, filename, data string) { + t.Logf("Overwriting %s", filename) + err := os.WriteFile(filename, []byte(data), 0o644) + require.NoError(t, err) +} + +func captureOutput(t testutil.TestingT, ctx context.Context, args []string) string { + t.Logf("run args: [%s]", strings.Join(args, ", ")) + r := NewRunner(t, ctx, args...) + stdout, stderr, err := r.Run() + require.NoError(t, err) + out := stderr.String() + stdout.String() + return ReplaceOutput(t, ctx, out) +} + +func assertEqualTexts(t testutil.TestingT, filename1, filename2, expected, out string) { + if len(out) < 1000 && len(expected) < 1000 { + // This shows full strings + diff which could be useful when debugging newlines + assert.Equal(t, expected, out) + } else { + // only show diff for large texts + diff := testutil.Diff(filename1, filename2, expected, out) + t.Errorf("Diff:\n" + diff) + } +} + +func logDiff(t testutil.TestingT, filename1, filename2, expected, out string) { + diff := testutil.Diff(filename1, filename2, expected, out) + t.Logf("Diff:\n" + diff) +} + +func RequireOutput(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string) { + _, filename, _, _ := runtime.Caller(1) + dir := filepath.Dir(filename) + expectedPath := filepath.Join(dir, expectedFilename) + expected := ReadFile(t, ctx, expectedPath) + + out := captureOutput(t, ctx, args) + + if out != expected { + actual := fmt.Sprintf("Output from %v", args) + assertEqualTexts(t, expectedFilename, actual, expected, out) + + if os.Getenv("TESTS_OUTPUT") == "OVERWRITE" { + WriteFile(t, ctx, expectedPath, out) + } + } +} + +func RequireOutputJQ(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string, ignorePaths []string) { + _, filename, _, _ := runtime.Caller(1) + dir := filepath.Dir(filename) + expectedPath := filepath.Join(dir, expectedFilename) + expected := ReadFile(t, ctx, expectedPath) + + out := captureOutput(t, ctx, args) + + if out != expected { + patch, err := jsondiff.CompareJSON([]byte(expected), []byte(out)) + actual := fmt.Sprintf("Output from %v", args) + if err != nil { + t.Logf("CompareJSON error for %s vs %s: %s (fallback to textual comparison)", args, expectedFilename, err) + assertEqualTexts(t, expectedFilename, actual, expected, out) + } else { + logDiff(t, expectedFilename, actual, expected, out) + ignoredDiffs := []string{} + erroredDiffs := []string{} + for _, op := range patch { + if matchesPrefixes(ignorePaths, op.Path) { + ignoredDiffs = append(ignoredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) + } else { + erroredDiffs = append(erroredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) + } + } + if len(ignoredDiffs) > 0 { + t.Logf("Ignored differences between %s and %s:\n ==> %s", expectedFilename, args, strings.Join(ignoredDiffs, "\n ==> ")) + } + if len(erroredDiffs) > 0 { + t.Errorf("Unexpected differences between %s and %s:\n ==> %s", expectedFilename, args, strings.Join(erroredDiffs, "\n ==> ")) + } + } + + if os.Getenv("TESTS_OUTPUT") == "OVERWRITE" { + WriteFile(t, ctx, filepath.Join(dir, expectedFilename), out) + } + } +} + +func matchesPrefixes(prefixes []string, path string) bool { + for _, p := range prefixes { + if p == path { + return true + } + if strings.HasPrefix(path, p+"/") { + return true + } + } + return false +} + +var ( + uuidRegex = regexp.MustCompile(`[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}`) + numIdRegex = regexp.MustCompile(`[0-9]{3,}`) + privatePathRegex = regexp.MustCompile(`(/tmp|/private)(/.*)/([a-zA-Z0-9]+)`) +) + +func ReplaceOutput(t testutil.TestingT, ctx context.Context, out string) string { + out = NormalizeNewlines(out) + replacements := GetReplacementsMap(ctx) + if replacements == nil { + t.Fatal("WithReplacementsMap was not called") + } + for key, value := range replacements.AllFromFront() { + out = strings.ReplaceAll(out, key, value) + } + + out = uuidRegex.ReplaceAllString(out, "") + out = numIdRegex.ReplaceAllString(out, "") + out = privatePathRegex.ReplaceAllString(out, "/tmp/.../$3") + + return out +} + +type key int + +const ( + replacementsMapKey = key(1) +) + +func WithReplacementsMap(ctx context.Context) (context.Context, *orderedmap.OrderedMap[string, string]) { + value := ctx.Value(replacementsMapKey) + if value != nil { + if existingMap, ok := value.(*orderedmap.OrderedMap[string, string]); ok { + return ctx, existingMap + } + } + + newMap := orderedmap.NewOrderedMap[string, string]() + ctx = context.WithValue(ctx, replacementsMapKey, newMap) + return ctx, newMap +} + +func GetReplacementsMap(ctx context.Context) *orderedmap.OrderedMap[string, string] { + value := ctx.Value(replacementsMapKey) + if value != nil { + if existingMap, ok := value.(*orderedmap.OrderedMap[string, string]); ok { + return existingMap + } + } + return nil +} + +func setKV(replacements *orderedmap.OrderedMap[string, string], key, value string) { + if key == "" || value == "" { + return + } + replacements.Set(key, value) +} + +func PrepareReplacements(t testutil.TestingT, replacements *orderedmap.OrderedMap[string, string], w *databricks.WorkspaceClient) { + // in some clouds (gcp) w.Config.Host includes "https://" prefix in others it's really just a host (azure) + host := strings.TrimPrefix(strings.TrimPrefix(w.Config.Host, "http://"), "https://") + setKV(replacements, host, "$DATABRICKS_HOST") + setKV(replacements, w.Config.ClusterID, "$DATABRICKS_CLUSTER_ID") + setKV(replacements, w.Config.WarehouseID, "$DATABRICKS_WAREHOUSE_ID") + setKV(replacements, w.Config.ServerlessComputeID, "$DATABRICKS_SERVERLESS_COMPUTE_ID") + setKV(replacements, w.Config.MetadataServiceURL, "$DATABRICKS_METADATA_SERVICE_URL") + setKV(replacements, w.Config.AccountID, "$DATABRICKS_ACCOUNT_ID") + setKV(replacements, w.Config.Token, "$DATABRICKS_TOKEN") + setKV(replacements, w.Config.Username, "$DATABRICKS_USERNAME") + setKV(replacements, w.Config.Password, "$DATABRICKS_PASSWORD") + setKV(replacements, w.Config.Profile, "$DATABRICKS_CONFIG_PROFILE") + setKV(replacements, w.Config.ConfigFile, "$DATABRICKS_CONFIG_FILE") + setKV(replacements, w.Config.GoogleServiceAccount, "$DATABRICKS_GOOGLE_SERVICE_ACCOUNT") + setKV(replacements, w.Config.GoogleCredentials, "$GOOGLE_CREDENTIALS") + setKV(replacements, w.Config.AzureResourceID, "$DATABRICKS_AZURE_RESOURCE_ID") + setKV(replacements, w.Config.AzureClientSecret, "$ARM_CLIENT_SECRET") + // setKV(replacements, w.Config.AzureClientID, "$ARM_CLIENT_ID") + setKV(replacements, w.Config.AzureClientID, "$USERNAME") + setKV(replacements, w.Config.AzureTenantID, "$ARM_TENANT_ID") + setKV(replacements, w.Config.ActionsIDTokenRequestURL, "$ACTIONS_ID_TOKEN_REQUEST_URL") + setKV(replacements, w.Config.ActionsIDTokenRequestToken, "$ACTIONS_ID_TOKEN_REQUEST_TOKEN") + setKV(replacements, w.Config.AzureEnvironment, "$ARM_ENVIRONMENT") + setKV(replacements, w.Config.ClientID, "$DATABRICKS_CLIENT_ID") + setKV(replacements, w.Config.ClientSecret, "$DATABRICKS_CLIENT_SECRET") + setKV(replacements, w.Config.DatabricksCliPath, "$DATABRICKS_CLI_PATH") + setKV(replacements, w.Config.AuthType, "$DATABRICKS_AUTH_TYPE") +} + +func PrepareReplacementsUser(t testutil.TestingT, replacements *orderedmap.OrderedMap[string, string], u iam.User) { + // There could be exact matches or overlap between different name fields, so sort them by length + // to ensure we match the largest one first and map them all to the same token + names := []string{ + u.DisplayName, + u.UserName, + iamutil.GetShortUserName(&u), + u.Name.FamilyName, + u.Name.GivenName, + } + if u.Name != nil { + names = append(names, u.Name.FamilyName) + names = append(names, u.Name.GivenName) + } + for _, val := range u.Emails { + names = append(names, val.Value) + } + stableSortReverseLength(names) + + for _, name := range names { + setKV(replacements, name, "$USERNAME") + } + + for ind, val := range u.Groups { + setKV(replacements, val.Value, fmt.Sprintf("$USER.Groups[%d]", ind)) + } + + setKV(replacements, u.Id, "$USER.Id") + + for ind, val := range u.Roles { + setKV(replacements, val.Value, fmt.Sprintf("$USER.Roles[%d]", ind)) + } + + // Schemas []UserSchema `json:"schemas,omitempty"` +} + +func stableSortReverseLength(strs []string) { + slices.SortStableFunc(strs, func(a, b string) int { + return len(b) - len(a) + }) +} + +func NormalizeNewlines(input string) string { + output := strings.ReplaceAll(input, "\r\n", "\n") + return strings.ReplaceAll(output, "\r", "\n") +} diff --git a/internal/testcli/golden_test.go b/internal/testcli/golden_test.go new file mode 100644 index 0000000000..7aba982806 --- /dev/null +++ b/internal/testcli/golden_test.go @@ -0,0 +1,20 @@ +package testcli + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSort(t *testing.T) { + input := []string{"a", "bc", "cd"} + stableSortReverseLength(input) + assert.Equal(t, []string{"bc", "cd", "a"}, input) +} + +func TestMatchesPrefixes(t *testing.T) { + assert.False(t, matchesPrefixes([]string{}, "")) + assert.False(t, matchesPrefixes([]string{"/hello", "/hello/world"}, "")) + assert.True(t, matchesPrefixes([]string{"/hello", "/a/b"}, "/hello")) + assert.True(t, matchesPrefixes([]string{"/hello", "/a/b"}, "/a/b/c")) +} diff --git a/internal/testutil/diff.go b/internal/testutil/diff.go new file mode 100644 index 0000000000..1a2c043bd7 --- /dev/null +++ b/internal/testutil/diff.go @@ -0,0 +1,14 @@ +package testutil + +import ( + "fmt" + + "github.com/hexops/gotextdiff" + "github.com/hexops/gotextdiff/myers" + "github.com/hexops/gotextdiff/span" +) + +func Diff(filename1, filename2, s1, s2 string) string { + edits := myers.ComputeEdits(span.URIFromPath(filename1), s1, s2) + return fmt.Sprint(gotextdiff.ToUnified(filename1, filename2, s1, edits)) +} From e7b77eb8bc54e0221cb3d40a2e6ba11144327085 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 18 Dec 2024 21:33:42 +0100 Subject: [PATCH 02/20] Ignore all under /workspace/current_user --- integration/bundle/init_default_python_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index b17ea687c3..da8ff0c2f4 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -97,9 +97,7 @@ func testDefaultPython(t *testing.T, pythonVersion string) { "/resources/jobs/project_name_$UNIQUE_PRJ_job/url", "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/catalog", "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/url", - "/workspace/current_user/externalId", - "/workspace/current_user/groups", - "/workspace/current_user/name/familyName", + "/workspace/current_user", } testcli.RequireOutputJQ(t, ctx, []string{"bundle", "summary", "--output", "json"}, "testdata/default_python/bundle_summary.txt", ignoredFields) From 755b8de2b5cff36fbd4f93c65e94acb405c6742a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 18 Dec 2024 22:08:34 +0100 Subject: [PATCH 03/20] pass --python to "uv pip install" --- integration/bundle/init_default_python_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index da8ff0c2f4..25f1f77482 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -60,10 +60,10 @@ func testDefaultPython(t *testing.T, pythonVersion string) { testcli.PrepareReplacementsUser(t, replacements, *user) } - tmpDir1 := pythontest.RequirePythonVENV(t, ctx, pythonVersion, true) + tmpDir1, pythonExe := pythontest.RequirePythonVENV(t, ctx, pythonVersion, true) extras, ok := extraInstalls[pythonVersion] if ok { - args := append([]string{"pip", "install"}, extras...) + args := append([]string{"pip", "install", "--python", pythonExe}, extras...) testutil.RunCommand(t, "uv", args...) } From 8e1c38ac59092b823062737ddd9a8661ae684e38 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Dec 2024 12:58:59 +0100 Subject: [PATCH 04/20] ensure correct newlines for testdata --- internal/testcli/golden.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index 5186005071..e22790cd34 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -26,7 +26,8 @@ func ReadFile(t testutil.TestingT, ctx context.Context, filename string) string return "" } require.NoError(t, err) - return string(data) + // On CI, on Windows \n in the file somehow end up as \r\n + return NormalizeNewlines(string(data)) } func WriteFile(t testutil.TestingT, ctx context.Context, filename, data string) { From bc3120fe1a4638c4e48db69dbdf2bd7b049e011f Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Dec 2024 15:19:17 +0100 Subject: [PATCH 05/20] split code --- .../bundle/init_default_python_test.go | 17 ++++- internal/testcli/golden.go | 75 ++++--------------- internal/testcli/golden_test.go | 7 -- internal/testutil/diff.go | 14 ---- libs/golden/golden.go | 68 +++++++++++++++++ libs/golden/golden_test.go | 20 +++++ 6 files changed, 115 insertions(+), 86 deletions(-) delete mode 100644 internal/testutil/diff.go create mode 100644 libs/golden/golden.go create mode 100644 libs/golden/golden_test.go diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index 25f1f77482..25a84e3f29 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -3,6 +3,7 @@ package bundle_test import ( "encoding/json" "os" + "os/exec" "path/filepath" "testing" @@ -60,11 +61,19 @@ func testDefaultPython(t *testing.T, pythonVersion string) { testcli.PrepareReplacementsUser(t, replacements, *user) } - tmpDir1, pythonExe := pythontest.RequirePythonVENV(t, ctx, pythonVersion, true) + tmpDir := t.TempDir() + + opts := pythontest.VenvOpts{ + PythonVersion: pythonVersion, + Dir: tmpDir, + } + + pythontest.RequireActivatedPythonEnv(t, ctx, &opts) extras, ok := extraInstalls[pythonVersion] if ok { - args := append([]string{"pip", "install", "--python", pythonExe}, extras...) - testutil.RunCommand(t, "uv", args...) + args := append([]string{"pip", "install", "--python", opts.PythonExe}, extras...) + cmd := exec.Command("uv", args...) + require.NoError(t, cmd.Run()) } projectName := "project_name_" + uniqueProjectId @@ -77,7 +86,7 @@ func testDefaultPython(t *testing.T, pythonVersion string) { } b, err := json.Marshal(initConfig) require.NoError(t, err) - err = os.WriteFile(filepath.Join(tmpDir1, "config.json"), b, 0o644) + err = os.WriteFile(filepath.Join(tmpDir, "config.json"), b, 0o644) require.NoError(t, err) testcli.RequireOutput(t, ctx, []string{"bundle", "init", "default-python", "--config-file", "config.json"}, "testdata/default_python/bundle_init.txt") diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index e22790cd34..210d10e409 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -9,17 +9,19 @@ import ( "runtime" "slices" "strings" + "testing" "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/golden" "github.com/databricks/cli/libs/iamutil" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/elliotchance/orderedmap/v3" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/wI2L/jsondiff" ) +var OverwriteMode = os.Getenv("TESTS_OUTPUT") == "OVERWRITE" + func ReadFile(t testutil.TestingT, ctx context.Context, filename string) string { data, err := os.ReadFile(filename) if os.IsNotExist(err) { @@ -30,12 +32,6 @@ func ReadFile(t testutil.TestingT, ctx context.Context, filename string) string return NormalizeNewlines(string(data)) } -func WriteFile(t testutil.TestingT, ctx context.Context, filename, data string) { - t.Logf("Overwriting %s", filename) - err := os.WriteFile(filename, []byte(data), 0o644) - require.NoError(t, err) -} - func captureOutput(t testutil.TestingT, ctx context.Context, args []string) string { t.Logf("run args: [%s]", strings.Join(args, ", ")) r := NewRunner(t, ctx, args...) @@ -45,20 +41,10 @@ func captureOutput(t testutil.TestingT, ctx context.Context, args []string) stri return ReplaceOutput(t, ctx, out) } -func assertEqualTexts(t testutil.TestingT, filename1, filename2, expected, out string) { - if len(out) < 1000 && len(expected) < 1000 { - // This shows full strings + diff which could be useful when debugging newlines - assert.Equal(t, expected, out) - } else { - // only show diff for large texts - diff := testutil.Diff(filename1, filename2, expected, out) - t.Errorf("Diff:\n" + diff) - } -} - -func logDiff(t testutil.TestingT, filename1, filename2, expected, out string) { - diff := testutil.Diff(filename1, filename2, expected, out) - t.Logf("Diff:\n" + diff) +func WriteFile(t testutil.TestingT, filename, data string) { + t.Logf("Overwriting %s", filename) + err := os.WriteFile(filename, []byte(data), 0o644) + require.NoError(t, err) } func RequireOutput(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string) { @@ -71,10 +57,10 @@ func RequireOutput(t testutil.TestingT, ctx context.Context, args []string, expe if out != expected { actual := fmt.Sprintf("Output from %v", args) - assertEqualTexts(t, expectedFilename, actual, expected, out) + golden.AssertEqualTexts(t, expectedFilename, actual, expected, out) - if os.Getenv("TESTS_OUTPUT") == "OVERWRITE" { - WriteFile(t, ctx, expectedPath, out) + if OverwriteMode { + WriteFile(t, expectedPath, out) } } } @@ -88,46 +74,13 @@ func RequireOutputJQ(t testutil.TestingT, ctx context.Context, args []string, ex out := captureOutput(t, ctx, args) if out != expected { - patch, err := jsondiff.CompareJSON([]byte(expected), []byte(out)) actual := fmt.Sprintf("Output from %v", args) - if err != nil { - t.Logf("CompareJSON error for %s vs %s: %s (fallback to textual comparison)", args, expectedFilename, err) - assertEqualTexts(t, expectedFilename, actual, expected, out) - } else { - logDiff(t, expectedFilename, actual, expected, out) - ignoredDiffs := []string{} - erroredDiffs := []string{} - for _, op := range patch { - if matchesPrefixes(ignorePaths, op.Path) { - ignoredDiffs = append(ignoredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) - } else { - erroredDiffs = append(erroredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) - } - } - if len(ignoredDiffs) > 0 { - t.Logf("Ignored differences between %s and %s:\n ==> %s", expectedFilename, args, strings.Join(ignoredDiffs, "\n ==> ")) - } - if len(erroredDiffs) > 0 { - t.Errorf("Unexpected differences between %s and %s:\n ==> %s", expectedFilename, args, strings.Join(erroredDiffs, "\n ==> ")) - } - } + golden.AssertEqualJSONs(t.(*testing.T), expectedFilename, actual, expected, out, ignorePaths) - if os.Getenv("TESTS_OUTPUT") == "OVERWRITE" { - WriteFile(t, ctx, filepath.Join(dir, expectedFilename), out) - } - } -} - -func matchesPrefixes(prefixes []string, path string) bool { - for _, p := range prefixes { - if p == path { - return true - } - if strings.HasPrefix(path, p+"/") { - return true + if OverwriteMode { + WriteFile(t, expectedPath, out) } } - return false } var ( diff --git a/internal/testcli/golden_test.go b/internal/testcli/golden_test.go index 7aba982806..215bf33d32 100644 --- a/internal/testcli/golden_test.go +++ b/internal/testcli/golden_test.go @@ -11,10 +11,3 @@ func TestSort(t *testing.T) { stableSortReverseLength(input) assert.Equal(t, []string{"bc", "cd", "a"}, input) } - -func TestMatchesPrefixes(t *testing.T) { - assert.False(t, matchesPrefixes([]string{}, "")) - assert.False(t, matchesPrefixes([]string{"/hello", "/hello/world"}, "")) - assert.True(t, matchesPrefixes([]string{"/hello", "/a/b"}, "/hello")) - assert.True(t, matchesPrefixes([]string{"/hello", "/a/b"}, "/a/b/c")) -} diff --git a/internal/testutil/diff.go b/internal/testutil/diff.go deleted file mode 100644 index 1a2c043bd7..0000000000 --- a/internal/testutil/diff.go +++ /dev/null @@ -1,14 +0,0 @@ -package testutil - -import ( - "fmt" - - "github.com/hexops/gotextdiff" - "github.com/hexops/gotextdiff/myers" - "github.com/hexops/gotextdiff/span" -) - -func Diff(filename1, filename2, s1, s2 string) string { - edits := myers.ComputeEdits(span.URIFromPath(filename1), s1, s2) - return fmt.Sprint(gotextdiff.ToUnified(filename1, filename2, s1, edits)) -} diff --git a/libs/golden/golden.go b/libs/golden/golden.go new file mode 100644 index 0000000000..6c72bb36db --- /dev/null +++ b/libs/golden/golden.go @@ -0,0 +1,68 @@ +package golden + +import ( + "fmt" + "strings" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/hexops/gotextdiff" + "github.com/hexops/gotextdiff/myers" + "github.com/hexops/gotextdiff/span" + "github.com/stretchr/testify/assert" + "github.com/wI2L/jsondiff" +) + +func UnifiedDiff(filename1, filename2, s1, s2 string) string { + edits := myers.ComputeEdits(span.URIFromPath(filename1), s1, s2) + return fmt.Sprint(gotextdiff.ToUnified(filename1, filename2, s1, edits)) +} + +func AssertEqualTexts(t testutil.TestingT, filename1, filename2, expected, out string) { + if len(out) < 1000 && len(expected) < 1000 { + // This shows full strings + diff which could be useful when debugging newlines + assert.Equal(t, expected, out) + } else { + // only show diff for large texts + diff := UnifiedDiff(filename1, filename2, expected, out) + t.Errorf("Diff:\n" + diff) + } +} + +func AssertEqualJSONs(t *testing.T, expectedName, outName, expected, out string, ignorePaths []string) { + patch, err := jsondiff.CompareJSON([]byte(expected), []byte(out)) + if err != nil { + t.Logf("CompareJSON error for %s vs %s: %s (fallback to textual comparison)", outName, expectedName, err) + AssertEqualTexts(t, expectedName, outName, expected, out) + } else { + diff := UnifiedDiff(expectedName, outName, expected, out) + t.Logf("Diff:\n%s", diff) + ignoredDiffs := []string{} + erroredDiffs := []string{} + for _, op := range patch { + if matchesPrefixes(ignorePaths, op.Path) { + ignoredDiffs = append(ignoredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) + } else { + erroredDiffs = append(erroredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) + } + } + if len(ignoredDiffs) > 0 { + t.Logf("Ignored differences between %s and %s:\n ==> %s", expectedName, outName, strings.Join(ignoredDiffs, "\n ==> ")) + } + if len(erroredDiffs) > 0 { + t.Errorf("Unexpected differences between %s and %s:\n ==> %s", expectedName, outName, strings.Join(erroredDiffs, "\n ==> ")) + } + } +} + +func matchesPrefixes(prefixes []string, path string) bool { + for _, p := range prefixes { + if p == path { + return true + } + if strings.HasPrefix(path, p+"/") { + return true + } + } + return false +} diff --git a/libs/golden/golden_test.go b/libs/golden/golden_test.go new file mode 100644 index 0000000000..7f11583cf9 --- /dev/null +++ b/libs/golden/golden_test.go @@ -0,0 +1,20 @@ +package golden + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDiff(t *testing.T) { + assert.Equal(t, "", UnifiedDiff("a", "b", "", "")) + assert.Equal(t, "", UnifiedDiff("a", "b", "abc", "abc")) + assert.Equal(t, "+123", UnifiedDiff("a", "b", "abc", "abc\123")) +} + +func TestMatchesPrefixes(t *testing.T) { + assert.False(t, matchesPrefixes([]string{}, "")) + assert.False(t, matchesPrefixes([]string{"/hello", "/hello/world"}, "")) + assert.True(t, matchesPrefixes([]string{"/hello", "/a/b"}, "/hello")) + assert.True(t, matchesPrefixes([]string{"/hello", "/a/b"}, "/a/b/c")) +} From 94ca2772710b7033891534dce3a8c82cecbc14c2 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Dec 2024 15:20:52 +0100 Subject: [PATCH 06/20] rename golden to testdiff --- internal/testcli/golden.go | 6 +++--- libs/{golden/golden.go => testdiff/testdiff.go} | 2 +- libs/{golden/golden_test.go => testdiff/testdiff_test.go} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename libs/{golden/golden.go => testdiff/testdiff.go} (99%) rename libs/{golden/golden_test.go => testdiff/testdiff_test.go} (97%) diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index 210d10e409..c1e1345daa 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -12,8 +12,8 @@ import ( "testing" "github.com/databricks/cli/internal/testutil" - "github.com/databricks/cli/libs/golden" "github.com/databricks/cli/libs/iamutil" + "github.com/databricks/cli/libs/testdiff" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/elliotchance/orderedmap/v3" @@ -57,7 +57,7 @@ func RequireOutput(t testutil.TestingT, ctx context.Context, args []string, expe if out != expected { actual := fmt.Sprintf("Output from %v", args) - golden.AssertEqualTexts(t, expectedFilename, actual, expected, out) + testdiff.AssertEqualTexts(t, expectedFilename, actual, expected, out) if OverwriteMode { WriteFile(t, expectedPath, out) @@ -75,7 +75,7 @@ func RequireOutputJQ(t testutil.TestingT, ctx context.Context, args []string, ex if out != expected { actual := fmt.Sprintf("Output from %v", args) - golden.AssertEqualJSONs(t.(*testing.T), expectedFilename, actual, expected, out, ignorePaths) + testdiff.AssertEqualJSONs(t.(*testing.T), expectedFilename, actual, expected, out, ignorePaths) if OverwriteMode { WriteFile(t, expectedPath, out) diff --git a/libs/golden/golden.go b/libs/testdiff/testdiff.go similarity index 99% rename from libs/golden/golden.go rename to libs/testdiff/testdiff.go index 6c72bb36db..7a0077f096 100644 --- a/libs/golden/golden.go +++ b/libs/testdiff/testdiff.go @@ -1,4 +1,4 @@ -package golden +package testdiff import ( "fmt" diff --git a/libs/golden/golden_test.go b/libs/testdiff/testdiff_test.go similarity index 97% rename from libs/golden/golden_test.go rename to libs/testdiff/testdiff_test.go index 7f11583cf9..144883c551 100644 --- a/libs/golden/golden_test.go +++ b/libs/testdiff/testdiff_test.go @@ -1,4 +1,4 @@ -package golden +package testdiff import ( "testing" From 3dec4ea29bd1f04ae107c45b6579227baad020ff Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Dec 2024 15:33:48 +0100 Subject: [PATCH 07/20] fix test --- libs/testdiff/testdiff_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/testdiff/testdiff_test.go b/libs/testdiff/testdiff_test.go index 144883c551..869fee78a3 100644 --- a/libs/testdiff/testdiff_test.go +++ b/libs/testdiff/testdiff_test.go @@ -9,7 +9,7 @@ import ( func TestDiff(t *testing.T) { assert.Equal(t, "", UnifiedDiff("a", "b", "", "")) assert.Equal(t, "", UnifiedDiff("a", "b", "abc", "abc")) - assert.Equal(t, "+123", UnifiedDiff("a", "b", "abc", "abc\123")) + assert.Equal(t, "--- a\n+++ b\n@@ -1 +1,2 @@\n abc\n+123\n", UnifiedDiff("a", "b", "abc\n", "abc\n123\n")) } func TestMatchesPrefixes(t *testing.T) { From 1ac400114e78d0a81812d7e53496ba49fd6f2305 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Dec 2024 15:36:12 +0100 Subject: [PATCH 08/20] rename AssertEqualJSONs to AssertEqualJQ --- internal/testcli/golden.go | 2 +- libs/testdiff/testdiff.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index c1e1345daa..6c1d14ad69 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -75,7 +75,7 @@ func RequireOutputJQ(t testutil.TestingT, ctx context.Context, args []string, ex if out != expected { actual := fmt.Sprintf("Output from %v", args) - testdiff.AssertEqualJSONs(t.(*testing.T), expectedFilename, actual, expected, out, ignorePaths) + testdiff.AssertEqualJQ(t.(*testing.T), expectedFilename, actual, expected, out, ignorePaths) if OverwriteMode { WriteFile(t, expectedPath, out) diff --git a/libs/testdiff/testdiff.go b/libs/testdiff/testdiff.go index 7a0077f096..fad12c7680 100644 --- a/libs/testdiff/testdiff.go +++ b/libs/testdiff/testdiff.go @@ -29,7 +29,7 @@ func AssertEqualTexts(t testutil.TestingT, filename1, filename2, expected, out s } } -func AssertEqualJSONs(t *testing.T, expectedName, outName, expected, out string, ignorePaths []string) { +func AssertEqualJQ(t *testing.T, expectedName, outName, expected, out string, ignorePaths []string) { patch, err := jsondiff.CompareJSON([]byte(expected), []byte(out)) if err != nil { t.Logf("CompareJSON error for %s vs %s: %s (fallback to textual comparison)", outName, expectedName, err) From 004d0df9fe3ec2fc99e7b782359f8b25debd2af8 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Dec 2024 17:58:48 +0100 Subject: [PATCH 09/20] fix test - add missing Chdir --- integration/bundle/init_default_python_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index 25a84e3f29..3011fb9528 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -62,6 +62,7 @@ func testDefaultPython(t *testing.T, pythonVersion string) { } tmpDir := t.TempDir() + testutil.Chdir(t, tmpDir) opts := pythontest.VenvOpts{ PythonVersion: pythonVersion, From b972bcd50aec077b25ad6bde363dba82cc2997bc Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Dec 2024 19:01:07 +0100 Subject: [PATCH 10/20] fix comparison on Windows: ignore slash difference, ignore terraform path --- .../bundle/init_default_python_test.go | 1 + libs/testdiff/testdiff.go | 35 +++++++++++++++---- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index 3011fb9528..fed1355f96 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -102,6 +102,7 @@ func testDefaultPython(t *testing.T, pythonVersion string) { }) ignoredFields := []string{ + "/bundle/terraform/exec_path", "/resources/jobs/project_name_$UNIQUE_PRJ_job/email_notifications", "/resources/jobs/project_name_$UNIQUE_PRJ_job/job_clusters/0/new_cluster/node_type_id", "/resources/jobs/project_name_$UNIQUE_PRJ_job/url", diff --git a/libs/testdiff/testdiff.go b/libs/testdiff/testdiff.go index fad12c7680..b4ec6fe7d4 100644 --- a/libs/testdiff/testdiff.go +++ b/libs/testdiff/testdiff.go @@ -37,17 +37,17 @@ func AssertEqualJQ(t *testing.T, expectedName, outName, expected, out string, ig } else { diff := UnifiedDiff(expectedName, outName, expected, out) t.Logf("Diff:\n%s", diff) - ignoredDiffs := []string{} + allowedDiffs := []string{} erroredDiffs := []string{} for _, op := range patch { - if matchesPrefixes(ignorePaths, op.Path) { - ignoredDiffs = append(ignoredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) + if allowDifference(ignorePaths, op) { + allowedDiffs = append(allowedDiffs, fmt.Sprintf("%7s %s %v old=%v", op.Type, op.Path, op.Value, op.OldValue)) } else { - erroredDiffs = append(erroredDiffs, fmt.Sprintf("%7s %s %v", op.Type, op.Path, op.Value)) + erroredDiffs = append(erroredDiffs, fmt.Sprintf("%7s %s %v old=%v", op.Type, op.Path, op.Value, op.OldValue)) } } - if len(ignoredDiffs) > 0 { - t.Logf("Ignored differences between %s and %s:\n ==> %s", expectedName, outName, strings.Join(ignoredDiffs, "\n ==> ")) + if len(allowedDiffs) > 0 { + t.Logf("Allowed differences between %s and %s:\n ==> %s", expectedName, outName, strings.Join(allowedDiffs, "\n ==> ")) } if len(erroredDiffs) > 0 { t.Errorf("Unexpected differences between %s and %s:\n ==> %s", expectedName, outName, strings.Join(erroredDiffs, "\n ==> ")) @@ -55,6 +55,29 @@ func AssertEqualJQ(t *testing.T, expectedName, outName, expected, out string, ig } } +func allowDifference(ignorePaths []string, op jsondiff.Operation) bool { + if matchesPrefixes(ignorePaths, op.Path) { + return true + } + if op.Type == "replace" && almostSameStrings(op.OldValue, op.Value) { + return true + } + return false +} + +// compare strings and ignore forward vs backward slashes +func almostSameStrings(v1, v2 any) bool { + s1, ok := v1.(string) + if !ok { + return false + } + s2, ok := v2.(string) + if !ok { + return false + } + return strings.ReplaceAll(s1, "\\", "/") == strings.ReplaceAll(s2, "\\", "/") +} + func matchesPrefixes(prefixes []string, path string) bool { for _, p := range prefixes { if p == path { From c8beeb23d6c510f0cc0a74ab2399cb074b2efecd Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 09:58:10 +0100 Subject: [PATCH 11/20] clean ups; update NOTICE --- NOTICE | 4 ++++ integration/bundle/init_default_python_test.go | 5 ++--- internal/testcli/golden.go | 2 -- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/NOTICE b/NOTICE index 52fc537432..fa214a49c7 100644 --- a/NOTICE +++ b/NOTICE @@ -97,3 +97,7 @@ License - https://github.com/stretchr/testify/blob/master/LICENSE whilp/git-urls - https://github.com/whilp/git-urls Copyright (c) 2020 Will Maier License - https://github.com/whilp/git-urls/blob/master/LICENSE + +github.com/wI2L/jsondiff v0.6.1 +Copyright (c) 2020-2024 William Poussier +License - https://github.com/wI2L/jsondiff/blob/master/LICENSE diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index fed1355f96..b6ffb89600 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -57,9 +57,8 @@ func testDefaultPython(t *testing.T, pythonVersion string) { user, err := wt.W.CurrentUser.Me(ctx) require.NoError(t, err) - if user != nil { - testcli.PrepareReplacementsUser(t, replacements, *user) - } + require.NotNil(t, user) + testcli.PrepareReplacementsUser(t, replacements, *user) tmpDir := t.TempDir() testutil.Chdir(t, tmpDir) diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index 6c1d14ad69..5299ec5b38 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -204,8 +204,6 @@ func PrepareReplacementsUser(t testutil.TestingT, replacements *orderedmap.Order for ind, val := range u.Roles { setKV(replacements, val.Value, fmt.Sprintf("$USER.Roles[%d]", ind)) } - - // Schemas []UserSchema `json:"schemas,omitempty"` } func stableSortReverseLength(strs []string) { From f6e8f268e2c4fa79c192a66b29f39e6f309d0629 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 11:08:41 +0100 Subject: [PATCH 12/20] Remove orderedmap; use slice wrapped in struct --- go.mod | 1 - go.sum | 2 - internal/testcli/golden.go | 112 ++++++++++++++++++++----------------- 3 files changed, 62 insertions(+), 53 deletions(-) diff --git a/go.mod b/go.mod index 49f11668d5..748d554b61 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/Masterminds/semver/v3 v3.3.1 // MIT github.com/briandowns/spinner v1.23.1 // Apache 2.0 github.com/databricks/databricks-sdk-go v0.54.0 // Apache 2.0 - github.com/elliotchance/orderedmap/v3 v3.0.0 // MIT github.com/fatih/color v1.18.0 // MIT github.com/google/uuid v1.6.0 // BSD-3-Clause github.com/hashicorp/go-version v1.7.0 // MPL 2.0 diff --git a/go.sum b/go.sum index 3959a1c1e8..1e806ea036 100644 --- a/go.sum +++ b/go.sum @@ -37,8 +37,6 @@ github.com/databricks/databricks-sdk-go v0.54.0/go.mod h1:ds+zbv5mlQG7nFEU5ojLtg github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/elliotchance/orderedmap/v3 v3.0.0 h1:Yay/tDjX+vzza+Drcoo8VEbuBnOYGpgenCXWcpQSFDg= -github.com/elliotchance/orderedmap/v3 v3.0.0/go.mod h1:G+Hc2RwaZvJMcS4JpGCOyViCnGeKf0bTYCGTO4uhjSo= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index 5299ec5b38..cb24c4568d 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -16,7 +16,6 @@ import ( "github.com/databricks/cli/libs/testdiff" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/iam" - "github.com/elliotchance/orderedmap/v3" "github.com/stretchr/testify/require" ) @@ -95,10 +94,7 @@ func ReplaceOutput(t testutil.TestingT, ctx context.Context, out string) string if replacements == nil { t.Fatal("WithReplacementsMap was not called") } - for key, value := range replacements.AllFromFront() { - out = strings.ReplaceAll(out, key, value) - } - + out = replacements.Replace(out) out = uuidRegex.ReplaceAllString(out, "") out = numIdRegex.ReplaceAllString(out, "") out = privatePathRegex.ReplaceAllString(out, "/tmp/.../$3") @@ -112,67 +108,83 @@ const ( replacementsMapKey = key(1) ) -func WithReplacementsMap(ctx context.Context) (context.Context, *orderedmap.OrderedMap[string, string]) { +type Replacement struct { + Old string + New string +} + +type ReplacementsContext struct { + Repls []Replacement +} + +func (r *ReplacementsContext) Replace(s string) string { + for _, repl := range r.Repls { + s = strings.ReplaceAll(s, repl.Old, repl.New) + } + return s +} + +func (r *ReplacementsContext) Set(old, new string) { + if old == "" || new == "" { + return + } + r.Repls = append(r.Repls, Replacement{Old: old, New: new}) +} + +func WithReplacementsMap(ctx context.Context) (context.Context, *ReplacementsContext) { value := ctx.Value(replacementsMapKey) if value != nil { - if existingMap, ok := value.(*orderedmap.OrderedMap[string, string]); ok { + if existingMap, ok := value.(*ReplacementsContext); ok { return ctx, existingMap } } - newMap := orderedmap.NewOrderedMap[string, string]() + newMap := &ReplacementsContext{} ctx = context.WithValue(ctx, replacementsMapKey, newMap) return ctx, newMap } -func GetReplacementsMap(ctx context.Context) *orderedmap.OrderedMap[string, string] { +func GetReplacementsMap(ctx context.Context) *ReplacementsContext { value := ctx.Value(replacementsMapKey) if value != nil { - if existingMap, ok := value.(*orderedmap.OrderedMap[string, string]); ok { + if existingMap, ok := value.(*ReplacementsContext); ok { return existingMap } } return nil } -func setKV(replacements *orderedmap.OrderedMap[string, string], key, value string) { - if key == "" || value == "" { - return - } - replacements.Set(key, value) -} - -func PrepareReplacements(t testutil.TestingT, replacements *orderedmap.OrderedMap[string, string], w *databricks.WorkspaceClient) { +func PrepareReplacements(t testutil.TestingT, r *ReplacementsContext, w *databricks.WorkspaceClient) { // in some clouds (gcp) w.Config.Host includes "https://" prefix in others it's really just a host (azure) host := strings.TrimPrefix(strings.TrimPrefix(w.Config.Host, "http://"), "https://") - setKV(replacements, host, "$DATABRICKS_HOST") - setKV(replacements, w.Config.ClusterID, "$DATABRICKS_CLUSTER_ID") - setKV(replacements, w.Config.WarehouseID, "$DATABRICKS_WAREHOUSE_ID") - setKV(replacements, w.Config.ServerlessComputeID, "$DATABRICKS_SERVERLESS_COMPUTE_ID") - setKV(replacements, w.Config.MetadataServiceURL, "$DATABRICKS_METADATA_SERVICE_URL") - setKV(replacements, w.Config.AccountID, "$DATABRICKS_ACCOUNT_ID") - setKV(replacements, w.Config.Token, "$DATABRICKS_TOKEN") - setKV(replacements, w.Config.Username, "$DATABRICKS_USERNAME") - setKV(replacements, w.Config.Password, "$DATABRICKS_PASSWORD") - setKV(replacements, w.Config.Profile, "$DATABRICKS_CONFIG_PROFILE") - setKV(replacements, w.Config.ConfigFile, "$DATABRICKS_CONFIG_FILE") - setKV(replacements, w.Config.GoogleServiceAccount, "$DATABRICKS_GOOGLE_SERVICE_ACCOUNT") - setKV(replacements, w.Config.GoogleCredentials, "$GOOGLE_CREDENTIALS") - setKV(replacements, w.Config.AzureResourceID, "$DATABRICKS_AZURE_RESOURCE_ID") - setKV(replacements, w.Config.AzureClientSecret, "$ARM_CLIENT_SECRET") - // setKV(replacements, w.Config.AzureClientID, "$ARM_CLIENT_ID") - setKV(replacements, w.Config.AzureClientID, "$USERNAME") - setKV(replacements, w.Config.AzureTenantID, "$ARM_TENANT_ID") - setKV(replacements, w.Config.ActionsIDTokenRequestURL, "$ACTIONS_ID_TOKEN_REQUEST_URL") - setKV(replacements, w.Config.ActionsIDTokenRequestToken, "$ACTIONS_ID_TOKEN_REQUEST_TOKEN") - setKV(replacements, w.Config.AzureEnvironment, "$ARM_ENVIRONMENT") - setKV(replacements, w.Config.ClientID, "$DATABRICKS_CLIENT_ID") - setKV(replacements, w.Config.ClientSecret, "$DATABRICKS_CLIENT_SECRET") - setKV(replacements, w.Config.DatabricksCliPath, "$DATABRICKS_CLI_PATH") - setKV(replacements, w.Config.AuthType, "$DATABRICKS_AUTH_TYPE") -} - -func PrepareReplacementsUser(t testutil.TestingT, replacements *orderedmap.OrderedMap[string, string], u iam.User) { + r.Set(host, "$DATABRICKS_HOST") + r.Set(w.Config.ClusterID, "$DATABRICKS_CLUSTER_ID") + r.Set(w.Config.WarehouseID, "$DATABRICKS_WAREHOUSE_ID") + r.Set(w.Config.ServerlessComputeID, "$DATABRICKS_SERVERLESS_COMPUTE_ID") + r.Set(w.Config.MetadataServiceURL, "$DATABRICKS_METADATA_SERVICE_URL") + r.Set(w.Config.AccountID, "$DATABRICKS_ACCOUNT_ID") + r.Set(w.Config.Token, "$DATABRICKS_TOKEN") + r.Set(w.Config.Username, "$DATABRICKS_USERNAME") + r.Set(w.Config.Password, "$DATABRICKS_PASSWORD") + r.Set(w.Config.Profile, "$DATABRICKS_CONFIG_PROFILE") + r.Set(w.Config.ConfigFile, "$DATABRICKS_CONFIG_FILE") + r.Set(w.Config.GoogleServiceAccount, "$DATABRICKS_GOOGLE_SERVICE_ACCOUNT") + r.Set(w.Config.GoogleCredentials, "$GOOGLE_CREDENTIALS") + r.Set(w.Config.AzureResourceID, "$DATABRICKS_AZURE_RESOURCE_ID") + r.Set(w.Config.AzureClientSecret, "$ARM_CLIENT_SECRET") + // r.Set(w.Config.AzureClientID, "$ARM_CLIENT_ID") + r.Set(w.Config.AzureClientID, "$USERNAME") + r.Set(w.Config.AzureTenantID, "$ARM_TENANT_ID") + r.Set(w.Config.ActionsIDTokenRequestURL, "$ACTIONS_ID_TOKEN_REQUEST_URL") + r.Set(w.Config.ActionsIDTokenRequestToken, "$ACTIONS_ID_TOKEN_REQUEST_TOKEN") + r.Set(w.Config.AzureEnvironment, "$ARM_ENVIRONMENT") + r.Set(w.Config.ClientID, "$DATABRICKS_CLIENT_ID") + r.Set(w.Config.ClientSecret, "$DATABRICKS_CLIENT_SECRET") + r.Set(w.Config.DatabricksCliPath, "$DATABRICKS_CLI_PATH") + r.Set(w.Config.AuthType, "$DATABRICKS_AUTH_TYPE") +} + +func PrepareReplacementsUser(t testutil.TestingT, r *ReplacementsContext, u iam.User) { // There could be exact matches or overlap between different name fields, so sort them by length // to ensure we match the largest one first and map them all to the same token names := []string{ @@ -192,17 +204,17 @@ func PrepareReplacementsUser(t testutil.TestingT, replacements *orderedmap.Order stableSortReverseLength(names) for _, name := range names { - setKV(replacements, name, "$USERNAME") + r.Set(name, "$USERNAME") } for ind, val := range u.Groups { - setKV(replacements, val.Value, fmt.Sprintf("$USER.Groups[%d]", ind)) + r.Set(val.Value, fmt.Sprintf("$USER.Groups[%d]", ind)) } - setKV(replacements, u.Id, "$USER.Id") + r.Set(u.Id, "$USER.Id") for ind, val := range u.Roles { - setKV(replacements, val.Value, fmt.Sprintf("$USER.Roles[%d]", ind)) + r.Set(val.Value, fmt.Sprintf("$USER.Roles[%d]", ind)) } } From 8945ee34e9ca59060b38d85aa36e39c15af35676 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 13:38:45 +0100 Subject: [PATCH 13/20] switch to testutil.TestingT interface I've also considered https://pkg.go.dev/github.com/stretchr/testify/require#TestingT to avoid dependency on /internal/ but that one does not have Logf method that I use --- libs/testdiff/testdiff.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/testdiff/testdiff.go b/libs/testdiff/testdiff.go index b4ec6fe7d4..1e1df727ad 100644 --- a/libs/testdiff/testdiff.go +++ b/libs/testdiff/testdiff.go @@ -3,7 +3,6 @@ package testdiff import ( "fmt" "strings" - "testing" "github.com/databricks/cli/internal/testutil" "github.com/hexops/gotextdiff" @@ -29,7 +28,7 @@ func AssertEqualTexts(t testutil.TestingT, filename1, filename2, expected, out s } } -func AssertEqualJQ(t *testing.T, expectedName, outName, expected, out string, ignorePaths []string) { +func AssertEqualJQ(t testutil.TestingT, expectedName, outName, expected, out string, ignorePaths []string) { patch, err := jsondiff.CompareJSON([]byte(expected), []byte(out)) if err != nil { t.Logf("CompareJSON error for %s vs %s: %s (fallback to textual comparison)", outName, expectedName, err) From b91e7fb9052a2165253d4c66a65d7e61c2775c70 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 13:46:02 +0100 Subject: [PATCH 14/20] re-order replacement --- integration/bundle/init_default_python_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index b6ffb89600..1efdfff269 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -53,12 +53,11 @@ func testDefaultPython(t *testing.T, pythonVersion string) { ctx, replacements := testcli.WithReplacementsMap(ctx) replacements.Set(uniqueProjectId, "$UNIQUE_PRJ") - testcli.PrepareReplacements(t, replacements, wt.W) - user, err := wt.W.CurrentUser.Me(ctx) require.NoError(t, err) require.NotNil(t, user) testcli.PrepareReplacementsUser(t, replacements, *user) + testcli.PrepareReplacements(t, replacements, wt.W) tmpDir := t.TempDir() testutil.Chdir(t, tmpDir) From b645c47e87dbc65efaba3c4fe1bbd7fe22ee7957 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 13:49:08 +0100 Subject: [PATCH 15/20] disable $DATABRICKS_AUTH_TYPE and add a comment --- internal/testcli/golden.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index cb24c4568d..4f9bbb0e91 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -118,6 +118,7 @@ type ReplacementsContext struct { } func (r *ReplacementsContext) Replace(s string) string { + // QQQ Should probably only replace whole words for _, repl := range r.Repls { s = strings.ReplaceAll(s, repl.Old, repl.New) } @@ -181,7 +182,8 @@ func PrepareReplacements(t testutil.TestingT, r *ReplacementsContext, w *databri r.Set(w.Config.ClientID, "$DATABRICKS_CLIENT_ID") r.Set(w.Config.ClientSecret, "$DATABRICKS_CLIENT_SECRET") r.Set(w.Config.DatabricksCliPath, "$DATABRICKS_CLI_PATH") - r.Set(w.Config.AuthType, "$DATABRICKS_AUTH_TYPE") + // This is set to words like "path" that happen too frequently + //r.Set(w.Config.AuthType, "$DATABRICKS_AUTH_TYPE") } func PrepareReplacementsUser(t testutil.TestingT, r *ReplacementsContext, u iam.User) { From e685482e75d5256109055e2874459011df2c93fa Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 13:50:38 +0100 Subject: [PATCH 16/20] update NOTICE; fix formatting --- NOTICE | 4 ++++ internal/testcli/golden.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NOTICE b/NOTICE index fa214a49c7..f6b59e0b09 100644 --- a/NOTICE +++ b/NOTICE @@ -101,3 +101,7 @@ License - https://github.com/whilp/git-urls/blob/master/LICENSE github.com/wI2L/jsondiff v0.6.1 Copyright (c) 2020-2024 William Poussier License - https://github.com/wI2L/jsondiff/blob/master/LICENSE + +https://github.com/hexops/gotextdiff +Copyright (c) 2009 The Go Authors. All rights reserved. +License - https://github.com/hexops/gotextdiff/blob/main/LICENSE diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index 4f9bbb0e91..d11c05c32a 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -183,7 +183,7 @@ func PrepareReplacements(t testutil.TestingT, r *ReplacementsContext, w *databri r.Set(w.Config.ClientSecret, "$DATABRICKS_CLIENT_SECRET") r.Set(w.Config.DatabricksCliPath, "$DATABRICKS_CLI_PATH") // This is set to words like "path" that happen too frequently - //r.Set(w.Config.AuthType, "$DATABRICKS_AUTH_TYPE") + // r.Set(w.Config.AuthType, "$DATABRICKS_AUTH_TYPE") } func PrepareReplacementsUser(t testutil.TestingT, r *ReplacementsContext, u iam.User) { From 90b7413c353e052304146983c761cee6429db586 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 14:13:13 +0100 Subject: [PATCH 17/20] restore "go 1.23" line --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 748d554b61..2dda0cd609 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/databricks/cli -go 1.23.0 +go 1.23 toolchain go1.23.4 From 1e242cbeb11b3b29715bb8d612452bdbec3493df Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 14:23:22 +0100 Subject: [PATCH 18/20] rename RequireOutput to AssertOutput --- integration/bundle/init_default_python_test.go | 11 +++++------ internal/testcli/golden.go | 12 ++++++------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index 1efdfff269..4196852f02 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -88,17 +88,17 @@ func testDefaultPython(t *testing.T, pythonVersion string) { err = os.WriteFile(filepath.Join(tmpDir, "config.json"), b, 0o644) require.NoError(t, err) - testcli.RequireOutput(t, ctx, []string{"bundle", "init", "default-python", "--config-file", "config.json"}, "testdata/default_python/bundle_init.txt") + testcli.AssertOutput(t, ctx, []string{"bundle", "init", "default-python", "--config-file", "config.json"}, "testdata/default_python/bundle_init.txt") testutil.Chdir(t, projectName) - testcli.RequireOutput(t, ctx, []string{"bundle", "validate"}, "testdata/default_python/bundle_validate.txt") - - testcli.RequireOutput(t, ctx, []string{"bundle", "deploy"}, "testdata/default_python/bundle_deploy.txt") t.Cleanup(func() { // Delete the stack testcli.RequireSuccessfulRun(t, ctx, "bundle", "destroy", "--auto-approve") }) + testcli.AssertOutput(t, ctx, []string{"bundle", "validate"}, "testdata/default_python/bundle_validate.txt") + testcli.AssertOutput(t, ctx, []string{"bundle", "deploy"}, "testdata/default_python/bundle_deploy.txt") + ignoredFields := []string{ "/bundle/terraform/exec_path", "/resources/jobs/project_name_$UNIQUE_PRJ_job/email_notifications", @@ -108,6 +108,5 @@ func testDefaultPython(t *testing.T, pythonVersion string) { "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/url", "/workspace/current_user", } - - testcli.RequireOutputJQ(t, ctx, []string{"bundle", "summary", "--output", "json"}, "testdata/default_python/bundle_summary.txt", ignoredFields) + testcli.AssertOutputJQ(t, ctx, []string{"bundle", "summary", "--output", "json"}, "testdata/default_python/bundle_summary.txt", ignoredFields) } diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index d11c05c32a..f4f082cb39 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -16,7 +16,7 @@ import ( "github.com/databricks/cli/libs/testdiff" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/iam" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" ) var OverwriteMode = os.Getenv("TESTS_OUTPUT") == "OVERWRITE" @@ -26,7 +26,7 @@ func ReadFile(t testutil.TestingT, ctx context.Context, filename string) string if os.IsNotExist(err) { return "" } - require.NoError(t, err) + assert.NoError(t, err) // On CI, on Windows \n in the file somehow end up as \r\n return NormalizeNewlines(string(data)) } @@ -35,7 +35,7 @@ func captureOutput(t testutil.TestingT, ctx context.Context, args []string) stri t.Logf("run args: [%s]", strings.Join(args, ", ")) r := NewRunner(t, ctx, args...) stdout, stderr, err := r.Run() - require.NoError(t, err) + assert.NoError(t, err) out := stderr.String() + stdout.String() return ReplaceOutput(t, ctx, out) } @@ -43,10 +43,10 @@ func captureOutput(t testutil.TestingT, ctx context.Context, args []string) stri func WriteFile(t testutil.TestingT, filename, data string) { t.Logf("Overwriting %s", filename) err := os.WriteFile(filename, []byte(data), 0o644) - require.NoError(t, err) + assert.NoError(t, err) } -func RequireOutput(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string) { +func AssertOutput(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string) { _, filename, _, _ := runtime.Caller(1) dir := filepath.Dir(filename) expectedPath := filepath.Join(dir, expectedFilename) @@ -64,7 +64,7 @@ func RequireOutput(t testutil.TestingT, ctx context.Context, args []string, expe } } -func RequireOutputJQ(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string, ignorePaths []string) { +func AssertOutputJQ(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string, ignorePaths []string) { _, filename, _, _ := runtime.Caller(1) dir := filepath.Dir(filename) expectedPath := filepath.Join(dir, expectedFilename) From 418564aac1d128341a00d68d55556a04841586ad Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 14:43:29 +0100 Subject: [PATCH 19/20] get rid of "_, filename, _, _ := runtime.Caller(1)" Instead, store original WD on env var. --- .../bundle/init_default_python_test.go | 48 +++++++++++++------ internal/testcli/golden.go | 16 ++----- internal/testutil/env.go | 9 ++++ 3 files changed, 47 insertions(+), 26 deletions(-) diff --git a/integration/bundle/init_default_python_test.go b/integration/bundle/init_default_python_test.go index 4196852f02..9b65636e96 100644 --- a/integration/bundle/init_default_python_test.go +++ b/integration/bundle/init_default_python_test.go @@ -88,7 +88,12 @@ func testDefaultPython(t *testing.T, pythonVersion string) { err = os.WriteFile(filepath.Join(tmpDir, "config.json"), b, 0o644) require.NoError(t, err) - testcli.AssertOutput(t, ctx, []string{"bundle", "init", "default-python", "--config-file", "config.json"}, "testdata/default_python/bundle_init.txt") + testcli.AssertOutput( + t, + ctx, + []string{"bundle", "init", "default-python", "--config-file", "config.json"}, + testutil.TestData("testdata/default_python/bundle_init.txt"), + ) testutil.Chdir(t, projectName) t.Cleanup(func() { @@ -96,17 +101,32 @@ func testDefaultPython(t *testing.T, pythonVersion string) { testcli.RequireSuccessfulRun(t, ctx, "bundle", "destroy", "--auto-approve") }) - testcli.AssertOutput(t, ctx, []string{"bundle", "validate"}, "testdata/default_python/bundle_validate.txt") - testcli.AssertOutput(t, ctx, []string{"bundle", "deploy"}, "testdata/default_python/bundle_deploy.txt") - - ignoredFields := []string{ - "/bundle/terraform/exec_path", - "/resources/jobs/project_name_$UNIQUE_PRJ_job/email_notifications", - "/resources/jobs/project_name_$UNIQUE_PRJ_job/job_clusters/0/new_cluster/node_type_id", - "/resources/jobs/project_name_$UNIQUE_PRJ_job/url", - "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/catalog", - "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/url", - "/workspace/current_user", - } - testcli.AssertOutputJQ(t, ctx, []string{"bundle", "summary", "--output", "json"}, "testdata/default_python/bundle_summary.txt", ignoredFields) + testcli.AssertOutput( + t, + ctx, + []string{"bundle", "validate"}, + testutil.TestData("testdata/default_python/bundle_validate.txt"), + ) + testcli.AssertOutput( + t, + ctx, + []string{"bundle", "deploy"}, + testutil.TestData("testdata/default_python/bundle_deploy.txt"), + ) + + testcli.AssertOutputJQ( + t, + ctx, + []string{"bundle", "summary", "--output", "json"}, + testutil.TestData("testdata/default_python/bundle_summary.txt"), + []string{ + "/bundle/terraform/exec_path", + "/resources/jobs/project_name_$UNIQUE_PRJ_job/email_notifications", + "/resources/jobs/project_name_$UNIQUE_PRJ_job/job_clusters/0/new_cluster/node_type_id", + "/resources/jobs/project_name_$UNIQUE_PRJ_job/url", + "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/catalog", + "/resources/pipelines/project_name_$UNIQUE_PRJ_pipeline/url", + "/workspace/current_user", + }, + ) } diff --git a/internal/testcli/golden.go b/internal/testcli/golden.go index f4f082cb39..34f38f18a4 100644 --- a/internal/testcli/golden.go +++ b/internal/testcli/golden.go @@ -4,9 +4,7 @@ import ( "context" "fmt" "os" - "path/filepath" "regexp" - "runtime" "slices" "strings" "testing" @@ -46,17 +44,14 @@ func WriteFile(t testutil.TestingT, filename, data string) { assert.NoError(t, err) } -func AssertOutput(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string) { - _, filename, _, _ := runtime.Caller(1) - dir := filepath.Dir(filename) - expectedPath := filepath.Join(dir, expectedFilename) +func AssertOutput(t testutil.TestingT, ctx context.Context, args []string, expectedPath string) { expected := ReadFile(t, ctx, expectedPath) out := captureOutput(t, ctx, args) if out != expected { actual := fmt.Sprintf("Output from %v", args) - testdiff.AssertEqualTexts(t, expectedFilename, actual, expected, out) + testdiff.AssertEqualTexts(t, expectedPath, actual, expected, out) if OverwriteMode { WriteFile(t, expectedPath, out) @@ -64,17 +59,14 @@ func AssertOutput(t testutil.TestingT, ctx context.Context, args []string, expec } } -func AssertOutputJQ(t testutil.TestingT, ctx context.Context, args []string, expectedFilename string, ignorePaths []string) { - _, filename, _, _ := runtime.Caller(1) - dir := filepath.Dir(filename) - expectedPath := filepath.Join(dir, expectedFilename) +func AssertOutputJQ(t testutil.TestingT, ctx context.Context, args []string, expectedPath string, ignorePaths []string) { expected := ReadFile(t, ctx, expectedPath) out := captureOutput(t, ctx, args) if out != expected { actual := fmt.Sprintf("Output from %v", args) - testdiff.AssertEqualJQ(t.(*testing.T), expectedFilename, actual, expected, out, ignorePaths) + testdiff.AssertEqualJQ(t.(*testing.T), expectedPath, actual, expected, out, ignorePaths) if OverwriteMode { WriteFile(t, expectedPath, out) diff --git a/internal/testutil/env.go b/internal/testutil/env.go index 10557c4e63..2597519200 100644 --- a/internal/testutil/env.go +++ b/internal/testutil/env.go @@ -47,6 +47,9 @@ func Chdir(t TestingT, dir string) string { wd, err := os.Getwd() require.NoError(t, err) + if os.Getenv("TESTS_ORIG_WD") == "" { + t.Setenv("TESTS_ORIG_WD", wd) + } abs, err := filepath.Abs(dir) require.NoError(t, err) @@ -61,3 +64,9 @@ func Chdir(t TestingT, dir string) string { return wd } + +// Helper to get absolute path to testdata file. +// It only able to helps if case Chdir() above was called or directory was not changed at all. +func TestData(filename string) string { + return filepath.Join(os.Getenv("TESTS_ORIG_WD"), filename) +} From 74345897c2b043417cdce3fbc2a2b876c83a82d6 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Dec 2024 15:18:59 +0100 Subject: [PATCH 20/20] Comment on TestData --- internal/testutil/env.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/testutil/env.go b/internal/testutil/env.go index 2597519200..598229655f 100644 --- a/internal/testutil/env.go +++ b/internal/testutil/env.go @@ -65,8 +65,9 @@ func Chdir(t TestingT, dir string) string { return wd } -// Helper to get absolute path to testdata file. -// It only able to helps if case Chdir() above was called or directory was not changed at all. +// Return filename ff testutil.Chdir was not called. +// Return absolute path to filename testutil.Chdir() was called. func TestData(filename string) string { + // Note, if TESTS_ORIG_WD is not set, Getenv return "" and Join returns filename return filepath.Join(os.Getenv("TESTS_ORIG_WD"), filename) }