Skip to content

Commit 9202af4

Browse files
authored
Add testmask tool for conditional CI test execution (#4017)
## Changes A new tool `testmask` under `tools/` that determines the set of Makefile targets to run for testing. The test jobs all depend on a job that runs this tool and use its output to determine if they should run or skip. If a change only touches files under `experimental/ssh`, for example, it won't trigger the main test target. We can extend this to add more selective targets, only if we are sure that there is no dependency between the files matched for the target and other files in the repository. This, for now, relies on convention rather than strict enforcement. ### How Every file in the diff between the PR and the PR base is attributed to one of the targets. If all files map to the same target, only that target is run. If files map to multiple targets, multiple targets are run. If there are files that cannot be mapped to a specific target, then the main `test` target is run. ### Mapping to GitHub Actions This approach uses a GitHub Actions test job for every target. This results in a status check mark for each of them, making it easy to eyeball which tests ran on a PR, which tests can on main, and when they broke. Executing the same locally is straightforward because they are all named after Makefile targets. It applies to pull requests and merge queue checks. Scheduled runs and main branch pushes always run the full test suite. ## Why Not all tests need to run for every change. Being more selective reduces build times and removes noise in PRs. ## Tests The tests for this PR show it works. A new test in `tools/testmask` confirms that the targets it maps to are present in the Makefile. PRs for each of the targets: * #4023 * #4024 * #4025 * #4026
1 parent 066b564 commit 9202af4

File tree

8 files changed

+538
-39
lines changed

8 files changed

+538
-39
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
name: 'Setup Build Environment'
2+
description: 'Sets up the build environment with Go, Python, uv, and ruff'
3+
4+
inputs:
5+
cache-key:
6+
description: 'Cache key identifier for Go cache'
7+
required: true
8+
9+
runs:
10+
using: 'composite'
11+
steps:
12+
- name: Checkout repository and submodules
13+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
14+
15+
- name: Create cache identifier
16+
run: echo "${{ inputs.cache-key }}" > cache.txt
17+
shell: bash
18+
19+
- name: Setup Go
20+
uses: actions/setup-go@44694675825211faa026b3c33043df3e48a5fa00 # v6.0.0
21+
with:
22+
go-version-file: go.mod
23+
cache-dependency-path: |
24+
go.sum
25+
cache.txt
26+
27+
- name: Setup Python
28+
uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0
29+
with:
30+
python-version: '3.13'
31+
32+
- name: Install uv
33+
uses: astral-sh/setup-uv@85856786d1ce8acfbcc2f13a5f3fbd6b938f9f41 # v7.1.2
34+
with:
35+
version: "0.8.9"
36+
37+
- name: Install ruff (Python linter and formatter)
38+
uses: astral-sh/ruff-action@57714a7c8a2e59f32539362ba31877a1957dded1 # v3.5.1
39+
with:
40+
version: "0.9.1"
41+
args: "--version"
42+
43+
- name: Pull external libraries
44+
run: |
45+
go mod download
46+
pip3 install wheel==0.45.1
47+
shell: bash

.github/workflows/push.yml

Lines changed: 173 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,52 @@ jobs:
3131
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
3232
run: gh cache delete --all --repo databricks/cli || true
3333

34+
testmask:
35+
runs-on: ubuntu-latest
36+
outputs:
37+
targets: ${{ steps.mask1.outputs.targets || steps.mask2.outputs.targets || steps.mask3.outputs.targets }}
38+
steps:
39+
- name: Checkout repository
40+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
41+
with:
42+
fetch-depth: 0
43+
44+
- name: Setup Go
45+
uses: actions/setup-go@44694675825211faa026b3c33043df3e48a5fa00 # v6.0.0
46+
with:
47+
go-version-file: tools/go.mod
48+
49+
- name: Run testmask (pull requests)
50+
if: ${{ github.event_name == 'pull_request' }}
51+
id: mask1
52+
working-directory: tools/testmask
53+
run: |
54+
go run . ${{ github.event.pull_request.head.sha }} ${{ github.event.pull_request.base.sha }} | tee output.json
55+
echo "targets=$(jq -c '.' output.json)" >> $GITHUB_OUTPUT
56+
57+
- name: Run testmask (merge group)
58+
if: ${{ github.event_name == 'merge_group' }}
59+
id: mask2
60+
working-directory: tools/testmask
61+
run: |
62+
go run . ${{ github.event.merge_group.head_sha }} ${{ github.event.merge_group.base_sha }} | tee output.json
63+
echo "targets=$(jq -c '.' output.json)" >> $GITHUB_OUTPUT
64+
65+
- name: Run testmask (other events)
66+
if: ${{ github.event_name != 'pull_request' && github.event_name != 'merge_group' }}
67+
id: mask3
68+
working-directory: tools/testmask
69+
run: |
70+
# Always run all tests
71+
echo "targets=[\"test\"]" >> $GITHUB_OUTPUT
72+
3473
tests:
35-
needs: cleanups
74+
needs:
75+
- cleanups
76+
- testmask
77+
78+
# Only run if the target is in the list of targets from testmask
79+
if: ${{ contains(fromJSON(needs.testmask.outputs.targets), 'test') }}
3680
runs-on: ${{ matrix.os }}
3781

3882
strategy:
@@ -56,37 +100,10 @@ jobs:
56100
- name: Checkout repository and submodules
57101
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
58102

59-
- name: Create deployment-specific cache identifier
60-
run: echo "${{ matrix.deployment }}" > deployment-type.txt
61-
62-
- name: Setup Go
63-
uses: actions/setup-go@44694675825211faa026b3c33043df3e48a5fa00 # v6.0.0
64-
with:
65-
go-version-file: go.mod
66-
cache-dependency-path: |
67-
go.sum
68-
deployment-type.txt
69-
70-
- name: Setup Python
71-
uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0
103+
- name: Setup build environment
104+
uses: ./.github/actions/setup-build-environment
72105
with:
73-
python-version: '3.13'
74-
75-
- name: Install uv
76-
uses: astral-sh/setup-uv@85856786d1ce8acfbcc2f13a5f3fbd6b938f9f41 # v7.1.2
77-
with:
78-
version: "0.8.9"
79-
80-
- name: Install ruff (Python linter and formatter)
81-
uses: astral-sh/ruff-action@57714a7c8a2e59f32539362ba31877a1957dded1 # v3.5.1
82-
with:
83-
version: "0.9.1"
84-
args: "--version"
85-
86-
- name: Pull external libraries
87-
run: |
88-
go mod download
89-
pip3 install wheel==0.45.1
106+
cache-key: test-${{ matrix.deployment }}
90107

91108
- name: Run tests without coverage
92109
# We run tests without coverage on PR, merge_group, and schedule because we don't make use of coverage information
@@ -107,6 +124,131 @@ jobs:
107124
- name: Analyze slow tests
108125
run: make slowest
109126

127+
test-exp-aitools:
128+
needs:
129+
- cleanups
130+
- testmask
131+
132+
# Only run if the target is in the list of targets from testmask
133+
if: ${{ contains(fromJSON(needs.testmask.outputs.targets), 'test-exp-aitools') }}
134+
name: "make test-exp-aitools"
135+
runs-on: ${{ matrix.os }}
136+
137+
strategy:
138+
fail-fast: false
139+
matrix:
140+
os:
141+
- macos-latest
142+
- ubuntu-latest
143+
- windows-latest
144+
145+
steps:
146+
- name: Checkout repository and submodules
147+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
148+
149+
- name: Setup build environment
150+
uses: ./.github/actions/setup-build-environment
151+
with:
152+
cache-key: test-exp-aitools
153+
154+
- name: Run tests
155+
run: |
156+
make test-exp-aitools
157+
158+
test-exp-apps-mcp:
159+
needs:
160+
- cleanups
161+
- testmask
162+
163+
# Only run if the target is in the list of targets from testmask
164+
if: ${{ contains(fromJSON(needs.testmask.outputs.targets), 'test-exp-apps-mcp') }}
165+
name: "make test-exp-apps-mcp"
166+
runs-on: ${{ matrix.os }}
167+
168+
strategy:
169+
fail-fast: false
170+
matrix:
171+
os:
172+
- macos-latest
173+
- ubuntu-latest
174+
# The Windows tests are broken; see https://github.com/databricks/cli/pull/4024.
175+
# - windows-latest
176+
177+
steps:
178+
- name: Checkout repository and submodules
179+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
180+
181+
- name: Setup build environment
182+
uses: ./.github/actions/setup-build-environment
183+
with:
184+
cache-key: test-exp-apps-mcp
185+
186+
- name: Run tests
187+
run: |
188+
make test-exp-apps-mcp
189+
190+
test-exp-ssh:
191+
needs:
192+
- cleanups
193+
- testmask
194+
195+
# Only run if the target is in the list of targets from testmask
196+
if: ${{ contains(fromJSON(needs.testmask.outputs.targets), 'test-exp-ssh') }}
197+
name: "make test-exp-ssh"
198+
runs-on: ${{ matrix.os }}
199+
200+
strategy:
201+
fail-fast: false
202+
matrix:
203+
os:
204+
- macos-latest
205+
- ubuntu-latest
206+
- windows-latest
207+
208+
steps:
209+
- name: Checkout repository and submodules
210+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
211+
212+
- name: Setup build environment
213+
uses: ./.github/actions/setup-build-environment
214+
with:
215+
cache-key: test-exp-ssh
216+
217+
- name: Run tests
218+
run: |
219+
make test-exp-ssh
220+
221+
test-pipelines:
222+
needs:
223+
- cleanups
224+
- testmask
225+
226+
# Only run if the target is in the list of targets from testmask
227+
if: ${{ contains(fromJSON(needs.testmask.outputs.targets), 'test-pipelines') }}
228+
name: "make test-pipelines"
229+
runs-on: ${{ matrix.os }}
230+
231+
strategy:
232+
fail-fast: false
233+
matrix:
234+
os:
235+
- macos-latest
236+
- ubuntu-latest
237+
- windows-latest
238+
239+
steps:
240+
- name: Checkout repository and submodules
241+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
242+
243+
- name: Setup build environment
244+
uses: ./.github/actions/setup-build-environment
245+
with:
246+
cache-key: test-pipelines
247+
248+
- name: Run tests
249+
run: |
250+
make test-pipelines
251+
110252
validate-generated-is-up-to-date:
111253
needs: cleanups
112254
runs-on: ubuntu-latest

Makefile

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
default: checks fmt lint
22

3-
# gotestsum: when go test args are used with --rerun-fails the list of packages to test must be specified by the --packages flag
4-
PACKAGES=--packages "./acceptance/... ./libs/... ./internal/... ./cmd/... ./bundle/... ./experimental/aitools/... ./experimental/ssh/... ."
3+
# Default packages to test (all)
4+
TEST_PACKAGES = ./acceptance/internal ./libs/... ./internal/... ./cmd/... ./bundle/... ./experimental/aitools/... ./experimental/ssh/... .
5+
6+
# Default acceptance test filter (all)
7+
ACCEPTANCE_TEST_FILTER = ""
58

69
GO_TOOL ?= go tool -modfile=tools/go.mod
710
GOTESTSUM_FORMAT ?= pkgname-and-test-fails
@@ -55,11 +58,24 @@ links:
5558
# Checks other than 'fmt' and 'lint'; these are fast, so can be run first
5659
checks: tidy ws links
5760

58-
test:
59-
${GOTESTSUM_CMD} ${PACKAGES} -- -timeout=${LOCAL_TIMEOUT} -short
6061

61-
test-slow:
62-
${GOTESTSUM_CMD} ${PACKAGES} -- -timeout=${LOCAL_TIMEOUT}
62+
# Run short unit and acceptance tests (testing.Short() is true).
63+
test: test-unit test-acc
64+
65+
# Run all unit and acceptance tests.
66+
test-slow: test-slow-unit test-slow-acc
67+
68+
test-unit:
69+
${GOTESTSUM_CMD} --packages "${TEST_PACKAGES}" -- -timeout=${LOCAL_TIMEOUT} -short
70+
71+
test-slow-unit:
72+
${GOTESTSUM_CMD} --packages "${TEST_PACKAGES}" -- -timeout=${LOCAL_TIMEOUT}
73+
74+
test-acc:
75+
${GOTESTSUM_CMD} --packages ./acceptance/... -- -timeout=${LOCAL_TIMEOUT} -short -run ${ACCEPTANCE_TEST_FILTER}
76+
77+
test-slow-acc:
78+
${GOTESTSUM_CMD} --packages ./acceptance/... -- -timeout=${LOCAL_TIMEOUT} -run ${ACCEPTANCE_TEST_FILTER}
6379

6480
# Updates acceptance test output (local tests)
6581
test-update:
@@ -82,7 +98,7 @@ slowest:
8298

8399
cover:
84100
rm -fr ./acceptance/build/cover/
85-
VERBOSE_TEST=1 CLI_GOCOVERDIR=build/cover ${GOTESTSUM_CMD} ${PACKAGES} -- -coverprofile=coverage.txt -timeout=${LOCAL_TIMEOUT}
101+
VERBOSE_TEST=1 CLI_GOCOVERDIR=build/cover ${GOTESTSUM_CMD} --packages ${TEST_PACKAGES} -- -coverprofile=coverage.txt -timeout=${LOCAL_TIMEOUT}
86102
rm -fr ./acceptance/build/cover-merged/
87103
mkdir -p acceptance/build/cover-merged/
88104
go tool covdata merge -i $$(printf '%s,' acceptance/build/cover/* | sed 's/,$$//') -o acceptance/build/cover-merged/
@@ -150,4 +166,16 @@ generate:
150166
$(GENKIT_BINARY) update-sdk
151167

152168

153-
.PHONY: lint lintfull tidy lintcheck fmt fmtfull test cover showcover build snapshot snapshot-release schema integration integration-short acc-cover acc-showcover docs ws wsfix links checks test-update test-update-templates test-update-aws test-update-all generate-validation
169+
.PHONY: lint lintfull tidy lintcheck fmt fmtfull test test-unit test-acc test-slow test-slow-unit test-slow-acc cover showcover build snapshot snapshot-release schema integration integration-short acc-cover acc-showcover docs ws wsfix links checks test-update test-update-templates test-update-aws test-update-all generate-validation
170+
171+
test-exp-aitools:
172+
make test TEST_PACKAGES="./experimental/aitools/..." ACCEPTANCE_TEST_FILTER="TestAccept/idontexistyet/aitools"
173+
174+
test-exp-apps-mcp:
175+
make test TEST_PACKAGES="./experimental/apps-mcp/..." ACCEPTANCE_TEST_FILTER="TestAccept/idontexistyet/apps-mcp"
176+
177+
test-exp-ssh:
178+
make test TEST_PACKAGES="./experimental/ssh/..." ACCEPTANCE_TEST_FILTER="TestAccept/ssh"
179+
180+
test-pipelines:
181+
make test TEST_PACKAGES="./cmd/pipelines/..." ACCEPTANCE_TEST_FILTER="TestAccept/pipelines"

tools/testmask/git.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"os/exec"
6+
"strings"
7+
)
8+
9+
// GetChangedFiles returns the list of files changed between two git refs.
10+
func GetChangedFiles(headRef, baseRef string) ([]string, error) {
11+
cmd := exec.Command("git", "diff", "--name-only", baseRef, headRef)
12+
output, err := cmd.Output()
13+
if err != nil {
14+
return nil, fmt.Errorf("failed to get diff between %s and %s: %w", baseRef, headRef, err)
15+
}
16+
17+
lines := strings.Split(string(output), "\n")
18+
19+
// Drop the last line (always empty)
20+
if len(lines) > 0 {
21+
lines = lines[:len(lines)-1]
22+
}
23+
24+
return lines, nil
25+
}

0 commit comments

Comments
 (0)