Skip to content

chore: refactor tests related to AIP 4235#17027

Merged
parthea merged 6 commits into
mainfrom
use-empty-string-instead-of-none
May 22, 2026
Merged

chore: refactor tests related to AIP 4235#17027
parthea merged 6 commits into
mainfrom
use-empty-string-instead-of-none

Conversation

@parthea

@parthea parthea commented May 11, 2026

Copy link
Copy Markdown
Contributor

This PR depends on #16944. Please review #16944 first.

The PR addresses the feedback in #16944 (comment) , specifically 2.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request modifies test templates and golden files by replacing None assignments with empty strings to clear auto-populated fields in request objects. The reviewer suggests that using del is more appropriate than assigning an empty string, as the latter explicitly sets a value rather than unsetting the field. This distinction is important for backends that no longer treat empty strings as 'no-op' signals, especially when dealing with proto3 optional fields.

@parthea parthea force-pushed the use-empty-string-instead-of-none branch from 55fb3e2 to ef0883c Compare May 11, 2026 17:13
@parthea parthea changed the title chore: use empty string instead of None when clearing Protobuf strings chore: use mock.ANY instead of None in empty request tests to cater for AIP-4235 May 11, 2026
@parthea parthea force-pushed the use-empty-string-instead-of-none branch from ef0883c to a527b09 Compare May 11, 2026 17:15
@parthea parthea marked this pull request as draft May 11, 2026 17:23
@parthea parthea changed the title chore: use mock.ANY instead of None in empty request tests to cater for AIP-4235 chore: use empty string instead of None when clearing Protobuf strings to cater for AIP-4235 May 11, 2026
@parthea parthea force-pushed the use-empty-string-instead-of-none branch from a527b09 to 68a6358 Compare May 11, 2026 17:34
@parthea parthea force-pushed the use-empty-string-instead-of-none branch from 68a6358 to e07298e Compare May 11, 2026 17:49
@parthea parthea changed the title chore: use empty string instead of None when clearing Protobuf strings to cater for AIP-4235 chore: refactor tests related to AIP 4235 May 11, 2026
@parthea parthea marked this pull request as ready for review May 11, 2026 17:50
Base automatically changed from repro-508597813 to main May 11, 2026 18:14

@chalmerlowe chalmerlowe 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.

LGTM

@parthea parthea assigned parthea and unassigned ohmayr May 22, 2026
@parthea parthea merged commit 1e6989d into main May 22, 2026
90 of 92 checks passed
@parthea parthea deleted the use-empty-string-instead-of-none branch May 22, 2026 16:47
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.

3 participants