acc: CloudSlow only narrows Cloud = true tests - #6354
Merged
janniklasrose merged 3 commits intoAug 24, 2026
Conversation
Not needed because parent's `CloudSlow = true` flipped `Cloud` to `true`. Follow-up will stop that behaviour (CloudSlow should only affect `Cloud = true`). However, this test _should_ actually be `Cloud = true` so this config can be removed.
Previously CloudSlow implied Cloud: any test with CloudSlow = true was flipped to Cloud = true during config materialisation and in the runner's skip logic, even when Cloud was explicitly false. This made CloudSlow a second, surprising way to opt a test into the cloud run and let an inherited CloudSlow = true silently override an explicit Cloud = false. Make CloudSlow only narrow an already-enabled cloud run: it is honoured only when Cloud = true, where it skips the cloud run under -short. The runner now gates on Cloud alone and applies the -short skip afterwards. Tests that reached the cloud run solely via the implication now set Cloud = true explicitly to preserve their behaviour. The vector_search_indexes subtests that set CloudSlow = false purely to defeat the old implication drop that workaround, since Cloud = false alone now means local-only. Co-authored-by: Isaac <no-reply@databricks.com>
denik
approved these changes
Aug 24, 2026
| @@ -1,4 +1,4 @@ | |||
| Cloud = false | |||
| CloudSlow = false | |||
| CloudSlow = true | |||
Contributor
There was a problem hiding this comment.
we could also omit CloudSlow from out.test.toml if Cloud = false
Contributor
Author
There was a problem hiding this comment.
It's indeed not needed in that case, but I kind of like seeing the entire (including the inherited) config materialize in out.test.toml
Collaborator
Integration test reportCommit: 42e107d
Top 5 slowest tests (at least 2 minutes):
|
janniklasrose
enabled auto-merge
August 24, 2026 10:55
janniklasrose
deleted the
janniklasrose/cloudslow-only-affects-cloud-true
branch
August 24, 2026 11:36
dkling-it
pushed a commit
to dkling-it/cli
that referenced
this pull request
Aug 24, 2026
Not needed because parent's `CloudSlow = true` flipped `Cloud` to `true`. Follow-up (databricks#6354) will stop that behaviour (CloudSlow should only affect `Cloud = true`). However, this test _should_ actually be `Cloud = true` so this config can be removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously
CloudSlowimpliedCloud = true: any test withCloudSlow = truewas flipped toCloud = trueduring config materialisation and in the runner's skip logic, even whenCloudwas explicitly false.This change makes
CloudSlowonly narrow an already-enabled cloud run: it is honoured only whenCloud = true, where it skips the cloud run under-short. The runner now gates onCloudalone and applies the-shortskip afterwards.Tests that reached the cloud run solely via the implication now set
Cloud = trueexplicitly to preserve their behaviour. The vector_search_indexes subtests that setCloudSlow = falsepurely to defeat the old implication drop that workaround, sinceCloud = falsealone now means local-only.