Skip to content

test(adk): assert real tls_insecure_skip_verify tag in omitempty test#2199

Open
anxkhn wants to merge 1 commit into
kagent-dev:mainfrom
anxkhn:patch-12
Open

test(adk): assert real tls_insecure_skip_verify tag in omitempty test#2199
anxkhn wants to merge 1 commit into
kagent-dev:mainfrom
anxkhn:patch-12

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What

TestMarshalJSON_OmitemptyFields (go/api/adk/types_test.go) checks that
zero-valued omitempty fields are omitted from the marshalled model JSON by
listing the expected-absent JSON keys in wantAbsent. The OpenAI and Anthropic
cases listed "tls_disable_verify", but no Go struct field serializes to that
key: the TLS-verify field on BaseModel is

TLSInsecureSkipVerify *bool `json:"tls_insecure_skip_verify,omitempty"`

so it marshals to tls_insecure_skip_verify. Because the marshaller never emits
tls_disable_verify, raw["tls_disable_verify"] was always absent and the
assertion was vacuously true, it could never fail. As a result the omitempty
behavior of the TLS-verify pointer was not actually covered by this test.
tls_disable_verify is a Python-only legacy validation alias
(python/packages/kagent-adk/.../types.py, via AliasChoices) and has never
been a Go json tag.

This changes the two wantAbsent entries to the real json tag
tls_insecure_skip_verify so the assertion exercises the intended omission
behavior. No production code is touched; the ,omitempty tag is already correct,
so the corrected test stays green on the current code.

Why it is correct

The sibling presence test TestMarshalJSON_BaseModelFields already asserts on
raw["tls_insecure_skip_verify"], confirming that is the tag the marshaller
emits and that the absent-key form was a copy/paste slip from the Python field
name.

How it was verified

Temporarily dropping ,omitempty from BaseModel.TLSInsecureSkipVerify makes the
corrected test fail:

field "tls_insecure_skip_verify" should be omitted when zero-valued, but was present

while the previous tls_disable_verify assertion still passed under the same
broken condition, demonstrating the old check was a no-op. Restoring ,omitempty
makes the corrected test pass again.

Local checks (module github.com/kagent-dev/kagent/go, go 1.26):

  • go test ./api/adk/ -run TestMarshalJSON_OmitemptyFields -v -> PASS (4/4)
  • go test -race ./api/adk/ -> PASS
  • gofmt -l, go vet ./api/adk/, golangci-lint run ./api/adk/... -> clean / 0 issues

TestMarshalJSON_OmitemptyFields listed "tls_disable_verify" in the
wantAbsent slices for the OpenAI and Anthropic cases, but no Go struct
field serializes to that key: the TLS-verify field on BaseModel is
tagged tls_insecure_skip_verify. Because the marshaller never emits
tls_disable_verify, the absence check was vacuously true and could
never fail, leaving the omitempty behavior of the TLS-verify pointer
untested (tls_disable_verify is a Python-only legacy alias).

Use the actual json tag tls_insecure_skip_verify so the assertion
exercises the intended omitempty behavior. Verified by temporarily
dropping ,omitempty from the field: the corrected test then fails
(the old one still passed), and it passes again once ,omitempty is
restored.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@anxkhn anxkhn requested a review from a team as a code owner July 9, 2026 21:23
Copilot AI review requested due to automatic review settings July 9, 2026 21:23
@github-actions github-actions Bot added the testing Additional testing required label Jul 9, 2026

Copilot AI 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.

Pull request overview

This PR fixes a gap in Go JSON marshaling test coverage by updating TestMarshalJSON_OmitemptyFields to assert omission of the actual TLS verification JSON key (tls_insecure_skip_verify) rather than a non-existent/legacy alias (tls_disable_verify), ensuring the test can fail when omitempty behavior regresses.

Changes:

  • Update OpenAI and Anthropic wantAbsent lists to use tls_insecure_skip_verify.
  • Make the omitempty assertion meaningful for BaseModel.TLSInsecureSkipVerify (a *bool tagged json:"tls_insecure_skip_verify,omitempty").

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Additional testing required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants