Skip to content

Commit 3ca6328

Browse files
GroxxGroxx
authored
Internal workflow client test improvements (#1331)
Followups from #1327: 1. simplifying the memo/search-attr encoding tests 2. making sure the dataconverter tests can detect correct vs incorrect encoder use 1 is pretty simple: memos and search attrs are ALWAYS JSON because the server must interpret them, so the exact bytes should not change in the future. A hardcoded string is easy to verify, and strengthens the guarantee that "this should not change even if other encoding changes". 2 is more interesting: as originally written, the test would still pass if the custom dataconverter was *not* saved and used. The type-assertion to check the internal field somewhat prevents that, but it's unnecessary and a disjointed check. So it has been rewritten: now the serialized request bytes must clearly come from the test encoder, not the default encoder, and they must be detectably different.
1 parent 08b284a commit 3ca6328

File tree

1 file changed

+32
-53
lines changed

1 file changed

+32
-53
lines changed

‎internal/internal_workflow_client_test.go

Lines changed: 32 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ package internal
2121

2222
import (
2323
"context"
24-
"encoding/json"
2524
"errors"
2625
"fmt"
2726
"log"
@@ -32,6 +31,7 @@ import (
3231
"github.com/golang/mock/gomock"
3332
"github.com/pborman/uuid"
3433
"github.com/stretchr/testify/assert"
34+
"github.com/stretchr/testify/require"
3535
"github.com/stretchr/testify/suite"
3636
"go.uber.org/yarpc"
3737

@@ -1143,8 +1143,7 @@ func (s *workflowClientTestSuite) TestStartWorkflow_WithContext() {
11431143

11441144
func (s *workflowClientTestSuite) TestStartWorkflow_WithDataConverter() {
11451145
dc := newTestDataConverter()
1146-
s.client = NewClient(s.service, domain, &ClientOptions{DataConverter: dc})
1147-
client := s.client.(*workflowClient)
1146+
client := NewClient(s.service, domain, &ClientOptions{DataConverter: dc})
11481147
options := StartWorkflowOptions{
11491148
ID: workflowID,
11501149
TaskList: tasklist,
@@ -1156,21 +1155,23 @@ func (s *workflowClientTestSuite) TestStartWorkflow_WithDataConverter() {
11561155
}
11571156
input := []byte("test")
11581157

1158+
correctlyEncoded, err := dc.ToData([]interface{}{input})
1159+
require.NoError(s.T(), err, "test data converter should not fail on simple inputs")
1160+
defaultEncoded, err := getDefaultDataConverter().ToData([]interface{}{input})
1161+
require.NoError(s.T(), err, "default data converter should not fail on simple inputs")
1162+
1163+
// sanity check: we must be able to tell right from wrong
1164+
require.NotEqual(s.T(), correctlyEncoded, defaultEncoded, "test data converter should encode differently or the test is not valid")
1165+
11591166
createResponse := &shared.StartWorkflowExecutionResponse{
11601167
RunId: common.StringPtr(runID),
11611168
}
11621169
s.service.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(createResponse, nil).
11631170
Do(func(_ interface{}, req *shared.StartWorkflowExecutionRequest, _ ...interface{}) {
1164-
dc := client.dataConverter
1165-
encodedArg, _ := dc.ToData(input)
1166-
s.Equal(req.Input, encodedArg)
1167-
var decodedArg []byte
1168-
dc.FromData(req.Input, &decodedArg)
1169-
s.Equal(input, decodedArg)
1171+
assert.Equal(s.T(), correctlyEncoded, req.Input, "client-encoded data should use the customized data converter")
11701172
})
11711173

11721174
resp, err := client.StartWorkflow(context.Background(), options, f1, input)
1173-
s.Equal(newTestDataConverter(), client.dataConverter)
11741175
s.NoError(err)
11751176
s.Equal(createResponse.GetRunId(), resp.RunID)
11761177
}
@@ -1197,14 +1198,8 @@ func (s *workflowClientTestSuite) TestStartWorkflow_WithMemoAndSearchAttr() {
11971198

11981199
s.service.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(startResp, nil).
11991200
Do(func(_ interface{}, req *shared.StartWorkflowExecutionRequest, _ ...interface{}) {
1200-
var resultMemo, resultAttr string
1201-
err := json.Unmarshal(req.Memo.Fields["testMemo"], &resultMemo)
1202-
s.NoError(err)
1203-
s.Equal("memo value", resultMemo)
1204-
1205-
err = json.Unmarshal(req.SearchAttributes.IndexedFields["testAttr"], &resultAttr)
1206-
s.NoError(err)
1207-
s.Equal("attr value", resultAttr)
1201+
s.Equal(`"memo value"`, req.Memo.Fields["testMemo"], "memos must be JSON-encoded")
1202+
s.Equal(`"attr value"`, req.SearchAttributes.IndexedFields["testAttr"], "search attributes must be JSON-encoded")
12081203
})
12091204

12101205
_, err := s.client.StartWorkflow(context.Background(), options, wf)
@@ -1271,14 +1266,8 @@ func (s *workflowClientTestSuite) TestSignalWithStartWorkflow_WithMemoAndSearchA
12711266
s.service.EXPECT().SignalWithStartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any(),
12721267
gomock.Any(), gomock.Any(), gomock.Any()).Return(startResp, nil).
12731268
Do(func(_ interface{}, req *shared.SignalWithStartWorkflowExecutionRequest, _ ...interface{}) {
1274-
var resultMemo, resultAttr string
1275-
err := json.Unmarshal(req.Memo.Fields["testMemo"], &resultMemo)
1276-
s.NoError(err)
1277-
s.Equal("memo value", resultMemo)
1278-
1279-
err = json.Unmarshal(req.SearchAttributes.IndexedFields["testAttr"], &resultAttr)
1280-
s.NoError(err)
1281-
s.Equal("attr value", resultAttr)
1269+
s.Equal(`"memo value"`, req.Memo.Fields["testMemo"], "memos must be JSON-encoded")
1270+
s.Equal(`"attr value"`, req.SearchAttributes.IndexedFields["testAttr"], "search attributes must be JSON-encoded")
12821271
})
12831272

12841273
_, err := s.client.SignalWithStartWorkflow(context.Background(), "wid", "signal", "value", options, wf)
@@ -1308,14 +1297,8 @@ func (s *workflowClientTestSuite) TestSignalWithStartWorkflowAsync_WithMemoAndSe
13081297
gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).
13091298
Do(func(_ interface{}, asyncReq *shared.SignalWithStartWorkflowExecutionAsyncRequest, _ ...interface{}) {
13101299
req := asyncReq.Request
1311-
var resultMemo, resultAttr string
1312-
err := json.Unmarshal(req.Memo.Fields["testMemo"], &resultMemo)
1313-
s.NoError(err)
1314-
s.Equal("memo value", resultMemo)
1315-
1316-
err = json.Unmarshal(req.SearchAttributes.IndexedFields["testAttr"], &resultAttr)
1317-
s.NoError(err)
1318-
s.Equal("attr value", resultAttr)
1300+
s.Equal(`"memo value"`, req.Memo.Fields["testMemo"], "memos must be JSON-encoded")
1301+
s.Equal(`"attr value"`, req.SearchAttributes.IndexedFields["testAttr"], "search attributes must be JSON-encoded")
13191302
})
13201303

13211304
_, err := s.client.SignalWithStartWorkflowAsync(context.Background(), "wid", "signal", "value", options, wf)
@@ -1394,8 +1377,7 @@ func (s *workflowClientTestSuite) TestStartWorkflowAsync() {
13941377

13951378
func (s *workflowClientTestSuite) TestStartWorkflowAsync_WithDataConverter() {
13961379
dc := newTestDataConverter()
1397-
s.client = NewClient(s.service, domain, &ClientOptions{DataConverter: dc})
1398-
client := s.client.(*workflowClient)
1380+
client := NewClient(s.service, domain, &ClientOptions{DataConverter: dc})
13991381
options := StartWorkflowOptions{
14001382
ID: workflowID,
14011383
TaskList: tasklist,
@@ -1407,19 +1389,22 @@ func (s *workflowClientTestSuite) TestStartWorkflowAsync_WithDataConverter() {
14071389
}
14081390
input := []byte("test")
14091391

1392+
// this test requires that the overridden test-data-converter is used, so we need to make sure the encoded input looks correct.
1393+
// the test data converter's output doesn't actually matter as long as it's noticeably different.
1394+
correctlyEncoded, err := dc.ToData([]interface{}{input})
1395+
require.NoError(s.T(), err, "test data converter should not fail on simple inputs")
1396+
wrongDefaultEncoding, err := getDefaultDataConverter().ToData([]interface{}{input})
1397+
require.NoError(s.T(), err, "default data converter should not fail on simple inputs")
1398+
1399+
// sanity check: we must be able to tell right from wrong
1400+
require.NotEqual(s.T(), correctlyEncoded, wrongDefaultEncoding, "test data converter should encode differently or the test is not valid")
1401+
14101402
s.service.EXPECT().StartWorkflowExecutionAsync(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).
14111403
Do(func(_ interface{}, asyncReq *shared.StartWorkflowExecutionAsyncRequest, _ ...interface{}) {
1412-
req := asyncReq.Request
1413-
dc := client.dataConverter
1414-
encodedArg, _ := dc.ToData(input)
1415-
s.Equal(req.Input, encodedArg)
1416-
var decodedArg []byte
1417-
dc.FromData(req.Input, &decodedArg)
1418-
s.Equal(input, decodedArg)
1404+
assert.Equal(s.T(), correctlyEncoded, asyncReq.Request.Input, "client-encoded data should use the customized data converter")
14191405
})
14201406

1421-
_, err := client.StartWorkflowAsync(context.Background(), options, f1, input)
1422-
s.Equal(newTestDataConverter(), client.dataConverter)
1407+
_, err = client.StartWorkflowAsync(context.Background(), options, f1, input)
14231408
s.NoError(err)
14241409
}
14251410

@@ -1445,14 +1430,8 @@ func (s *workflowClientTestSuite) TestStartWorkflowAsync_WithMemoAndSearchAttr()
14451430
s.service.EXPECT().StartWorkflowExecutionAsync(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).
14461431
Do(func(_ interface{}, asyncReq *shared.StartWorkflowExecutionAsyncRequest, _ ...interface{}) {
14471432
req := asyncReq.Request
1448-
var resultMemo, resultAttr string
1449-
err := json.Unmarshal(req.Memo.Fields["testMemo"], &resultMemo)
1450-
s.NoError(err)
1451-
s.Equal("memo value", resultMemo)
1452-
1453-
err = json.Unmarshal(req.SearchAttributes.IndexedFields["testAttr"], &resultAttr)
1454-
s.NoError(err)
1455-
s.Equal("attr value", resultAttr)
1433+
s.Equal(`"memo value"`, req.Memo.Fields["testMemo"], "memos must be JSON-encoded")
1434+
s.Equal(`"attr value"`, req.SearchAttributes.IndexedFields["testAttr"], "search attributes must be JSON-encoded")
14561435
})
14571436

14581437
_, err := s.client.StartWorkflowAsync(context.Background(), options, wf)

0 commit comments

Comments
 (0)