Skip to content

test(contract): pass --uris as a JSON array - #55

Merged
2ynn merged 2 commits into
mainfrom
tests/990640099
Sep 30, 2026
Merged

2ynn merged 2 commits into
mainfrom
tests/990640099

Conversation

@2ynn

@2ynn 2ynn commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

copybara/990640099 regenerates the CLI with cli.arrayFlagFormat: json, so files register --uris takes one JSON array and rejects the bare repeatable value this test passed ("expected a JSON array").

Pass '["gs://bucket/object"]' and assert the request body carries the URI as an array element ({"uris":["gs://bucket/object"]}), so the test also catches the array text being sent as a single string.

@2ynn
2ynn requested a review from a team September 30, 2026 01:27

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the contract tests in required_flags_test.go to verify that the --uris flag correctly sends a JSON array in the request body. The feedback suggests checking the error returned by io.ReadAll to prevent silent failures, and using a type assertion instead of fmt.Sprint on the atomic value to safely handle potential nil values and improve idiomatic Go usage.

Comment thread tests/contract/required_flags_test.go Resolved
Comment thread tests/contract/required_flags_test.go Resolved
copybara/990640099 regenerates the CLI with cli.arrayFlagFormat: json,
so files register --uris takes one JSON array and rejects the bare
repeatable value this test passed ("expected a JSON array").

Pass '["gs://bucket/object"]' and assert the request body carries the
URI as an array element ({"uris":["gs://bucket/object"]}), so the test
also catches the array text being sent as a single string.
Address review: read the captured body with a type assertion instead of
fmt.Sprint on the atomic.Value (no request reads as "", not <nil>), and
quote both sides of the mismatch so whitespace is visible.
@2ynn
2ynn merged commit 2854781 into main Sep 30, 2026
8 checks passed
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