Add acceptance tests #2081

denik · 2025-01-06T11:59:28Z

Changes

New kind of test is added - acceptance tests. See acceptance/README.md for explanation.
A few tests are converted to acceptance tests by moving databricks.yml to acceptance/ and adding corresponding script files.

As these tests run against compiled binary and can capture full output of the command, they can be useful to support major changes such as refactoring internal logging / diagnostics or complex variable interpolation.

These are currently run as part of 'make test' but the intention is to run them as part of integration tests as well.

Benefits

Full binary is tested, exactly as users get it.
- We're not testing custom set of mutators like many existing tests.
- Not mocking anything, real SDK is used (although the HTTP endpoint is not a real Databricks env).
Easy to maintain: output can be updated automatically.
Can easily set up external env, such as env vars, CLI args, .databrickscfg location etc.

Gaps

The tests currently share the test server and there is global place to define handlers. We should have a way for tests to override / add new handlers.

Tests

I manually checked that output of new acceptance tests matches previous asserts.

denik · 2025-01-06T14:53:23Z

https://github.com/databricks-eng/eng-dev-ecosystem/actions/runs/12634671597

andrewnester · 2025-01-06T14:29:34Z

acceptance/variables/complex/script

@@ -0,0 +1,4 @@
+$CLI bundle validate -o json | jq '{resources,variables}' > out.default.json


since it's a bash script, shall we better name it script.sh to make it more clear?

I don't know if 'bash' is going to stay or it is going to be the only thing that's used.

I think we might want to support other runners, especially for interactive use case. For example, expect or pexpect https://pexpect.readthedocs.io/en/stable/overview.html

Idea: We could use the extension to determine the runner? script.sh vs script.py?

We could still do that once we have add another runner. script for current bash case and script.py for python runner.

acceptance/acceptance_test.go

1. move databricks.yml; remove workspace.host from it. 2. add script 3. generate output.txt: TESTS_OUTPUT=OVERWRITE go test -v ./... -run TestAccept/clusters

pietern

A few small remaining things. Approving to unblock merge.

pietern · 2025-01-08T10:52:44Z

acceptance/acceptance_test.go

+			break
+		}
+
+		dir = filepath.Dir(dir)


Please add a require.True(t, filepath.IsLocal(dir)) in there to make sure this remains true.

acceptance/acceptance_test.go

acceptance/build/.gitignore

acceptance/merge-string-map/databricks.yml

denik had a problem deploying to test-trigger-is January 6, 2025 11:59 — with GitHub Actions Failure

denik force-pushed the denik/acceptance-tests branch from b036c0c to 60b1a77 Compare January 6, 2025 12:06

denik had a problem deploying to test-trigger-is January 6, 2025 12:06 — with GitHub Actions Failure

denik had a problem deploying to test-trigger-is January 6, 2025 12:10 — with GitHub Actions Failure

denik force-pushed the denik/acceptance-tests branch from d9a41f8 to 1dbab3c Compare January 6, 2025 12:22

denik had a problem deploying to test-trigger-is January 6, 2025 12:22 — with GitHub Actions Failure

denik force-pushed the denik/acceptance-tests branch from 1dbab3c to 28a69a1 Compare January 6, 2025 12:23

denik had a problem deploying to test-trigger-is January 6, 2025 12:23 — with GitHub Actions Failure

denik had a problem deploying to test-trigger-is January 6, 2025 12:29 — with GitHub Actions Failure

denik had a problem deploying to test-trigger-is January 6, 2025 12:38 — with GitHub Actions Failure

denik had a problem deploying to test-trigger-is January 6, 2025 12:40 — with GitHub Actions Failure

denik had a problem deploying to test-trigger-is January 6, 2025 12:43 — with GitHub Actions Failure

denik force-pushed the denik/acceptance-tests branch from 5e0f5f3 to 30aa795 Compare January 6, 2025 12:48

denik had a problem deploying to test-trigger-is January 6, 2025 12:48 — with GitHub Actions Failure

denik changed the base branch from main to gotestsum-format January 6, 2025 12:49

denik temporarily deployed to test-trigger-is January 6, 2025 12:56 — with GitHub Actions Inactive

denik had a problem deploying to test-trigger-is January 6, 2025 12:56 — with GitHub Actions Failure

Base automatically changed from gotestsum-format to main January 6, 2025 13:12

denik had a problem deploying to test-trigger-is January 6, 2025 13:13 — with GitHub Actions Failure

denik had a problem deploying to test-trigger-is January 6, 2025 13:31 — with GitHub Actions Failure

denik had a problem deploying to test-trigger-is January 6, 2025 13:37 — with GitHub Actions Failure

denik force-pushed the denik/acceptance-tests branch from 6be8afb to 151e62f Compare January 6, 2025 13:38

denik had a problem deploying to test-trigger-is January 6, 2025 13:38 — with GitHub Actions Failure

denik marked this pull request as ready for review January 6, 2025 13:44

denik requested review from pietern, andrewnester and shreyas-goenka as code owners January 6, 2025 13:44

denik had a problem deploying to test-trigger-is January 6, 2025 14:36 — with GitHub Actions Failure

andrewnester reviewed Jan 6, 2025

View reviewed changes

denik added 16 commits January 8, 2025 00:34

normalize new lines when comparing

2cd6db2

Use USERPROFILE on Windows

9c7f7dd

Rename TestAll to TestAccept

aa3f16d

migrate bundle/tests/clusters_test.go to acceptance/clusters

20c3b7b

1. move databricks.yml; remove workspace.host from it. 2. add script 3. generate output.txt: TESTS_OUTPUT=OVERWRITE go test -v ./... -run TestAccept/clusters

Add a test case for complex-transtive from Andrew

e1f7e0b

make "script" a constant

2f7a9fe

Add a test case for merging string and a map

e7534da

Fix merge-string-map

d8b0828

add non-json run to merge-string-map

ea27a33

convert override_job_tasks_test.go to acceptance test

6168f45

rename override-job-tasks to override/job_tasks

3382cb5

convert override_job_cluster_test.go to acceptance test

f5d658f

convert override_pipeline_cluster_test.go to acceptance test

bd25799

Add comments re env vars

6b4421d

simplify map -> slice

41248ba

do not copy prepare/cleanup scripts

2d6504e

denik force-pushed the denik/acceptance-tests branch from 384b212 to 2d6504e Compare January 7, 2025 23:35

denik temporarily deployed to test-trigger-is January 7, 2025 23:35 — with GitHub Actions Inactive

pietern approved these changes Jan 8, 2025

View reviewed changes

denik added 3 commits January 8, 2025 13:07

rename databricks-cli to databricks

bdc1782

Reorg tests; move appropriately under bundle & bundle/override

ed16fca

add require IsLocal

6088a3f

denik temporarily deployed to test-trigger-is January 8, 2025 12:15 — with GitHub Actions Inactive

denik enabled auto-merge January 8, 2025 12:16

remove potentially confusing comment

2e150fd

denik temporarily deployed to test-trigger-is January 8, 2025 12:20 — with GitHub Actions Inactive

denik added this pull request to the merge queue Jan 8, 2025

Merged via the queue into main with commit 185bbd2 Jan 8, 2025
9 checks passed

denik deleted the denik/acceptance-tests branch January 8, 2025 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add acceptance tests #2081

Add acceptance tests #2081

denik commented Jan 6, 2025 •

edited

Loading

denik commented Jan 6, 2025

andrewnester Jan 6, 2025

denik Jan 6, 2025 •

edited

Loading

shreyas-goenka Jan 7, 2025

denik Jan 7, 2025

pietern left a comment

pietern Jan 8, 2025

		@@ -0,0 +1,4 @@
		$CLI bundle validate -o json \| jq '{resources,variables}' > out.default.json

Add acceptance tests #2081

Add acceptance tests #2081

Conversation

denik commented Jan 6, 2025 • edited Loading

Changes

Benefits

Gaps

Tests

denik commented Jan 6, 2025

andrewnester Jan 6, 2025

Choose a reason for hiding this comment

denik Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

shreyas-goenka Jan 7, 2025

Choose a reason for hiding this comment

denik Jan 7, 2025

Choose a reason for hiding this comment

pietern left a comment

Choose a reason for hiding this comment

pietern Jan 8, 2025

Choose a reason for hiding this comment

denik commented Jan 6, 2025 •

edited

Loading

denik Jan 6, 2025 •

edited

Loading