Skip to content

chore: refactor tests related to AIP 4235#17027

Open
parthea wants to merge 5 commits into
mainfrom
use-empty-string-instead-of-none
Open

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

Conversation

@parthea
Copy link
Copy Markdown
Contributor

@parthea parthea commented May 11, 2026

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

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

Copy link
Copy Markdown
Contributor

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

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
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