Kimirock/metabase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make mage driver-decisions honor the "force-run" (#80128)

Browse Source
This commit is contained in:
Nemanja Glumac
2026-08-19 14:44:04 +02:00
committed by GitHub
parent 22581bf881
commit ac3cd1d211
4 changed files with 62 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
.github/workflows/drivers.yml vendored
View File

@@ -7,7 +7,7 @@ on:
type: boolean
default: false
force-run:
description: Run the full suite regardless of the diff (master / release branches).
description: Run the full suite regardless of the diff (master / release branches, ci:run-all label).
type: boolean
default: false
@@ -66,7 +66,7 @@ jobs:
# branch context, labels, and file changes to decide what runs.
MAGE_ARGS=(
--git-ref="${BASE_REF}"
--is-master-or-release="${IS_MASTER_OR_RELEASE}"
--force-run="${FORCE_RUN}"
--pr-labels="${{ join(github.event.pull_request.labels.*.name, ',') }}"
--skip="${{ inputs.skip }}"
)
@@ -82,7 +82,7 @@ jobs:
./bin/mage -driver-decisions "${MAGE_ARGS[@]}" --github-output-only >> "$GITHUB_OUTPUT"
env:
BASE_REF: ${{ github.event.pull_request.base.ref || github.ref_name }}
IS_MASTER_OR_RELEASE: ${{ inputs.force-run }}
FORCE_RUN: ${{ inputs.force-run }}
shell: bash
be-tests-athena-ee:

23
bb.edn
View File

@@ -391,7 +391,7 @@
-driver-decisions
{:doc "Determine which driver tests should run based on PR context. Outputs decisions for all drivers."
:examples [["./bin/mage -driver-decisions --git-ref=master --is-master-or-release=false --pr-labels= --skip=false" "Determine driver decisions for a PR"]
:examples [["./bin/mage -driver-decisions --git-ref=master --force-run=false --pr-labels= --skip=false" "Determine driver decisions for a PR"]
["./bin/mage -driver-decisions --pr-labels=ci:run-all-cloud-drivers" "Run all cloud drivers via label"]
["./bin/mage -driver-decisions --pr-labels=ci:run-mysql-mariadb" "Force-run a specific driver via label"]
["./bin/mage -driver-decisions --pr-labels=ci:run-all-drivers" "Force-run all drivers via label"]
@@ -406,12 +406,12 @@
"# How we decide to run a driver's tests. For each driver, we check these conditions in order (first match wins):\n"
(with-out-str
(table/table [["Priority" "Condition" "Result" "Reason"]
["1" "Global skip (no backend changes)" "SKIP" "workflow skip (no backend changes)"]
["2" "Driver is :h2 or :postgres" "RUN" "H2/Postgres always run"]
["3" "ci:run-all-drivers or ci:run-DRIVER" "RUN" "ci:run-all-drivers or ci:run-DRIVER label"]
["4" "Driver's own modules/drivers/* files changed" "RUN" "driver files changed (skip list ignored)"]
["5" "Configured status = skip (PR branches)" "SKIP" "driver is skipped by CI config"]
["6" "On master or release branch" "RUN" "master/release branch (skip list ignored)"]
["1" "Global force-run" "RUN" "master/release branch or ci:run-all label (skip list ignored)"]
["2" "Global skip (no backend changes)" "SKIP" "workflow skip (no backend changes)"]
["3" "Driver is :h2 or :postgres" "RUN" "H2/Postgres always run"]
["4" "ci:run-all-drivers or ci:run-DRIVER" "RUN" "ci:run-all-drivers or ci:run-DRIVER label"]
["5" "Driver's own modules/drivers/* files changed" "RUN" "driver files changed (skip list ignored)"]
["6" "Configured status = skip" "SKIP" "driver is skipped by CI config"]
["7" "Cloud driver + ci:run-all-cloud-drivers" "RUN" "ci:run-all-cloud-drivers label"]
["8" "Cloud driver + query-processor updated" "RUN" "query-processor module updated"]
["9" "Cloud driver + driver deps affected" "RUN" "driver module affected by shared code changes"]
@@ -423,9 +423,10 @@
["12" "Self-hosted driver, not affected" "SKIP" "driver module not affected"]]))
"\nThis table decides only WHETHER a driver runs. Whether its failures GATE is a separate\n"
"question, answered at the end of the job by ci-conductor's quarantine rules.\n"
"\nNote: the remote skip list (status = skip) is ignored entirely on master and\n"
"release-* branches -- every driver runs and gates there (Priority 5 is bypassed). It is\n"
"also ignored for any driver whose own modules/drivers/* files the run changes.\n"
"\nNote: the remote skip list (status = skip) is ignored entirely on a force-run --\n"
"master and release-* branches, or a ci:run-all label -- so every driver runs there.\n"
"Whether failures gate remains determined by CI Conductor's quarantine rules. The skip\n"
"list is also ignored for any driver whose own modules/drivers/* files the run changes.\n"
"\n\n## Labels of Interest\n\n"
"These PR labels influence driver test decisions:\n\n"
" ci:run-all-drivers Force-run ALL drivers (overrides the skip list)\n"
@@ -440,7 +441,7 @@
(str/join ", " (map name (sort mage.modules/all-drivers)))
"\n")))
:options [[nil "--git-ref REF" "The git ref to compute changed modules against"]
[nil "--is-master-or-release MASTER_OR_RELEASE"]
[nil "--force-run FORCE_RUN" "Run every driver regardless of the diff"]
[nil "--pr-labels PR_LABELS" "Comma delimited labels on the pr"]
[nil "--skip SKIP"]
[nil "--github-output-only" "Only output GITHUB_OUTPUT format (no human-readable output)"]]

51
mage/src/mage/modules.clj
View File

@@ -383,19 +383,6 @@
(.getMessage e) "); treating all drivers as required."))))
#{})))
(defn- effective-skipped-drivers
"Set of skipped drivers to actually enforce for this run.
The skip list is fetched on the fly from a remote file (ci-test-config.json, see
[[skip-drivers]]) and can change at any time. We intentionally IGNORE it on `master` and
`release-*` branches: there, every driver must run and gate, so a stray remote skip entry
can't silently disable a driver's tests. On PR/feature branches the skip list is honored,
subject to changes to the driver's own files and the ci:run-<driver> label."
[skipped is-master-or-release]
(if is-master-or-release
#{}
skipped))
(defn- parse-bool
"Parse a string boolean from CLI args. Returns true for 'true', false otherwise."
[s]
@@ -427,22 +414,28 @@
- deps.edn is changed (triggers all drivers)
- Clojure modules that the 'driver' module depends on are changed"
[driver
{:keys [is-master-or-release pr-labels skip particular-driver-changed?]}
{:keys [force-run pr-labels skip particular-driver-changed?]}
driver-deps-affected?
skipped-drivers
updated]
(cond
;; Priority 1: Global skip (no backend changes)
;; Priority 1: Global force-run. Every driver runs; the remote skip list is not consulted at
;; all, so a stray entry there can't silently disable a driver's tests.
force-run
{:should-run true
:reason "force-run (master/release branch or ci:run-all label)"}
;; Priority 2: Global skip (no backend changes)
skip
{:should-run false
:reason "workflow skip (no backend changes)"}
;; Priority 2: H2 and Postgres always run when backend tests run
;; Priority 3: H2 and Postgres always run when backend tests run
(#{:h2 :postgres} driver)
{:should-run true
:reason "H2/Postgres always run"}
;; Priority 3: ci:run-all-drivers or ci:run-<driver> label
;; Priority 4: ci:run-all-drivers or ci:run-<driver> label
(or (contains? pr-labels "ci:run-all-drivers")
(contains? pr-labels (run-driver-label driver)))
{:should-run true
@@ -450,24 +443,18 @@
"ci:run-all-drivers label"
(str (run-driver-label driver) " label"))}
;; Priority 4: The driver's own source changed - run it even when the CI config skips it,
;; Priority 5: The driver's own source changed - run it even when the CI config skips it,
;; since the change is exactly what needs testing.
(contains? particular-driver-changed? driver)
{:should-run true
:reason "driver files changed"}
;; Priority 5: Drivers the CI config marks `skip`; Priorities 3 and 4 are the ways to force
;; one to run. On master/release this set is empty (see [[effective-skipped-drivers]]), so the
;; skip list is ignored there and we fall through to Priority 6.
;; Priority 6: Drivers the CI config marks `skip`; Priorities 1, 4 and 5 are the ways to force
;; one to run.
(contains? skipped-drivers driver)
{:should-run false
:reason "driver is skipped by CI config"}
;; Priority 6: Master/release branch - every driver the CI config does not skip runs
is-master-or-release
{:should-run true
:reason "master/release branch"}
;; Priority 7: Cloud driver + ci:run-all-cloud-drivers label
(and (contains? cloud-drivers driver)
(contains? pr-labels "ci:run-all-cloud-drivers"))
@@ -510,23 +497,23 @@
Usage:
./bin/mage -driver-decisions \\
--git-ref=master \\
--is-master-or-release=false \\
--force-run=false \\
--pr-labels=ci:run-all-cloud-drivers,other-label \\
--skip=false"
[{:keys [options] :as _parsed}]
(let [github-output-only? (some? (:github-output-only options))
git-ref (get options :git-ref "master")
is-master-or-release (parse-bool (:is-master-or-release options))
force-run (parse-bool (:force-run options))
;; Detect file changes for ALL drivers via git diff
particular-driver-changed? (drivers-with-file-changes git-ref)
ctx {:git-ref git-ref
:is-master-or-release is-master-or-release
:force-run force-run
:pr-labels (parse-labels (:pr-labels options))
:skip (parse-bool (:skip options))
:particular-driver-changed? particular-driver-changed?}
;; On master/release we drop the remote skip list entirely (see
;; [[effective-skipped-drivers]]) so every driver runs and gates.
skipped (effective-skipped-drivers (skip-drivers) is-master-or-release)
;; force-run decides every driver on its own, so the remote skip list is neither fetched
;; nor honored there.
skipped (if force-run #{} (skip-drivers))
updated-files (u/updated-files git-ref)
updated (updated-files->updated-modules updated-files)
driver-affected? (driver-deps-affected? updated)

68
mage/test/mage/modules_test.clj
View File

@@ -11,14 +11,14 @@
(defn- make-ctx
"Create a context map with sensible defaults, overridable by opts."
[opts]
(merge {:is-master-or-release false
(merge {:force-run false
:pr-labels #{}
:skip false
:particular-driver-changed? #{}}
opts))
;;; =============================================================================
;;; Priority 4: Driver's own files changed (beats the CI config skip list)
;;; Priority 5: Driver's own files changed (beats the CI config skip list)
;;; =============================================================================
(deftest particular-driver-changes-beat-configured-skip
@@ -34,7 +34,7 @@
(is (= "driver files changed" (:reason result)))))))
;;; =============================================================================
;;; Priority 5: CI config skip list (respected on PR branches, ignored on master/release)
;;; Priority 6: CI config skip list (ignored on a force-run)
;;; =============================================================================
(deftest skipped-driver-does-not-run
@@ -50,34 +50,23 @@
(is (false? (:should-run result)))
(is (= "driver is skipped by CI config" (:reason result))))))))
(deftest effective-skip-list-ignored-on-master-and-release
(testing "the remote skip list is dropped on master/release, but honored on PR branches"
(let [skipped #{:databricks}]
(is (= #{} (#'mage.modules/effective-skipped-drivers skipped true))
"master/release: nothing is skipped")
(is (= #{:databricks} (#'mage.modules/effective-skipped-drivers skipped false))
"PR/feature branch: skipped drivers stay skipped"))))
(deftest skipped-driver-runs-on-master
(testing "a driver marked skip in the remote config still runs (and gates) on master/release"
;; Production clears the skip set on master/release via effective-skipped-drivers,
;; so the decision falls through Priority 5 to Priority 6.
(let [skipped (#'mage.modules/effective-skipped-drivers #{:mysql} true)
result (mage.modules/driver-decision :mysql
(make-ctx {:is-master-or-release true})
(deftest skipped-driver-runs-on-force-run
(testing "a driver marked skip in the remote config still runs (and gates) on a force-run"
(let [result (mage.modules/driver-decision :mysql
(make-ctx {:force-run true})
true ; driver-deps-affected?
skipped
#{:mysql} ; skipped
#{})] ; updated
(is (true? (:should-run result)))
(is (= "master/release branch" (:reason result))))))
(is (= "force-run (master/release branch or ci:run-all label)" (:reason result))))))
(deftest skip-config-names-are-translated
(testing "Config names from ci-test-config.json are translated to internal driver keywords via driver-directory->drivers"
(testing "directory names are translated to internal keywords"
;; bigquery-cloud-sdk -> :bigquery (via driver-directory->drivers mapping).
;; Use a PR/feature branch here, since the skip list is ignored on master/release.
;; Not a force-run here, since the skip list is ignored on one.
(let [result (mage.modules/driver-decision :bigquery
(make-ctx {:is-master-or-release false})
(make-ctx {:force-run false})
false
#{:bigquery} ; as if translated from "bigquery-cloud-sdk"
#{})]
@@ -91,7 +80,7 @@
"mongo should map to multiple test jobs"))))
;;; =============================================================================
;;; Priority 1: Global skip
;;; Priority 2: Global skip
;;; =============================================================================
(deftest global-skip-skips-all-drivers
@@ -117,14 +106,14 @@
(is (= "workflow skip (no backend changes)" (:reason result))))))
;;; =============================================================================
;;; Priority 2: H2 and Postgres always run
;;; Priority 3: H2 and Postgres always run
;;; =============================================================================
(deftest h2-and-postgres-always-run
(testing "H2 and Postgres always run when not globally skipped"
(doseq [driver [:h2 :postgres]]
(let [result (mage.modules/driver-decision driver
(make-ctx {:is-master-or-release false})
(make-ctx {:force-run false})
false ; driver module not affected
#{} ; skipped
#{})] ; updated
@@ -145,7 +134,7 @@
(is (= "workflow skip (no backend changes)" (:reason result)))))))
;;; =============================================================================
;;; Priority 3: ci:run-all-drivers / ci:run-<driver> labels
;;; Priority 4: ci:run-all-drivers / ci:run-<driver> labels
;;; =============================================================================
(deftest ci-run-all-drivers-forces-run
@@ -200,21 +189,20 @@
(is (= "ci:run-mysql label" (:reason result))))))
;;; =============================================================================
;;; Priority 6: Master/release branch
;;; Priority 1: Global force-run
;;; =============================================================================
(deftest master-branch-runs-all-drivers
(testing "All drivers run on master/release branch"
;; H2/Postgres hit priority 2 first, others hit priority 6
(doseq [driver [:mysql :mongo :athena :bigquery :snowflake]]
(deftest force-run-runs-all-drivers
(testing "All drivers run on a force-run, even the globally skipped ones"
(doseq [driver [:h2 :postgres :mysql :mongo :athena :bigquery :snowflake]]
(let [result (mage.modules/driver-decision driver
(make-ctx {:is-master-or-release true})
(make-ctx {:force-run true :skip true})
false ; even if not affected
#{} ; skipped
#{driver} ; skipped
#{})] ; updated
(is (true? (:should-run result))
(str driver " should run on master"))
(is (= "master/release branch" (:reason result)))))))
(str driver " should run on a force-run"))
(is (= "force-run (master/release branch or ci:run-all label)" (:reason result)))))))
;;; =============================================================================
;;; Priority 11: Driver deps affected (self-hosted only)
@@ -222,7 +210,7 @@
(deftest driver-deps-affected-runs-self-hosted-drivers
(testing "Self-hosted drivers run when driver module is affected"
;; H2/Postgres hit priority 2 first, others hit priority 11
;; H2/Postgres hit priority 3 first, others hit priority 11
(doseq [driver [:mysql :mongo :oracle :sqlserver]]
(let [result (mage.modules/driver-decision driver
(make-ctx {})
@@ -294,7 +282,7 @@
(deftest self-hosted-driver-not-affected-skips
(testing "Self-hosted driver skips when driver module not affected"
;; H2/Postgres always run (priority 2), so test other self-hosted drivers
;; H2/Postgres always run (priority 3), so test other self-hosted drivers
(doseq [driver [:mysql :mongo :oracle :sqlserver]]
(let [result (mage.modules/driver-decision driver
(make-ctx {})
@@ -417,12 +405,12 @@
(is (= #{} (#'mage.modules/skip-drivers))))))
(deftest non-skip-driver-follows-normal-run-rules
(testing "a driver that isn't skipped follows the normal decision rules (runs on master)"
(testing "a driver that isn't skipped follows the normal decision rules (runs on a force-run)"
(with-redefs [mage.modules/read-ci-test-config (constantly example-config)]
(let [result (mage.modules/driver-decision :snowflake
(make-ctx {:is-master-or-release true})
(make-ctx {:force-run true})
false
(#'mage.modules/skip-drivers)
#{})]
(is (true? (:should-run result)))
(is (= "master/release branch" (:reason result)))))))
(is (= "force-run (master/release branch or ci:run-all label)" (:reason result)))))))