Skip to content

structwalk: drop the upper-bound field-count assertion that churns on SDK bumps#5778

Merged
pietern merged 2 commits into
mainfrom
structwalk-drop-upper-bound
Jun 30, 2026
Merged

structwalk: drop the upper-bound field-count assertion that churns on SDK bumps#5778
pietern merged 2 commits into
mainfrom
structwalk-drop-upper-bound

Conversation

@pietern

@pietern pietern commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No description provided.

@pietern
structwalk: drop the upper-bound field-count assertion that churns on…
e454897 
… SDK bumps

The upper bound on the number of fields WalkType discovers has no real
integrity value: it only loosely approximated "the walk didn't explode",
and every few SDK bumps adds enough fields to push the count past the
ceiling, forcing a churn-only edit to a magic number.

Drop the maxLen parameter and its assert.Less from testStruct. The
properties that matter are kept: the lower bound still catches a walk
that silently truncates, the present checks confirm specific fields are
reachable, and the notPresent checks still verify circular references
terminate at one level - which is the precise guard the upper bound was
only gesturing at.

Co-authored-by: Isaac
@pietern pietern temporarily deployed to test-trigger-is June 30, 2026 12:33 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is June 30, 2026 12:33 — with GitHub Actions Inactive
@pietern pietern requested review from denik and janniklasrose June 30, 2026 12:34
janniklasrose
janniklasrose approved these changes Jun 30, 2026
View reviewed changes
Comment thread libs/structs/structwalk/walktype_test.go Outdated
denik
denik approved these changes Jun 30, 2026
View reviewed changes

@denik denik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be simpler to just set it to 100_000

@pietern @janniklasrose
Apply suggestions from code review
1be1cb6 
Co-authored-by: Jan N Rose <janniklas.rose@gmail.com>
@pietern pietern enabled auto-merge June 30, 2026 12:49
@denik

denik commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

TestTypeJobSettings and TestTypeRoot asserted both a lower and an upper bound on the number of fields WalkType discovers. The upper bound (650 for JobSettings, 6000 for config.Root) has no real integrity value: it only loosely approximated "the walk didn't explode", and every few SDK bumps adds enough fields to push the count past the ceiling, forcing a churn-only edit to a magic number.

This drops the maxLen parameter and its assert.Less from testStruct. The properties that matter are kept: the lower bound still catches a walk that silently truncates, the present checks confirm specific fields are reachable, and the notPresent checks still verify circular references terminate at one level (e.g. tasks[*].for_each_task.task.for_each_task.task.task_key must be absent) — which is the precise guard the upper bound was only gesturing at.

can you drop all this? title already explains it.

@pietern pietern disabled auto-merge June 30, 2026 12:49
@pietern pietern temporarily deployed to test-trigger-is June 30, 2026 12:49 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is June 30, 2026 12:49 — with GitHub Actions Inactive
@pietern pietern enabled auto-merge June 30, 2026 12:50
@pietern pietern added this pull request to the merge queue Jun 30, 2026
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 1be1cb6

Run: 28445572925

Env 🟨​KNOWN 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
🟨​ aws linux 7 1 13 231 1037 4:43
🟨​ aws windows 7 1 13 233 1035 5:36
💚​ aws-ucws linux 8 13 315 955 4:20
💚​ aws-ucws windows 8 13 317 953 3:20
💚​ azure linux 2 15 231 1036 3:39
💚​ azure windows 2 15 233 1034 2:54
💚​ azure-ucws linux 2 15 317 952 4:30
💚​ azure-ucws windows 2 15 319 950 3:16
💚​ gcp linux 2 15 230 1038 3:19
💚​ gcp windows 2 15 232 1036 2:39
21 interesting tests: 13 SKIP, 7 KNOWN, 1 RECOVERED
Test Name aws linux aws windows aws-ucws linux aws-ucws windows azure linux azure windows azure-ucws linux azure-ucws windows gcp linux gcp windows
🟨​ TestAccept 🟨​K 🟨​K 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/invariant/no_drift 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/permissions 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions 🟨​K 🟨​K 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions 🟨​K 🟨​K 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 🟨​K 🟨​K 💚​R 💚​R
🙈​ TestAccept/bundle/resources/postgres_branches/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/replace_existing 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/update_protected 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/without_branch_id 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_endpoints/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_projects/update_display_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/synced_database_tables/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestFetchRepositoryInfoAPI_FromRepo 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R

Merged via the queue into main with commit cb73bba Jun 30, 2026
23 checks passed
@pietern pietern deleted the structwalk-drop-upper-bound branch June 30, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@denik denik denik approved these changes
@janniklasrose janniklasrose janniklasrose approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pietern @denik @eng-dev-ecosystem-bot @janniklasrose