Skip to content

Commit f6b8ece

Browse files
committed
address comments
1 parent 15f442a commit f6b8ece

File tree

4 files changed

+11
-60
lines changed

4 files changed

+11
-60
lines changed

internal/batch/batch_future.go

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,8 @@ import (
99
"go.uber.org/cadence/internal"
1010
)
1111

12-
type BatchFuture interface {
13-
internal.Future
14-
GetFutures() []internal.Future
15-
}
16-
17-
type batchFutureImpl struct {
12+
// BatchFutureImpl is an implementation of BatchFuture
13+
type BatchFutureImpl struct {
1814
futures []internal.Future
1915
settables []internal.Settable
2016
factories []func(ctx internal.Context) internal.Future
@@ -24,7 +20,7 @@ type batchFutureImpl struct {
2420
wg internal.WaitGroup
2521
}
2622

27-
func NewBatchFuture(ctx internal.Context, batchSize int, factories []func(ctx internal.Context) internal.Future) (BatchFuture, error) {
23+
func NewBatchFuture(ctx internal.Context, batchSize int, factories []func(ctx internal.Context) internal.Future) (*BatchFutureImpl, error) {
2824
var futures []internal.Future
2925
var settables []internal.Settable
3026
for range factories {
@@ -33,7 +29,7 @@ func NewBatchFuture(ctx internal.Context, batchSize int, factories []func(ctx in
3329
settables = append(settables, settable)
3430
}
3531

36-
batchFuture := &batchFutureImpl{
32+
batchFuture := &BatchFutureImpl{
3733
futures: futures,
3834
settables: settables,
3935
factories: factories,
@@ -45,11 +41,11 @@ func NewBatchFuture(ctx internal.Context, batchSize int, factories []func(ctx in
4541
return batchFuture, nil
4642
}
4743

48-
func (b *batchFutureImpl) GetFutures() []internal.Future {
44+
func (b *BatchFutureImpl) GetFutures() []internal.Future {
4945
return b.futures
5046
}
5147

52-
func (b *batchFutureImpl) start(ctx internal.Context) {
48+
func (b *BatchFutureImpl) start(ctx internal.Context) {
5349

5450
buffered := internal.NewBufferedChannel(ctx, b.batchSize) // buffered channel to limit the number of concurrent futures
5551
channel := internal.NewNamedChannel(ctx, "batch-future-channel")
@@ -91,7 +87,7 @@ func (b *batchFutureImpl) start(ctx internal.Context) {
9187
})
9288
}
9389

94-
func (b *batchFutureImpl) IsReady() bool {
90+
func (b *BatchFutureImpl) IsReady() bool {
9591
for _, future := range b.futures {
9692
if !future.IsReady() {
9793
return false
@@ -105,7 +101,7 @@ func (b *batchFutureImpl) IsReady() bool {
105101
// If valuePtr is a pointer to a slice, the slice will be resized to the length of the futures. Each element of the slice will be assigned with the underlying Future.Get() and thus behaves the same way.
106102
// If valuePtr is nil, no assignment will be made.
107103
// If error occurs, values will be set on successful futures and the errors of failed futures will be returned.
108-
func (b *batchFutureImpl) Get(ctx internal.Context, valuePtr interface{}) error {
104+
func (b *BatchFutureImpl) Get(ctx internal.Context, valuePtr interface{}) error {
109105
// No assignment if valuePtr is nil
110106
if valuePtr == nil {
111107
b.wg.Wait(ctx)

internal/batch/batch_future_test.go

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -174,42 +174,6 @@ func Test_BatchWorkflowUsingFutures(t *testing.T) {
174174
assert.Equal(t, expected, result)
175175
}
176176

177-
func futureTest(ctx internal.Context) error {
178-
f, s := internal.NewFuture(ctx)
179-
f2, s2 := internal.NewFuture(ctx)
180-
s2.Chain(f)
181-
182-
wg := internal.NewWaitGroup(ctx)
183-
wg.Add(1)
184-
internal.GoNamed(ctx, "future-test", func(ctx internal.Context) {
185-
defer wg.Done()
186-
internal.Sleep(ctx, time.Second*10)
187-
s.Set(1, nil)
188-
})
189-
190-
err := f2.Get(ctx, nil)
191-
if err != nil {
192-
return err
193-
}
194-
195-
err = f.Get(ctx, nil)
196-
if err != nil {
197-
return err
198-
}
199-
200-
wg.Wait(ctx)
201-
return err
202-
}
203-
204-
func Test_Futures(t *testing.T) {
205-
testSuite := &testsuite.WorkflowTestSuite{}
206-
env := testSuite.NewTestWorkflowEnvironment()
207-
208-
env.RegisterWorkflow(futureTest)
209-
210-
env.ExecuteWorkflow(futureTest)
211-
}
212-
213177
func batchWorkflowAssignWithSlice(ctx internal.Context) ([]int, error) {
214178
totalSize := 5
215179
concurrency := 2
@@ -287,11 +251,10 @@ func batchWorkflowAssignWithNil(ctx internal.Context) ([]int, error) {
287251
return nil, err
288252
}
289253

290-
var valuePtr []int
291254
if err := batchFuture.Get(ctx, nil); err != nil {
292255
return nil, err
293256
}
294-
return valuePtr, nil
257+
return nil, nil
295258
}
296259

297260
func Test_BatchFuture_Get(t *testing.T) {

internal/workflow_testsuite.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -601,12 +601,7 @@ func (t *TestWorkflowEnvironment) GetWorkflowResult(valuePtr interface{}) error
601601
if !t.impl.isTestCompleted {
602602
panic("workflow is not completed")
603603
}
604-
// fast lane for no result assignment
605-
if t.impl.testResult == nil || t.impl.testResult.HasValue() == false || valuePtr == nil {
606-
return t.impl.testError
607-
}
608-
if t.impl.testError != nil {
609-
t.impl.testResult.Get(valuePtr)
604+
if t.impl.testError != nil || t.impl.testResult == nil || t.impl.testResult.HasValue() == false || valuePtr == nil {
610605
return t.impl.testError
611606
}
612607
return t.impl.testResult.Get(valuePtr)

x/doc.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,2 @@
1+
// Package x is an experimental package for early-stage features. The API here is not stable and may change.
12
package x
2-
3-
/*
4-
Package x is an experimental package for early-stage features. The API here is not stable and may change.
5-
*/

0 commit comments

Comments
 (0)