mirror of
https://github.com/temporalio/temporal.git
synced 2026-08-30 18:41:49 -07:00
Fix flaky AttachLinksOnConflictUnionsLinks by comparing links as a set (#10740)
## What changed? - tests/activity_standalone_test.go: switched the assertion in AttachLinksOnConflictUnionsLinks from the order-sensitive protorequire.ProtoSliceEqual to the order-agnostic protorequire.ProtoElementsMatch. - common/testing/protorequire/require.go: added ProtoElementsMatch, and dropped the dependency on protoassert by inlining the comparison logic (using temporalproto.DeepEqual for element matching). - common/testing/protorequire/require_test.go: added TestProtoElementsMatch (in-order, out-of-order, struct-containing-proto, nested-proto, and a negative case) and TestDeepEqual (positive + negative). ## Why? The test was flaky: it passed in isolation but failed when run alongside the rest of the suite. Server-side, componentLinks (chasm/tree.go) builds the response by iterating a Go map keyed by request ID, so the order between request batches is non-deterministic. ProtoElementsMatch did not exist in protorequire, only in protoassert. Adding it (and removing the cross-package dependency) keeps protorequire self-contained and consistent with the other require-style helpers. ## How did you test it? - [X] built - [ ] run locally and tested manually - [X] covered by existing tests - [X] added new unit test(s) - [ ] added new functional test(s)
This commit is contained in:
@@ -2,10 +2,11 @@ package protorequire
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"reflect"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.temporal.io/server/common/testing/protoassert"
|
||||
"go.temporal.io/api/temporalproto"
|
||||
"google.golang.org/protobuf/proto"
|
||||
"google.golang.org/protobuf/reflect/protoreflect"
|
||||
"google.golang.org/protobuf/testing/protocmp"
|
||||
@@ -60,8 +61,8 @@ func NotProtoEqual(t require.TestingT, a proto.Message, b proto.Message) {
|
||||
if th, ok := t.(helper); ok {
|
||||
th.Helper()
|
||||
}
|
||||
if !protoassert.NotProtoEqual(t, a, b) {
|
||||
t.FailNow()
|
||||
if diff := cmp.Diff(a, b, protocmp.Transform()); diff == "" {
|
||||
require.Fail(t, "Expected protos to differ but they did not")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,8 +74,57 @@ func ProtoSliceEqual[T proto.Message](t require.TestingT, a []T, b []T) {
|
||||
if th, ok := t.(helper); ok {
|
||||
th.Helper()
|
||||
}
|
||||
if !protoassert.ProtoSliceEqual(t, a, b) {
|
||||
t.FailNow()
|
||||
if len(a) != len(b) {
|
||||
require.Fail(t, fmt.Sprintf("Proto slice length mismatch: want %d, got %d", len(a), len(b)))
|
||||
return
|
||||
}
|
||||
for i := range a {
|
||||
if diff := cmp.Diff(a[i], b[i], protocmp.Transform()); diff != "" {
|
||||
require.Fail(t, fmt.Sprintf("Proto mismatch at index %d (-want +got):\n%v", i, diff))
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ProtoElementsMatch behaves like require.ElementsMatch except in that it works for protobuf-generated structs.
|
||||
// Both arguments must be slices or arrays.
|
||||
func ProtoElementsMatch(t require.TestingT, a any, b any, msgAndArgs ...any) {
|
||||
if th, ok := t.(helper); ok {
|
||||
th.Helper()
|
||||
}
|
||||
aVal := reflect.ValueOf(a)
|
||||
bVal := reflect.ValueOf(b)
|
||||
if aVal.Len() != bVal.Len() {
|
||||
require.Fail(t, fmt.Sprintf("element count mismatch: want %d, got %d\nA: %+v\nB: %+v", aVal.Len(), bVal.Len(), a, b), msgAndArgs...)
|
||||
return
|
||||
}
|
||||
used := make([]bool, bVal.Len())
|
||||
for i := 0; i < aVal.Len(); i++ {
|
||||
want := aVal.Index(i).Interface()
|
||||
matched := false
|
||||
for j := 0; j < bVal.Len(); j++ {
|
||||
if used[j] {
|
||||
continue
|
||||
}
|
||||
if temporalproto.DeepEqual(want, bVal.Index(j).Interface()) {
|
||||
used[j] = true
|
||||
matched = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !matched {
|
||||
require.Fail(t, fmt.Sprintf("element not found in B: %+v\nA: %+v\nB: %+v", want, a, b), msgAndArgs...)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func DeepEqual(t require.TestingT, a any, b any) {
|
||||
if th, ok := t.(helper); ok {
|
||||
th.Helper()
|
||||
}
|
||||
if !temporalproto.DeepEqual(a, b) {
|
||||
require.Fail(t, "Values are not deeply equal")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,24 +139,19 @@ func (x ProtoAssertions) NotProtoEqual(a proto.Message, b proto.Message) {
|
||||
if th, ok := x.t.(helper); ok {
|
||||
th.Helper()
|
||||
}
|
||||
if !protoassert.NotProtoEqual(x.t, a, b) {
|
||||
x.t.FailNow()
|
||||
}
|
||||
NotProtoEqual(x.t, a, b)
|
||||
}
|
||||
|
||||
func (x ProtoAssertions) DeepEqual(a any, b any) {
|
||||
if th, ok := x.t.(helper); ok {
|
||||
th.Helper()
|
||||
}
|
||||
if !protoassert.DeepEqual(x.t, a, b) {
|
||||
x.t.FailNow()
|
||||
}
|
||||
DeepEqual(x.t, a, b)
|
||||
}
|
||||
|
||||
func (x ProtoAssertions) ProtoElementsMatch(a any, b any) bool {
|
||||
func (x ProtoAssertions) ProtoElementsMatch(a any, b any, msgAndArgs ...any) {
|
||||
if th, ok := x.t.(helper); ok {
|
||||
th.Helper()
|
||||
}
|
||||
|
||||
return protoassert.ProtoElementsMatch(x.t, a, b)
|
||||
ProtoElementsMatch(x.t, a, b, msgAndArgs...)
|
||||
}
|
||||
|
||||
@@ -61,3 +61,142 @@ func TestProtoEqualIgnoreFields(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
type canHazProto struct {
|
||||
A float64
|
||||
B *commonpb.WorkflowExecution
|
||||
}
|
||||
|
||||
func TestProtoElementsMatch(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
Name string
|
||||
A any
|
||||
B any
|
||||
}{{
|
||||
Name: "Shallow proto object - in order",
|
||||
A: []*commonpb.WorkflowExecution{{
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
}, {
|
||||
WorkflowId: "second workflow",
|
||||
RunId: myUUID,
|
||||
}},
|
||||
B: []*commonpb.WorkflowExecution{{
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
}, {
|
||||
WorkflowId: "second workflow",
|
||||
RunId: myUUID,
|
||||
}},
|
||||
}, {
|
||||
Name: "Shallow proto object - out of order",
|
||||
A: []*commonpb.WorkflowExecution{{
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
}, {
|
||||
WorkflowId: "second workflow",
|
||||
RunId: myUUID,
|
||||
}},
|
||||
B: []*commonpb.WorkflowExecution{{
|
||||
WorkflowId: "second workflow",
|
||||
RunId: myUUID,
|
||||
}, {
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
}},
|
||||
}, {
|
||||
Name: "Structs containing proto",
|
||||
A: []canHazProto{{
|
||||
A: 13,
|
||||
B: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}, {
|
||||
A: 12,
|
||||
B: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "second random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}},
|
||||
B: []canHazProto{{
|
||||
A: 12,
|
||||
B: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "second random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}, {
|
||||
A: 13,
|
||||
B: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}},
|
||||
}, {
|
||||
Name: "Nested proto struct",
|
||||
A: []*workflowpb.WorkflowExecutionInfo{
|
||||
{
|
||||
Execution: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}, {
|
||||
Execution: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "second random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}},
|
||||
B: []*workflowpb.WorkflowExecutionInfo{
|
||||
{
|
||||
Execution: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "second random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}, {
|
||||
Execution: &commonpb.WorkflowExecution{
|
||||
WorkflowId: "some random workflow ID",
|
||||
RunId: myUUID,
|
||||
},
|
||||
}},
|
||||
}} {
|
||||
t.Run(tc.Name, func(t *testing.T) {
|
||||
mt := &mockT{}
|
||||
protorequire.ProtoElementsMatch(mt, tc.A, tc.B)
|
||||
if mt.failed {
|
||||
t.Error("expected equality")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("mismatch fails", func(t *testing.T) {
|
||||
a := []*commonpb.WorkflowExecution{{WorkflowId: "wf-a", RunId: myUUID}}
|
||||
b := []*commonpb.WorkflowExecution{{WorkflowId: "wf-b", RunId: myUUID}}
|
||||
mt := &mockT{}
|
||||
protorequire.ProtoElementsMatch(mt, a, b)
|
||||
if !mt.failed {
|
||||
t.Fatal("expected comparison to fail for differing element sets")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestDeepEqual(t *testing.T) {
|
||||
a := &commonpb.WorkflowExecution{WorkflowId: "wf-1", RunId: myUUID}
|
||||
|
||||
t.Run("equal values pass", func(t *testing.T) {
|
||||
b := &commonpb.WorkflowExecution{WorkflowId: "wf-1", RunId: myUUID}
|
||||
mt := &mockT{}
|
||||
protorequire.DeepEqual(mt, a, b)
|
||||
if mt.failed {
|
||||
t.Fatal("expected equal values to pass")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("differing values fail", func(t *testing.T) {
|
||||
b := &commonpb.WorkflowExecution{WorkflowId: "wf-2", RunId: myUUID}
|
||||
mt := &mockT{}
|
||||
protorequire.DeepEqual(mt, a, b)
|
||||
if !mt.failed {
|
||||
t.Fatal("expected differing values to fail")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -885,7 +885,8 @@ func (s *standaloneActivityTestSuite) TestStart() {
|
||||
require.NoError(t, err)
|
||||
expected := append([]*commonpb.Link{}, firstLinks...)
|
||||
expected = append(expected, secondLinks...)
|
||||
protorequire.ProtoSliceEqual(t, expected, descResp.GetInfo().GetLinks())
|
||||
// Links across requests are stored in a map keyed by request ID, so their relative order is non-deterministic.
|
||||
protorequire.ProtoElementsMatch(t, expected, descResp.GetInfo().GetLinks())
|
||||
})
|
||||
|
||||
t.Run("AttachLinksOnConflictStoresRawInput", func(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user