Repository navigation
acc: record configuration edge cases ahead of the libs/dyn removal - #7085
Merged
Merged
Conversation
Twenty bundle validate tests found by reviewing libs/dyn against the structvar implementation (#6954): merge keys and aliases in target variables, duplicate keys, references that resolve to the wrong type, include overrides with zero values, null blocks, and diagnostic locations. Each pins the current behaviour; Badness marks the cases where the current behaviour is itself wrong. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 826969f
Top 6 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Records that duplicate keys are accepted without a warning, the later value replaces the earlier one as a whole (maps are not merged), with the processed JSON. Marked Badness: YAML requires keys to be unique. Co-authored-by: Isaac <no-reply@databricks.com>
Groups: yaml_anchors (anchors and merge keys), yaml_duplicate_keys, variables, targets, includes, nulls, references. The directory only made sense as the name of the purpose of this PR, not as a topic. Co-authored-by: Isaac <no-reply@databricks.com>
- Move existing anchor, merge-key, complex-variable and include-location tests into the new topic groups. - variables/complex_unknown_field_location is not Badness: the warning is reported at the key in the variable default, like any unknown field. - includes/merge_failure_short_circuits_diagnostics: the merge error also has no location. - app_resource_duplicate_name_location shows the merged entries as JSON. Co-authored-by: Isaac <no-reply@databricks.com>
bundle.deployment.lock resolved to enabled: false although an unset lock is enabled by default, which made the recorded value misleading. bundle.git has only string fields, so an unset field and an empty one mean the same. Co-authored-by: Isaac <no-reply@databricks.com>
shreyas-goenka
approved these changes
Oct 19, 2026
denik
enabled auto-merge
October 19, 2026 12:54
denik
added a commit
that referenced
this pull request
Oct 19, 2026
#7085 recorded main's behaviour. Where the typed configuration fixes a wrong behaviour the Badness is gone (nested and shorthand timestamps, an inline merge mapping, resources: null, a scalar targets section). Where it behaves differently in a wrong way a Badness records it: scalar target variable overrides inherited through a merge key, an aliased complex default rewritten in place, duplicate and invalid merge keys, a duplicate default key, wrong types turned into empty strings, the lost location of an unknown field in a variable default, and presets that an include cannot clear. Co-authored-by: Isaac <no-reply@databricks.com>
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.
Changes
Twenty-one new
bundle validateacceptance tests, each pinning current behaviour for a configuration edge case. They are grouped by topic underacceptance/bundle/validate/(yaml_anchors/for anchors and merge keys,yaml_duplicate_keys/,variables/,targets/,includes/,nulls/,references/); ten existing tests on the same topics moved into the groups.targets.*.variables(inherited through a target or root<<, aliases of a complex default, duplicate and malformed<<, inline mappings),""/0/false, diagnostics after a failed include merge,resources: null,targets: false,schedule: null,Badnessmarks the cases where current behaviour is itself wrong:resources:section is an error without a location (emptytargets,variables,syncare fine);targets: falsefails with an internal "failed to rewrite" message;scheduleis reported as missing its required fields;Why
They guard behaviour ahead of #7098, which replaces
libs/dyn: where it differs, the test shows it.Tests
Goldens recorded on main.
This pull request and its description were written by Isaac.