Skip to content

fix(flaky test): use proto.Equal for gRPC status comparison in TestUpdateFeature - #2774

Merged
t-kikuc merged 2 commits into
mainfrom
fix/flaky-update-feature-test
Aug 14, 2026
Merged

fix(flaky test): use proto.Equal for gRPC status comparison in TestUpdateFeature#2774
t-kikuc merged 2 commits into
mainfrom
fix/flaky-update-feature-test

Conversation

@t-kikuc

@t-kikuc t-kikuc commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Fix flaky TestUpdateFeature/fail:_variation_value_schema_violation_returns_InvalidArgument by replacing assert.Equal with proto.Equal for gRPC status error comparison.

Background

The test added in #2765 compares gRPC status errors using assert.Equal, which compares serialized proto bytes. When ErrorInfo.Metadata contains multiple map entries (messageKey + field), Go's non-deterministic map iteration order causes the proto serialization order to vary between runs, leading to intermittent test failures. Using proto.Equal performs semantic comparison that is insensitive to map field ordering.

The status error comparison used assert.Equal on serialized proto bytes,
which is non-deterministic when ErrorInfo.Metadata has multiple map
entries (messageKey + field). Switch to proto.Equal for semantic
comparison.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 14, 2026 05:41

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

Pull request overview

Attempts to stabilize TestUpdateFeature by semantically comparing gRPC statuses.

Changes:

  • Adds protobuf comparison support.
  • Reworks success and error assertions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/feature/api/feature_test.go Outdated
@t-kikuc t-kikuc changed the title fix: use proto.Equal for gRPC status comparison in TestUpdateFeature fix(flaky test): use proto.Equal for gRPC status comparison in TestUpdateFeature Aug 14, 2026
proto.Equal on Status.Proto() compares Any.value raw bytes, which still
suffers from non-deterministic map serialization in ErrorInfo.Metadata.
Instead, compare code/message individually and use Details() to unpack
each Any into its concrete proto.Message before comparing with
proto.Equal.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@t-kikuc
t-kikuc marked this pull request as ready for review August 14, 2026 06:10
@t-kikuc
t-kikuc enabled auto-merge (squash) August 14, 2026 06:10

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

Thank you!

@t-kikuc
t-kikuc merged commit 941987f into main Aug 14, 2026
12 checks passed
@t-kikuc
t-kikuc deleted the fix/flaky-update-feature-test branch August 14, 2026 06:42
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