Include acceptance tests in integration tests #2242

denik · 2025-01-27T15:44:56Z

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.

… utf8 (#2244) ## Changes - Do not start replacement / comparison if file is too large or not valid utf-8. - This helps to prevent replacements if there is accidentally a large binary (e.g. terraform). ## Tests Found this problem when working on #2242 -- the tests tried to applied replacements on terraform binary and crashed. With this change, an error is reported instead.

## Changes - If CLOUD_ENV is set to do not override with dummy value. This allows running acceptance tests as integration tests. - Needed for #2242 ## Tests Manually run the test suite against dogfood. `CLOUD_ENV=aws go test ./acceptance`

It uses workspace.profile, which is used before variables are interpolated, which breaks when this test is run against cloud. Needed for #2242

This does not work when this test is run against cloud. Needed for #2242

pietern · 2025-01-29T09:20:31Z

acceptance/bundle/templates/default-sql/test.toml

@@ -0,0 +1,12 @@
+[[Repls]]
+Old = '- (user_name|service_principal_name): \$USERNAME'
+New = '- user_name_OR_service_principal_name: $$USERNAME'


Can you include a comment explaining why these replacements need to happen? Good for posterity.

Good idea. For now, I've disabled bundle/templates in cloud env as there are many differences that deserve their own PR.

pietern · 2025-02-03T10:27:43Z

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]


Unrelated to this PR: why is there a test that runs TF directly?

Well it tests that terraform installation works. By manually adding TF_LOG you can also see that local provider is read.

pietern · 2025-02-03T10:28:29Z

acceptance/acceptance_test.go

+	cloudEnv := os.Getenv("CLOUD_ENV")
+	if config.LocalOnly && cloudEnv != "" {
+		t.Skipf("Disabled via LocalOnly setting in %s (CLOUD_ENV=%s)", configPath, cloudEnv)
+	}


I suspect that the presence of config.Server implies the local-only mode?

Interesting thought. I'd rather each option to be independent, even if in practice they correlate.

However, in case of Server & LocalOnly I don't see that -- you might need some stubs to run it locally but it already works with cloud.

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

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

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

denik force-pushed the denik/acceptance-integration branch from b6fee09 to 3284091 Compare January 27, 2025 16:16

denik temporarily deployed to test-trigger-is January 27, 2025 16:16 — with GitHub Actions Inactive

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

This was referenced Jan 27, 2025

acc: Avoid reading and applying replacements on large files; validate utf8 #2244

Merged

acc: Use real terraform when CLOUD_ENV is set #2245

Merged

denik force-pushed the denik/acceptance-integration branch from 53cbbee to 18e71cb Compare January 28, 2025 11:52

denik temporarily deployed to test-trigger-is January 28, 2025 11:52 — with GitHub Actions Inactive

denik force-pushed the denik/acceptance-integration branch from 18e71cb to d4003d7 Compare January 28, 2025 12:03

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

denik force-pushed the denik/acceptance-integration branch from d4003d7 to 6b9be38 Compare January 28, 2025 13:57

denik temporarily deployed to test-trigger-is January 28, 2025 13:57 — with GitHub Actions Inactive

denik mentioned this pull request Jan 28, 2025

acc: Disable git hooks #2249

Merged

denik added a commit that referenced this pull request Jan 28, 2025

Fix acceptance/bundle/variables/env_overrides when running on cloud

6392550

It uses workspace.profile, which is used before variables are interpolated, which breaks when this test is run against cloud. Needed for #2242

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

denik added a commit that referenced this pull request Jan 28, 2025

Fix env_overrides not to use variables in workspace.profile

1f845ab

This does not work when this test is run against cloud. Needed for #2242

denik mentioned this pull request Jan 28, 2025

Fix env_overrides not to use variables in workspace.profile #2251

Merged

denik added a commit that referenced this pull request Jan 28, 2025

Fix env_overrides not to use variables in workspace.profile

95e9d47

This does not work when this test is run against cloud. Needed for #2242

denik force-pushed the denik/acceptance-integration branch from 6392550 to 8ad39c9 Compare January 28, 2025 14:16

denik temporarily deployed to test-trigger-is January 28, 2025 14:16 — with GitHub Actions Inactive

denik added a commit that referenced this pull request Jan 28, 2025

Fix env_overrides not to use variables in workspace.profile (#2251)

4ba222a

This does not work when this test is run against cloud. Needed for #2242

denik temporarily deployed to test-trigger-is January 28, 2025 14:28 — with GitHub Actions Inactive

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

denik mentioned this pull request Jan 29, 2025

Add acceptance tests for bundle deploy #2254

Open

pietern reviewed Jan 29, 2025

View reviewed changes

denik force-pushed the denik/acceptance-integration branch from 65c44cc to 9971970 Compare January 29, 2025 14:17

denik temporarily deployed to test-trigger-is February 2, 2025 17:31 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is February 2, 2025 17:33 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is February 2, 2025 19:02 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is February 2, 2025 19:06 — with GitHub Actions Inactive

denik force-pushed the denik/acceptance-integration branch from 134072a to c05f339 Compare February 3, 2025 07:57

denik temporarily deployed to test-trigger-is February 3, 2025 07:57 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is February 3, 2025 08:00 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is February 3, 2025 08:04 — with GitHub Actions Inactive

denik added 10 commits February 3, 2025 09:08

Make default-sql work with service principal

5591eeb

Implement LocalOnly option

880f75c

terraform test: redact timings

dad8324

clean up .workspace.current_user

0ef8bdb

clean up current_user

f98bbec

disable resolve-builtin resolve-vars-in-root-path

1d25baf

resolve env_overrides

1b8e23c

disable templates

a22b2d8

clean up

1052746

don't include acceptance recursively

5c62de0

denik force-pushed the denik/acceptance-integration branch from c96ffb2 to 5c62de0 Compare February 3, 2025 08:08

denik temporarily deployed to test-trigger-is February 3, 2025 08:08 — with GitHub Actions Inactive

denik marked this pull request as ready for review February 3, 2025 08:22

denik requested review from andrewnester and shreyas-goenka as code owners February 3, 2025 08:22

denik requested a review from pietern February 3, 2025 08:22

pietern approved these changes Feb 3, 2025

View reviewed changes

denik added this pull request to the merge queue Feb 3, 2025

Merged via the queue into main with commit f267318 Feb 3, 2025
9 checks passed

denik deleted the denik/acceptance-integration branch February 3, 2025 10:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include acceptance tests in integration tests #2242

Include acceptance tests in integration tests #2242

denik commented Jan 27, 2025 •

edited

Loading

pietern Jan 29, 2025

denik Feb 3, 2025

pietern Feb 3, 2025

denik Feb 3, 2025

pietern Feb 3, 2025

denik Feb 3, 2025 •

edited

Loading

Include acceptance tests in integration tests #2242

Include acceptance tests in integration tests #2242

Conversation

denik commented Jan 27, 2025 • edited Loading

Changes

Tests

pietern Jan 29, 2025

Choose a reason for hiding this comment

denik Feb 3, 2025

Choose a reason for hiding this comment

pietern Feb 3, 2025

Choose a reason for hiding this comment

denik Feb 3, 2025

Choose a reason for hiding this comment

pietern Feb 3, 2025

Choose a reason for hiding this comment

denik Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

denik commented Jan 27, 2025 •

edited

Loading

denik Feb 3, 2025 •

edited

Loading