Add test about using variable in bundle.git.branch #2118

denik · 2025-01-10T11:05:50Z

This test checks load git details functionality + variable interpolation there.

The variables are not working there because LoadGitDetails mutator is running before variable interpolation.

This also includes a couple fixes for acceptance tests on Windows:

Correctly replace tmp path that is used for DATABRICKS_TF_EXEC_PATH
Pass -buildvcs=false to go build. It's not needed on CI but it is needed on my local windows VM.

denik · 2025-01-13T15:42:52Z

acceptance/script.prepare

+
+git-repo-init() {
+    git init -qb main
+    git config --global core.autocrlf false


@pietern Found out about this option because a warning was printed by git about this and was captured in test output. This must be the reason we need to fix crlf in golden files in integration tests.

It can be set in .gitattributes: https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings#per-repository-settings

pietern · 2025-01-14T14:21:48Z

acceptance/acceptance_test.go


 	repls := testdiff.ReplacementsContext{}
 	repls.Set(execPath, "$CLI")
+	repls.Set(toJson(t, tempHomeDir), "$TMPHOME")


Why both toJson and verbatim values?

Verbatim does not work in JSON output and vice versa.

If verbatim strings contains \ then you need both replacements if you want replacement to work in json and non-json context.

We don't need non-json here, I added it just in case. However, I think it's best to do that everywhere so you don't think about it -- #2142

pietern · 2025-01-14T14:22:39Z

acceptance/bundle/variables/git-branch/output.txt

+    "file_path": "/Workspace/Users/[email protected]/.bundle/git/prod/files",
+    "resource_path": "/Workspace/Users/[email protected]/.bundle/git/prod/resources",
+    "root_path": "/Workspace/Users/[email protected]/.bundle/git/prod",
+    "state_path": "/Workspace/Users/[email protected]/.bundle/git/prod/state"


Needs rebase on #2120 for $USERNAME.

pietern · 2025-01-14T14:23:43Z

acceptance/script.prepare

+
+git-repo-init() {
+    git init -qb main
+    git config --global core.autocrlf false


It can be set in .gitattributes: https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings#per-repository-settings

pietern

No blocking comments, approving to unblock merge.

## Changes Always include both verbatim and json version of replacement. This helps when the string in question contains \\ or other chars that need to be quoted. Needed for #2118 ## Tests Existing tests.

pietern · 2025-01-14T17:52:34Z

acceptance/bundle/variables/git-branch/databricks.yml

+
+variables:
+  deployment_branch:
+    # read from git; inferred should be set to true; current it's not happening


Does this belong to the stanza above? I don't understand what you expect to happen.

Updated the description -- fe0ec6b

pietern · 2025-01-14T17:52:51Z

acceptance/bundle/variables/git-branch/script

+trace $CLI bundle validate -o json | grep -v '"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"'


What do the greps do?

Remove line matching '"commit"' because it includes hash that varies from run to run.

denik · 2025-01-14T21:01:06Z

@pietern @andrewnester please stamp this PR - #2147 -- it's a dependency of this one.

## Changes Follow up to #2142 which did not actually enabled JSON conversion because of reversed condition on err. ## Tests Tested via #2118

- setting DATABRICKS_TF_EXEC_PATH to /tmp/ on Windows - disable crlf conversion in git

This reverts commit b434976.

denik requested review from pietern, andrewnester and shreyas-goenka as code owners January 10, 2025 11:05

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

denik marked this pull request as draft January 10, 2025 13:12

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

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

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

denik marked this pull request as ready for review January 13, 2025 10:51

denik commented Jan 13, 2025

View reviewed changes

pietern reviewed Jan 14, 2025

View reviewed changes

pietern approved these changes Jan 14, 2025

View reviewed changes

denik force-pushed the denik/variables-git-branch branch from 97f9bfb to 2e6a6f0 Compare January 14, 2025 14:45

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

denik changed the base branch from main to denik/replacements-json January 14, 2025 15:01

andrewnester approved these changes Jan 14, 2025

View reviewed changes

denik mentioned this pull request Jan 14, 2025

Golden files: always include JSON-ed string #2142

Merged

denik force-pushed the denik/variables-git-branch branch from 2e6a6f0 to 2c919ac Compare January 14, 2025 15:40

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

Base automatically changed from denik/replacements-json to main January 14, 2025 15:58

pietern reviewed Jan 14, 2025

View reviewed changes

denik mentioned this pull request Jan 14, 2025

Golden files: always include JSON-ed string (cont) #2147

Merged

denik force-pushed the denik/variables-git-branch branch from 5593021 to fe0ec6b Compare January 14, 2025 20:56

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

denik changed the base branch from main to denik/enable-json-replacer January 14, 2025 20:59

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

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

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

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

Golden files: always include JSON-ed string (cont) (#2147)

cc44e36

## Changes Follow up to #2142 which did not actually enabled JSON conversion because of reversed condition on err. ## Tests Tested via #2118

Base automatically changed from denik/enable-json-replacer to main January 15, 2025 08:46

denik added 10 commits January 15, 2025 09:48

Add test about using variable in bundle.git.branch

5a8b84f

fixes

8b6b288

add a comment re what's not working

856d7ec

fixes for Windows:

42e15f6

- setting DATABRICKS_TF_EXEC_PATH to /tmp/ on Windows - disable crlf conversion in git

rebase

d44a36f

clean up json

c438cea

log tempHomeDir

2bfb31d

give better explanation of the test

6ffa6b0

clean up

e41bc49

Revert "acceptance: add -buildvcs=false when building on Windows"

20d424e

This reverts commit b434976.

denik force-pushed the denik/variables-git-branch branch from 1901998 to 20d424e Compare January 15, 2025 08:49

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

denik enabled auto-merge January 15, 2025 08:49

denik disabled auto-merge January 15, 2025 09:32

denik enabled auto-merge January 15, 2025 09:32

denik disabled auto-merge January 15, 2025 09:34

denik enabled auto-merge January 15, 2025 09:34

denik disabled auto-merge January 15, 2025 09:34

denik merged commit 55494a0 into main Jan 15, 2025
8 of 9 checks passed

denik deleted the denik/variables-git-branch branch January 15, 2025 09:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test about using variable in bundle.git.branch #2118

Add test about using variable in bundle.git.branch #2118

denik commented Jan 10, 2025 •

edited

Loading

denik Jan 13, 2025

pietern Jan 14, 2025

pietern Jan 14, 2025

denik Jan 14, 2025

pietern Jan 14, 2025

pietern Jan 14, 2025

pietern left a comment

pietern Jan 14, 2025

denik Jan 14, 2025

pietern Jan 14, 2025

denik Jan 14, 2025

denik commented Jan 14, 2025

Add test about using variable in bundle.git.branch #2118

Add test about using variable in bundle.git.branch #2118

Conversation

denik commented Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietern left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denik commented Jan 14, 2025

denik commented Jan 10, 2025 •

edited

Loading