Skip to content

Guard that ValidatorConfig.Validate checks every field - #11898

Merged
chrsmith merged 4 commits into
mainfrom
chrsmith/callbacks-validator-config-check
Sep 2, 2026
Merged

Guard that ValidatorConfig.Validate checks every field#11898
chrsmith merged 4 commits into
mainfrom
chrsmith/callbacks-validator-config-check

Conversation

@chrsmith

@chrsmith chrsmith commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What changed?

Add a new unit test for common/callbacks.ValidatorConfig.

Why?

While working on a PR that extended ValidatorConfig and adding new fields, some bugs crept in because the new fields weren't checked in the Validate function. This unit test will now catch those automatically, by running Validate on an empty ValidatorConfig, and then confirming that every field of the type is found in the error message. (Reporting that it is uninitialized.)

This PR also allows me to remove this tiny change from an otherwise monstrous PR I am trying to slim down.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Potential risks

None

Validate() names each required field by hand, so a field added to
ValidatorConfig without a matching check stays nil and nil-panics on the
request path instead of failing at construction. Assert via reflection that
the error for an empty config names every field.

All four fields on main are checked today; this keeps that true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chrsmith
chrsmith marked this pull request as ready for review September 2, 2026 16:24
@chrsmith
chrsmith requested review from a team as code owners September 2, 2026 16:24
@chrsmith
chrsmith requested review from S15 and stephanos September 2, 2026 16:24
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @chrsmith's task in 2m 30s —— View job


Reviewed 1 file, 2 findings.
branch

Comment thread common/callbacks/validator_test.go Outdated
Comment thread common/callbacks/validator_test.go Outdated
chrsmith and others added 3 commits September 2, 2026 09:39
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@chrsmith
chrsmith enabled auto-merge (squash) September 2, 2026 17:18
@chrsmith
chrsmith merged commit e4fd5c4 into main Sep 2, 2026
62 of 64 checks passed
@chrsmith
chrsmith deleted the chrsmith/callbacks-validator-config-check branch September 2, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants