Skip to content

Commit aee6739

Browse files
Groxxvenkat1109
Groxx
authored and
venkat1109
committed
Parallelizing some tests (#899)
1 parent cc25a04 commit aee6739

File tree

9 files changed

+207
-163
lines changed

9 files changed

+207
-163
lines changed

‎internal/common/metrics/scope_test.go‎

Lines changed: 65 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -21,97 +21,69 @@
2121
package metrics
2222

2323
import (
24+
"io"
25+
"sync"
2426
"testing"
2527
"time"
2628

2729
"github.com/stretchr/testify/require"
2830
"github.com/uber-go/tally"
29-
"io"
30-
"sync"
3131
)
3232

3333
func Test_Counter(t *testing.T) {
34-
isReplay := true
35-
scope, closer, reporter := NewMetricsScope(&isReplay)
36-
scope.Counter("test-name").Inc(1)
37-
closer.Close()
38-
require.Equal(t, 0, len(reporter.Counts()))
39-
40-
isReplay = false
41-
scope, closer, reporter = NewMetricsScope(&isReplay)
42-
scope.Counter("test-name").Inc(3)
43-
closer.Close()
44-
require.Equal(t, 1, len(reporter.Counts()))
45-
require.Equal(t, int64(3), reporter.Counts()[0].Value())
34+
t.Parallel()
35+
replayed, executed := withScope(t, func(scope tally.Scope) {
36+
scope.Counter("test-name").Inc(3)
37+
})
38+
require.Equal(t, 0, len(replayed.Counts()))
39+
require.Equal(t, 1, len(executed.Counts()))
40+
require.Equal(t, int64(3), executed.Counts()[0].Value())
4641
}
4742

4843
func Test_Gauge(t *testing.T) {
49-
isReplay := true
50-
scope, closer, reporter := NewMetricsScope(&isReplay)
51-
scope.Gauge("test-name").Update(1)
52-
closer.Close()
53-
require.Equal(t, 0, len(reporter.Gauges()))
54-
55-
isReplay = false
56-
scope, closer, reporter = NewMetricsScope(&isReplay)
57-
scope.Gauge("test-name").Update(3)
58-
closer.Close()
59-
require.Equal(t, 1, len(reporter.Gauges()))
60-
require.Equal(t, float64(3), reporter.Gauges()[0].Value())
44+
t.Parallel()
45+
replayed, executed := withScope(t, func(scope tally.Scope) {
46+
scope.Gauge("test-name").Update(3)
47+
})
48+
require.Equal(t, 0, len(replayed.Gauges()))
49+
require.Equal(t, 1, len(executed.Gauges()))
50+
require.Equal(t, float64(3), executed.Gauges()[0].Value())
6151
}
6252

6353
func Test_Timer(t *testing.T) {
64-
isReplay := true
65-
scope, closer, reporter := NewMetricsScope(&isReplay)
66-
scope.Timer("test-name").Record(time.Second)
67-
sw := scope.Timer("test-stopwatch").Start()
68-
sw.Stop()
69-
closer.Close()
70-
require.Equal(t, 0, len(reporter.Timers()))
71-
72-
isReplay = false
73-
scope, closer, reporter = NewMetricsScope(&isReplay)
74-
scope.Timer("test-name").Record(time.Second)
75-
sw = scope.Timer("test-stopwatch").Start()
76-
sw.Stop()
77-
closer.Close()
78-
require.Equal(t, 2, len(reporter.Timers()))
79-
require.Equal(t, time.Second, reporter.Timers()[0].Value())
54+
t.Parallel()
55+
replayed, executed := withScope(t, func(scope tally.Scope) {
56+
scope.Timer("test-name").Record(time.Second)
57+
scope.Timer("test-stopwatch").Start().Stop()
58+
})
59+
require.Equal(t, 0, len(replayed.Timers()))
60+
require.Equal(t, 2, len(executed.Timers()))
61+
require.Equal(t, time.Second, executed.Timers()[0].Value())
8062
}
8163

8264
func Test_Histogram(t *testing.T) {
83-
isReplay := true
84-
scope, closer, reporter := NewMetricsScope(&isReplay)
85-
valueBuckets := tally.MustMakeLinearValueBuckets(0, 10, 10)
86-
scope.Histogram("test-hist-1", valueBuckets).RecordValue(5)
87-
scope.Histogram("test-hist-2", valueBuckets).RecordValue(15)
88-
closer.Close()
89-
require.Equal(t, 0, len(reporter.HistogramValueSamples()))
90-
scope, closer, reporter = NewMetricsScope(&isReplay)
91-
durationBuckets := tally.MustMakeLinearDurationBuckets(0, time.Hour, 10)
92-
scope.Histogram("test-hist-1", durationBuckets).RecordDuration(time.Minute)
93-
scope.Histogram("test-hist-2", durationBuckets).RecordDuration(time.Minute * 61)
94-
sw := scope.Histogram("test-hist-3", durationBuckets).Start()
95-
sw.Stop()
96-
closer.Close()
97-
require.Equal(t, 0, len(reporter.HistogramDurationSamples()))
98-
99-
isReplay = false
100-
scope, closer, reporter = NewMetricsScope(&isReplay)
101-
valueBuckets = tally.MustMakeLinearValueBuckets(0, 10, 10)
102-
scope.Histogram("test-hist-1", valueBuckets).RecordValue(5)
103-
scope.Histogram("test-hist-2", valueBuckets).RecordValue(15)
104-
closer.Close()
105-
require.Equal(t, 2, len(reporter.HistogramValueSamples()))
106-
107-
scope, closer, reporter = NewMetricsScope(&isReplay)
108-
durationBuckets = tally.MustMakeLinearDurationBuckets(0, time.Hour, 10)
109-
scope.Histogram("test-hist-1", durationBuckets).RecordDuration(time.Minute)
110-
scope.Histogram("test-hist-2", durationBuckets).RecordDuration(time.Minute * 61)
111-
sw = scope.Histogram("test-hist-3", durationBuckets).Start()
112-
sw.Stop()
113-
closer.Close()
114-
require.Equal(t, 3, len(reporter.HistogramDurationSamples()))
65+
t.Parallel()
66+
t.Run("values", func(t *testing.T) {
67+
t.Parallel()
68+
replayed, executed := withScope(t, func(scope tally.Scope) {
69+
valueBuckets := tally.MustMakeLinearValueBuckets(0, 10, 10)
70+
scope.Histogram("test-hist-1", valueBuckets).RecordValue(5)
71+
scope.Histogram("test-hist-2", valueBuckets).RecordValue(15)
72+
})
73+
require.Equal(t, 0, len(replayed.HistogramValueSamples()))
74+
require.Equal(t, 2, len(executed.HistogramValueSamples()))
75+
})
76+
t.Run("durations", func(t *testing.T) {
77+
t.Parallel()
78+
replayed, executed := withScope(t, func(scope tally.Scope) {
79+
durationBuckets := tally.MustMakeLinearDurationBuckets(0, time.Hour, 10)
80+
scope.Histogram("test-hist-1", durationBuckets).RecordDuration(time.Minute)
81+
scope.Histogram("test-hist-2", durationBuckets).RecordDuration(time.Minute * 61)
82+
scope.Histogram("test-hist-3", durationBuckets).Start().Stop()
83+
})
84+
require.Equal(t, 0, len(replayed.HistogramDurationSamples()))
85+
require.Equal(t, 3, len(executed.HistogramDurationSamples()))
86+
})
11587
}
11688

11789
func Test_ScopeCoverage(t *testing.T) {
@@ -190,6 +162,25 @@ func newTaggedMetricsScope() (*TaggedScope, io.Closer, *capturingStatsReporter)
190162
return &TaggedScope{Scope: scope}, closer, reporter
191163
}
192164

165+
// withScope runs your callback twice, once for "during replay" and once for "after replay" / "executing".
166+
// stats are captured, and the results are returned for your validation.
167+
func withScope(t *testing.T, cb func(scope tally.Scope)) (replayed *CapturingStatsReporter, executed *CapturingStatsReporter) {
168+
replaying, executing := true, false
169+
170+
replayingScope, replayingCloser, replayed := NewMetricsScope(&replaying)
171+
executingScope, executingCloser, executed := NewMetricsScope(&executing)
172+
173+
defer func() {
174+
require.NoError(t, replayingCloser.Close())
175+
require.NoError(t, executingCloser.Close())
176+
}()
177+
178+
cb(replayingScope)
179+
cb(executingScope)
180+
181+
return replayed, executed
182+
}
183+
193184
// capturingStatsReporter is a reporter used by tests to capture the metric so we can verify our tests.
194185
type capturingStatsReporter struct {
195186
counts []capturedCount

‎internal/common/metrics/service_wrapper_test.go‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -114,60 +114,65 @@ func runTest(
114114
name string,
115115
) {
116116
t.Run(name, func(t *testing.T) {
117+
t.Parallel()
118+
// gomock mutates the returns slice, which leads to different test values between the two runs.
119+
// copy the slice until gomock fixes it: https://github.com/golang/mock/issues/353
120+
returns := append(make([]interface{}, 0, len(test.mockReturns)), test.mockReturns...)
121+
117122
mockService, wrapperService, closer, reporter := serviceFunc(t)
118123
switch test.serviceMethod {
119124
case "DeprecateDomain":
120-
mockService.EXPECT().DeprecateDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
125+
mockService.EXPECT().DeprecateDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
121126
case "DescribeDomain":
122-
mockService.EXPECT().DescribeDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
127+
mockService.EXPECT().DescribeDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
123128
case "GetWorkflowExecutionHistory":
124-
mockService.EXPECT().GetWorkflowExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
129+
mockService.EXPECT().GetWorkflowExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
125130
case "ListClosedWorkflowExecutions":
126-
mockService.EXPECT().ListClosedWorkflowExecutions(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
131+
mockService.EXPECT().ListClosedWorkflowExecutions(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
127132
case "ListOpenWorkflowExecutions":
128-
mockService.EXPECT().ListOpenWorkflowExecutions(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
133+
mockService.EXPECT().ListOpenWorkflowExecutions(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
129134
case "PollForActivityTask":
130-
mockService.EXPECT().PollForActivityTask(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
135+
mockService.EXPECT().PollForActivityTask(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
131136
case "PollForDecisionTask":
132-
mockService.EXPECT().PollForDecisionTask(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
137+
mockService.EXPECT().PollForDecisionTask(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
133138
case "RecordActivityTaskHeartbeat":
134-
mockService.EXPECT().RecordActivityTaskHeartbeat(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
139+
mockService.EXPECT().RecordActivityTaskHeartbeat(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
135140
case "RecordActivityTaskHeartbeatByID":
136-
mockService.EXPECT().RecordActivityTaskHeartbeatByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
141+
mockService.EXPECT().RecordActivityTaskHeartbeatByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
137142
case "RegisterDomain":
138-
mockService.EXPECT().RegisterDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
143+
mockService.EXPECT().RegisterDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
139144
case "RequestCancelWorkflowExecution":
140-
mockService.EXPECT().RequestCancelWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
145+
mockService.EXPECT().RequestCancelWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
141146
case "RespondActivityTaskCanceled":
142-
mockService.EXPECT().RespondActivityTaskCanceled(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
147+
mockService.EXPECT().RespondActivityTaskCanceled(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
143148
case "RespondActivityTaskCompleted":
144-
mockService.EXPECT().RespondActivityTaskCompleted(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
149+
mockService.EXPECT().RespondActivityTaskCompleted(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
145150
case "RespondActivityTaskFailed":
146-
mockService.EXPECT().RespondActivityTaskFailed(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
151+
mockService.EXPECT().RespondActivityTaskFailed(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
147152
case "RespondActivityTaskCanceledByID":
148-
mockService.EXPECT().RespondActivityTaskCanceledByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
153+
mockService.EXPECT().RespondActivityTaskCanceledByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
149154
case "RespondActivityTaskCompletedByID":
150-
mockService.EXPECT().RespondActivityTaskCompletedByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
155+
mockService.EXPECT().RespondActivityTaskCompletedByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
151156
case "RespondActivityTaskFailedByID":
152-
mockService.EXPECT().RespondActivityTaskFailedByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
157+
mockService.EXPECT().RespondActivityTaskFailedByID(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
153158
case "RespondDecisionTaskCompleted":
154-
mockService.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
159+
mockService.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
155160
case "SignalWorkflowExecution":
156-
mockService.EXPECT().SignalWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
161+
mockService.EXPECT().SignalWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
157162
case "SignaWithStartlWorkflowExecution":
158-
mockService.EXPECT().SignalWithStartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
163+
mockService.EXPECT().SignalWithStartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
159164
case "StartWorkflowExecution":
160-
mockService.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
165+
mockService.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
161166
case "TerminateWorkflowExecution":
162-
mockService.EXPECT().TerminateWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
167+
mockService.EXPECT().TerminateWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
163168
case "ResetWorkflowExecution":
164-
mockService.EXPECT().ResetWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
169+
mockService.EXPECT().ResetWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
165170
case "UpdateDomain":
166-
mockService.EXPECT().UpdateDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
171+
mockService.EXPECT().UpdateDomain(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
167172
case "QueryWorkflow":
168-
mockService.EXPECT().QueryWorkflow(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
173+
mockService.EXPECT().QueryWorkflow(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
169174
case "RespondQueryTaskCompleted":
170-
mockService.EXPECT().RespondQueryTaskCompleted(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockReturns...)
175+
mockService.EXPECT().RespondQueryTaskCompleted(gomock.Any(), gomock.Any(), gomock.Any()).Return(returns...)
171176
}
172177

173178
callOption := yarpc.CallOption{}
@@ -178,7 +183,7 @@ func runTest(
178183
inputs = append(inputs, reflect.ValueOf(callOption))
179184
method := reflect.ValueOf(wrapperService).MethodByName(test.serviceMethod)
180185
method.Call(inputs)
181-
closer.Close()
186+
require.NoError(t, closer.Close())
182187
validationFunc(t, reporter, test.serviceMethod, test.expectedCounters)
183188
})
184189
}

‎internal/encoded_test.go‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -50,29 +50,35 @@ func testDataConverterFunction(t *testing.T, dc DataConverter, f interface{}, ar
5050
return retValues[0].Interface().(string)
5151
}
5252

53-
func testDataConverterHelper(t *testing.T, dc DataConverter) {
54-
f1 := func(ctx Context, r []byte) string {
55-
return "result"
56-
}
57-
r1 := testDataConverterFunction(t, dc, f1, new(emptyCtx), []byte("test"))
58-
require.Equal(t, r1, "result")
59-
// No parameters.
60-
f2 := func() string {
61-
return "empty-result"
62-
}
63-
r2 := testDataConverterFunction(t, dc, f2)
64-
require.Equal(t, r2, "empty-result")
65-
// Nil parameter.
66-
f3 := func(r []byte) string {
67-
return "nil-result"
68-
}
69-
r3 := testDataConverterFunction(t, dc, f3, []byte(""))
70-
require.Equal(t, r3, "nil-result")
71-
}
72-
7353
func TestDefaultDataConverter(t *testing.T) {
54+
t.Parallel()
7455
dc := getDefaultDataConverter()
75-
testDataConverterHelper(t, dc)
56+
t.Run("result", func(t *testing.T) {
57+
t.Parallel()
58+
f1 := func(ctx Context, r []byte) string {
59+
return "result"
60+
}
61+
r1 := testDataConverterFunction(t, dc, f1, new(emptyCtx), []byte("test"))
62+
require.Equal(t, r1, "result")
63+
})
64+
t.Run("empty", func(t *testing.T) {
65+
t.Parallel()
66+
// No parameters.
67+
f2 := func() string {
68+
return "empty-result"
69+
}
70+
r2 := testDataConverterFunction(t, dc, f2)
71+
require.Equal(t, r2, "empty-result")
72+
})
73+
t.Run("nil", func(t *testing.T) {
74+
t.Parallel()
75+
// Nil parameter.
76+
f3 := func(r []byte) string {
77+
return "nil-result"
78+
}
79+
r3 := testDataConverterFunction(t, dc, f3, []byte(""))
80+
require.Equal(t, r3, "nil-result")
81+
})
7682
}
7783

7884
// testDataConverter implements encoded.DataConverter using gob
@@ -105,12 +111,8 @@ func (tdc *testDataConverter) FromData(input []byte, valuePtr ...interface{}) er
105111
return nil
106112
}
107113

108-
func TestTestDataConverter(t *testing.T) {
109-
dc := newTestDataConverter()
110-
testDataConverterHelper(t, dc)
111-
}
112-
113114
func TestDecodeArg(t *testing.T) {
115+
t.Parallel()
114116
dc := getDefaultDataConverter()
115117

116118
b, err := encodeArg(dc, testErrorDetails3)

‎internal/headers_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
)
2929

3030
func TestHeaderWriter(t *testing.T) {
31+
t.Parallel()
3132
tests := []struct {
3233
name string
3334
initial *shared.Header
@@ -81,7 +82,9 @@ func TestHeaderWriter(t *testing.T) {
8182
}
8283

8384
for _, test := range tests {
85+
test := test
8486
t.Run(test.name, func(t *testing.T) {
87+
t.Parallel()
8588
writer := NewHeaderWriter(test.initial)
8689
for key, val := range test.vals {
8790
writer.Set(key, val)
@@ -92,6 +95,7 @@ func TestHeaderWriter(t *testing.T) {
9295
}
9396

9497
func TestHeaderReader(t *testing.T) {
98+
t.Parallel()
9599
tests := []struct {
96100
name string
97101
header *shared.Header
@@ -123,7 +127,9 @@ func TestHeaderReader(t *testing.T) {
123127
}
124128

125129
for _, test := range tests {
130+
test := test
126131
t.Run(test.name, func(t *testing.T) {
132+
t.Parallel()
127133
reader := NewHeaderReader(test.header)
128134
err := reader.ForEachKey(func(key string, val []byte) error {
129135
if _, ok := test.keys[key]; !ok {

0 commit comments

Comments
 (0)