Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pietern merged 2 commits into from
Jun 30, 2026
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e454897
structwalk: drop the upper-bound field-count assertion that churns on…
pietern Jun 30, 2026
1be1cb6
Apply suggestions from code review
pietern Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions libs/structs/structwalk/walktype_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,14 @@ func TestTypeSelf(t *testing.T) {
}, getScalarFields(t, reflect.TypeFor[Self]()))
}

func testStruct(t *testing.T, typ reflect.Type, minLen, maxLen int, present map[string]any, notPresent []string) {
func testStruct(t *testing.T, typ reflect.Type, minLen int, present map[string]any, notPresent []string) {
results := getScalarFields(t, typ)

// Only assert a lower bound on the field count: SDK bumps regularly add fields,
// so an upper bound churns every few bumps without catching anything. A runaway
// walk (e.g. circular references failing to terminate) is caught precisely by the
// notPresent checks below instead.
assert.Greater(t, len(results), minLen, "Expected to find many fields in %s", typ)
assert.Less(t, len(results), maxLen, "Expected to find not so many fields in %s", typ)

for path, expectedValue := range present {
value, found := results[path]
Expand All @@ -113,7 +116,7 @@ func TestTypeJobSettings(t *testing.T) {
testStruct(t,
reflect.TypeFor[jobs.JobSettings](),
// Verify we found a reasonable number of fields (605 after SDK v0.136.0 bump)
500, 650,
500,
map[string]any{
"name": "",
"timeout_seconds": 0,
Expand All @@ -136,7 +139,7 @@ func TestTypeJobSettings(t *testing.T) {
func TestTypeRoot(t *testing.T) {
testStruct(t,
reflect.TypeFor[config.Root](),
5000, 6000, // 5813 after genie_space resource added
5000, // 5813 after genie_space resource added
Comment thread
pietern marked this conversation as resolved.
Outdated
map[string]any{
"bundle.target": "",
`variables.*.lookup.dashboard`: "",
Expand Down
Loading