From 2f798c4dedd7e4fc18d9f31cbd55135345e1b208 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 3 Feb 2025 11:03:18 +0100 Subject: [PATCH 1/3] acc: Remove initial '$CLI --version' call (#2280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is proven to be not necessary. ``` ~/work/cli/acceptance % hyperfine -w 2 'go test' # with change: Benchmark 1: go test Time (mean ± σ): 4.983 s ± 0.209 s [User: 6.073 s, System: 9.869 s] Range (min … max): 4.792 s … 5.483 s 10 runs ~/work/cli/acceptance % git stash # without change: ~/work/cli/acceptance % hyperfine -w 2 'go test' Benchmark 1: go test Time (mean ± σ): 5.018 s ± 0.100 s [User: 6.142 s, System: 10.234 s] Range (min … max): 4.899 s … 5.182 s 10 runs ``` --- acceptance/acceptance_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 5f0030eec..571168ca8 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -448,7 +448,6 @@ func BuildCLI(t *testing.T, buildDir, coverDir string) string { } RunCommand(t, args, "..") - RunCommand(t, []string{execPath, "--version"}, ".") return execPath } From fcedfe4c781a1f6de8f23d6e477ed045d91921ea Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 3 Feb 2025 11:29:13 +0100 Subject: [PATCH 2/3] acc: Consistent & detailed output for file issues (#2279) ## Changes - Include compact relPath in the error message title. Include full paths in separate lines below. - Previously sometimes full paths were printed, sometime only rel path. ## Tests Manually trigger the errors. --- acceptance/acceptance_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 571168ca8..cad8acf4f 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -335,7 +335,7 @@ func doComparison(t *testing.T, repls testdiff.ReplacementsContext, dirRef, dirN bufRef, okRef := tryReading(t, pathRef) bufNew, okNew := tryReading(t, pathNew) if !okRef && !okNew { - t.Errorf("Both files are missing or have errors: %s, %s", pathRef, pathNew) + t.Errorf("Both files are missing or have errors: %s\npathRef: %s\npathNew: %s", relPath, pathRef, pathNew) return } @@ -350,7 +350,7 @@ func doComparison(t *testing.T, repls testdiff.ReplacementsContext, dirRef, dirN // The test did not produce an expected output file. if okRef && !okNew { - t.Errorf("Missing output file: %s", relPath) + t.Errorf("Missing output file: %s\npathRef: %s\npathNew: %s", relPath, pathRef, pathNew) testdiff.AssertEqualTexts(t, pathRef, pathNew, valueRef, valueNew) if testdiff.OverwriteMode { t.Logf("Removing output file: %s", relPath) @@ -361,7 +361,7 @@ func doComparison(t *testing.T, repls testdiff.ReplacementsContext, dirRef, dirN // The test produced an unexpected output file. if !okRef && okNew { - t.Errorf("Unexpected output file: %s", relPath) + t.Errorf("Unexpected output file: %s\npathRef: %s\npathNew: %s", relPath, pathRef, pathNew) testdiff.AssertEqualTexts(t, pathRef, pathNew, valueRef, valueNew) if testdiff.OverwriteMode { t.Logf("Writing output file: %s", relPath) From f267318bb940849c8833308128ff9eaf08c35aa6 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 3 Feb 2025 11:43:25 +0100 Subject: [PATCH 3/3] Include acceptance tests in integration tests (#2242) ## Changes - Include acceptance directory in integration tests. Acceptance tests will not start local server if CLOUD_ENV is set, so they become integration tests. - Add dependency for vendor to integration, so that CLI can be build there. - Implement LocalOnly option in test.toml to opt out of running acceptance tests as integration tests. Use it in certain tests that are difficult or not necessary to fix when run as integration tests. - Update terraform test to redact timings out. - Clean up .workspace.current_user from outputs of the tests. ## Tests Existing tests. --- Makefile | 6 +++--- acceptance/acceptance_test.go | 5 +++++ acceptance/bundle/scripts/test.toml | 1 + acceptance/bundle/templates/test.toml | 2 ++ .../bundle/variables/env_overrides/test.toml | 2 ++ acceptance/bundle/variables/git-branch/output.txt | 14 ++------------ acceptance/bundle/variables/git-branch/script | 6 +++--- .../variables/prepend-workspace-var/output.txt | 7 +------ .../bundle/variables/prepend-workspace-var/script | 2 +- .../bundle/variables/resolve-builtin/output.txt | 5 ----- acceptance/bundle/variables/resolve-builtin/script | 2 +- .../bundle/variables/resolve-builtin/test.toml | 2 ++ .../variables/resolve-vars-in-root-path/output.txt | 5 ----- .../variables/resolve-vars-in-root-path/script | 2 +- .../variables/resolve-vars-in-root-path/test.toml | 2 ++ acceptance/config_test.go | 3 +++ acceptance/terraform/output.txt | 2 +- acceptance/terraform/test.toml | 3 +++ acceptance/workspace/jobs/create/test.toml | 1 + 19 files changed, 34 insertions(+), 38 deletions(-) create mode 100644 acceptance/bundle/scripts/test.toml create mode 100644 acceptance/bundle/templates/test.toml create mode 100644 acceptance/bundle/variables/env_overrides/test.toml create mode 100644 acceptance/bundle/variables/resolve-builtin/test.toml create mode 100644 acceptance/bundle/variables/resolve-vars-in-root-path/test.toml create mode 100644 acceptance/terraform/test.toml diff --git a/Makefile b/Makefile index d30ccef14..e18727934 100644 --- a/Makefile +++ b/Makefile @@ -51,12 +51,12 @@ schema: docs: go run ./bundle/docsgen ./bundle/internal/schema ./bundle/docsgen -INTEGRATION = gotestsum --format github-actions --rerun-fails --jsonfile output.json --packages "./integration/..." -- -parallel 4 -timeout=2h +INTEGRATION = gotestsum --format github-actions --rerun-fails --jsonfile output.json --packages "./acceptance ./integration/..." -- -parallel 4 -timeout=2h -integration: +integration: vendor $(INTEGRATION) -integration-short: +integration-short: vendor $(INTEGRATION) -short .PHONY: lint lintcheck fmt test cover showcover build snapshot vendor schema integration integration-short acc-cover acc-showcover docs diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index cad8acf4f..ccdf74bcb 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -209,6 +209,11 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont t.Skipf("Disabled via GOOS.%s setting in %s", runtime.GOOS, configPath) } + cloudEnv := os.Getenv("CLOUD_ENV") + if config.LocalOnly && cloudEnv != "" { + t.Skipf("Disabled via LocalOnly setting in %s (CLOUD_ENV=%s)", configPath, cloudEnv) + } + var tmpDir string var err error if KeepTmp { diff --git a/acceptance/bundle/scripts/test.toml b/acceptance/bundle/scripts/test.toml new file mode 100644 index 000000000..1dbd78681 --- /dev/null +++ b/acceptance/bundle/scripts/test.toml @@ -0,0 +1 @@ +LocalOnly = true # Deployment currently fails when run locally; once that is fixed, remove this setting diff --git a/acceptance/bundle/templates/test.toml b/acceptance/bundle/templates/test.toml new file mode 100644 index 000000000..90539263d --- /dev/null +++ b/acceptance/bundle/templates/test.toml @@ -0,0 +1,2 @@ +# At the moment, there are many differences across different envs w.r.t to catalog use, node type and so on. +LocalOnly = true diff --git a/acceptance/bundle/variables/env_overrides/test.toml b/acceptance/bundle/variables/env_overrides/test.toml new file mode 100644 index 000000000..439c2fab1 --- /dev/null +++ b/acceptance/bundle/variables/env_overrides/test.toml @@ -0,0 +1,2 @@ +# Cloud run fails with Error: failed to resolve cluster-policy: wrong-cluster-policy, err: Policy named 'wrong-cluster-policy' does not exist +LocalOnly = true diff --git a/acceptance/bundle/variables/git-branch/output.txt b/acceptance/bundle/variables/git-branch/output.txt index 5e7664f61..21ed9e7de 100644 --- a/acceptance/bundle/variables/git-branch/output.txt +++ b/acceptance/bundle/variables/git-branch/output.txt @@ -6,7 +6,7 @@ "git": { "actual_branch": "main", "branch": "", - "bundle_root_path": ".", + "bundle_root_path": "." }, "name": "git", "target": "prod", @@ -28,11 +28,6 @@ }, "workspace": { "artifact_path": "/Workspace/Users/$USERNAME/.bundle/git/prod/artifacts", - "current_user": { - "id": "$USER.Id", - "short_name": "$USERNAME", - "userName": "$USERNAME" - }, "file_path": "/Workspace/Users/$USERNAME/.bundle/git/prod/files", "resource_path": "/Workspace/Users/$USERNAME/.bundle/git/prod/resources", "root_path": "/Workspace/Users/$USERNAME/.bundle/git/prod", @@ -56,7 +51,7 @@ Validation OK! "git": { "actual_branch": "main", "branch": "dev-branch", - "bundle_root_path": ".", + "bundle_root_path": "." }, "name": "git", "target": "dev", @@ -78,11 +73,6 @@ Validation OK! }, "workspace": { "artifact_path": "/Workspace/Users/$USERNAME/.bundle/git/dev/artifacts", - "current_user": { - "id": "$USER.Id", - "short_name": "$USERNAME", - "userName": "$USERNAME" - }, "file_path": "/Workspace/Users/$USERNAME/.bundle/git/dev/files", "resource_path": "/Workspace/Users/$USERNAME/.bundle/git/dev/resources", "root_path": "/Workspace/Users/$USERNAME/.bundle/git/dev", diff --git a/acceptance/bundle/variables/git-branch/script b/acceptance/bundle/variables/git-branch/script index aed881f1f..8f99cc01b 100644 --- a/acceptance/bundle/variables/git-branch/script +++ b/acceptance/bundle/variables/git-branch/script @@ -1,6 +1,6 @@ git-repo-init -trace $CLI bundle validate -o json | grep -v '"commit"' +trace $CLI bundle validate -o json | jq 'del(.workspace.current_user, .bundle.git.commit)' trace $CLI bundle validate -trace $CLI bundle validate -o json -t dev | grep -v '"commit"' -trace $CLI bundle validate -t dev | grep -v '"commit"' +trace $CLI bundle validate -o json -t dev | jq 'del(.workspace.current_user, .bundle.git.commit)' +trace $CLI bundle validate -t dev rm -fr .git diff --git a/acceptance/bundle/variables/prepend-workspace-var/output.txt b/acceptance/bundle/variables/prepend-workspace-var/output.txt index ed6c2b2af..93b652894 100644 --- a/acceptance/bundle/variables/prepend-workspace-var/output.txt +++ b/acceptance/bundle/variables/prepend-workspace-var/output.txt @@ -53,15 +53,10 @@ }, "workspace": { "artifact_path": "/Users/$USERNAME/path/to/root/artifacts", - "current_user": { - "id": "$USER.Id", - "short_name": "$USERNAME", - "userName": "$USERNAME" - }, "file_path": "/Users/$USERNAME/path/to/root/files", "profile": "profile_name", "resource_path": "/Users/$USERNAME/path/to/root/resources", "root_path": "/Users/$USERNAME/path/to/root", "state_path": "/Users/$USERNAME/path/to/root/state" } -} \ No newline at end of file +} diff --git a/acceptance/bundle/variables/prepend-workspace-var/script b/acceptance/bundle/variables/prepend-workspace-var/script index de6bc8a17..e30ffb9c4 100644 --- a/acceptance/bundle/variables/prepend-workspace-var/script +++ b/acceptance/bundle/variables/prepend-workspace-var/script @@ -1,2 +1,2 @@ echo /Workspace should be prepended on all paths, but it is not the case: #2181 -$CLI bundle validate -o json +$CLI bundle validate -o json | jq 'del(.workspace.current_user)' diff --git a/acceptance/bundle/variables/resolve-builtin/output.txt b/acceptance/bundle/variables/resolve-builtin/output.txt index 0c1678f84..f37a2a19e 100644 --- a/acceptance/bundle/variables/resolve-builtin/output.txt +++ b/acceptance/bundle/variables/resolve-builtin/output.txt @@ -1,10 +1,5 @@ { "artifact_path": "TestResolveVariableReferences/bar/artifacts", - "current_user": { - "id": "$USER.Id", - "short_name": "$USERNAME", - "userName": "$USERNAME" - }, "file_path": "TestResolveVariableReferences/bar/baz", "resource_path": "TestResolveVariableReferences/bar/resources", "root_path": "TestResolveVariableReferences/bar", diff --git a/acceptance/bundle/variables/resolve-builtin/script b/acceptance/bundle/variables/resolve-builtin/script index fefd9abe6..558d0a7ca 100644 --- a/acceptance/bundle/variables/resolve-builtin/script +++ b/acceptance/bundle/variables/resolve-builtin/script @@ -1 +1 @@ -$CLI bundle validate -o json | jq .workspace +$CLI bundle validate -o json | jq .workspace | jq 'del(.current_user)' diff --git a/acceptance/bundle/variables/resolve-builtin/test.toml b/acceptance/bundle/variables/resolve-builtin/test.toml new file mode 100644 index 000000000..085fab6c0 --- /dev/null +++ b/acceptance/bundle/variables/resolve-builtin/test.toml @@ -0,0 +1,2 @@ +# Cloud run fails with Error: Path (TestResolveVariableReferences/bar/baz) doesn't start with '/' +LocalOnly = true diff --git a/acceptance/bundle/variables/resolve-vars-in-root-path/output.txt b/acceptance/bundle/variables/resolve-vars-in-root-path/output.txt index 51eb40c91..fb828d826 100644 --- a/acceptance/bundle/variables/resolve-vars-in-root-path/output.txt +++ b/acceptance/bundle/variables/resolve-vars-in-root-path/output.txt @@ -1,10 +1,5 @@ { "artifact_path": "TestResolveVariableReferencesToBundleVariables/bar/artifacts", - "current_user": { - "id": "$USER.Id", - "short_name": "$USERNAME", - "userName": "$USERNAME" - }, "file_path": "TestResolveVariableReferencesToBundleVariables/bar/files", "resource_path": "TestResolveVariableReferencesToBundleVariables/bar/resources", "root_path": "TestResolveVariableReferencesToBundleVariables/bar", diff --git a/acceptance/bundle/variables/resolve-vars-in-root-path/script b/acceptance/bundle/variables/resolve-vars-in-root-path/script index fefd9abe6..558d0a7ca 100644 --- a/acceptance/bundle/variables/resolve-vars-in-root-path/script +++ b/acceptance/bundle/variables/resolve-vars-in-root-path/script @@ -1 +1 @@ -$CLI bundle validate -o json | jq .workspace +$CLI bundle validate -o json | jq .workspace | jq 'del(.current_user)' diff --git a/acceptance/bundle/variables/resolve-vars-in-root-path/test.toml b/acceptance/bundle/variables/resolve-vars-in-root-path/test.toml new file mode 100644 index 000000000..d833bd848 --- /dev/null +++ b/acceptance/bundle/variables/resolve-vars-in-root-path/test.toml @@ -0,0 +1,2 @@ +# Cloud run fails with Error: Path (TestResolveVariableReferencesToBundleVariables/bar/files) doesn't start with '/' +LocalOnly = true diff --git a/acceptance/config_test.go b/acceptance/config_test.go index beceb6a08..c7be223de 100644 --- a/acceptance/config_test.go +++ b/acceptance/config_test.go @@ -26,6 +26,9 @@ type TestConfig struct { // If absent, default to true. GOOS map[string]bool + // If true, do not run this test against cloud environment + LocalOnly bool + // List of additional replacements to apply on this test. // Old is a regexp, New is a replacement expression. Repls []testdiff.Replacement diff --git a/acceptance/terraform/output.txt b/acceptance/terraform/output.txt index c3d453ea5..32589ddab 100644 --- a/acceptance/terraform/output.txt +++ b/acceptance/terraform/output.txt @@ -37,7 +37,7 @@ commands will detect it and remind you to do so if necessary. >>> $TERRAFORM plan -no-color data.databricks_current_user.me: Reading... -data.databricks_current_user.me: Read complete after 0s [id=$USER.Id] +data.databricks_current_user.me: Read complete after (redacted) [id=$USER.Id] Changes to Outputs: + username = "$USERNAME" diff --git a/acceptance/terraform/test.toml b/acceptance/terraform/test.toml new file mode 100644 index 000000000..a6849e30f --- /dev/null +++ b/acceptance/terraform/test.toml @@ -0,0 +1,3 @@ +[[Repls]] +Old = 'Read complete after [^\s]+' +New = 'Read complete after (redacted)' diff --git a/acceptance/workspace/jobs/create/test.toml b/acceptance/workspace/jobs/create/test.toml index 1bf36547b..e69569c18 100644 --- a/acceptance/workspace/jobs/create/test.toml +++ b/acceptance/workspace/jobs/create/test.toml @@ -1,3 +1,4 @@ +LocalOnly = true # request recording currently does not work with cloud environment RecordRequests = true [[Server]]