Skip to content

Commit 7154375

Browse files
phinzephinze
committed
command: fix flaky parallelism tests
Thanks to @mitchellh for helping sort out concurrency stuff here.
1 parent c56245c commit 7154375

4 files changed

Lines changed: 55 additions & 71 deletions

File tree

‎command/apply_test.go‎

Lines changed: 37 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"reflect"
1313
"strings"
1414
"sync"
15+
"sync/atomic"
1516
"testing"
1617
"time"
1718

@@ -58,80 +59,60 @@ func TestApply(t *testing.T) {
5859
}
5960
}
6061

61-
func TestApply_parallelism1(t *testing.T) {
62-
statePath := testTempFile(t)
62+
func TestApply_parallelism(t *testing.T) {
63+
provider := testProvider()
6364

64-
ui := new(cli.MockUi)
65-
p := testProvider()
66-
pr := new(terraform.MockResourceProvisioner)
65+
// This blocks all the appy functions. We close it when we exit so
66+
// they end quickly after this test finishes.
67+
block := make(chan struct{})
68+
defer close(block)
6769

68-
pr.ApplyFn = func(*terraform.InstanceState, *terraform.ResourceConfig) error {
69-
time.Sleep(time.Second)
70-
return nil
71-
}
70+
var runCount uint64
71+
provider.ApplyFn = func(
72+
i *terraform.InstanceInfo,
73+
s *terraform.InstanceState,
74+
d *terraform.InstanceDiff) (*terraform.InstanceState, error) {
75+
// Increment so we're counting parallelism
76+
atomic.AddUint64(&runCount, 1)
7277

73-
args := []string{
74-
"-state", statePath,
75-
"-parallelism=1",
76-
testFixturePath("parallelism"),
78+
// Block until we're done
79+
<-block
80+
81+
return nil, nil
7782
}
7883

84+
ui := new(cli.MockUi)
7985
c := &ApplyCommand{
8086
Meta: Meta{
81-
ContextOpts: testCtxConfigWithShell(p, pr),
87+
ContextOpts: testCtxConfig(provider),
8288
Ui: ui,
8389
},
8490
}
8591

86-
start := time.Now()
87-
if code := c.Run(args); code != 0 {
88-
t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
89-
}
90-
elapsed := time.Since(start).Seconds()
91-
92-
// This test should take exactly two seconds, plus some minor amount of execution time.
93-
if elapsed < 2 || elapsed > 2.2 {
94-
t.Fatalf("bad: %f\n\n%s", elapsed, ui.ErrorWriter.String())
95-
}
96-
97-
}
98-
99-
func TestApply_parallelism2(t *testing.T) {
100-
statePath := testTempFile(t)
101-
102-
ui := new(cli.MockUi)
103-
p := testProvider()
104-
pr := new(terraform.MockResourceProvisioner)
105-
106-
pr.ApplyFn = func(*terraform.InstanceState, *terraform.ResourceConfig) error {
107-
time.Sleep(time.Second)
108-
return nil
109-
}
110-
92+
par := uint64(5)
11193
args := []string{
112-
"-state", statePath,
113-
"-parallelism=2",
94+
fmt.Sprintf("-parallelism=%d", par),
11495
testFixturePath("parallelism"),
11596
}
11697

117-
c := &ApplyCommand{
118-
Meta: Meta{
119-
ContextOpts: testCtxConfigWithShell(p, pr),
120-
Ui: ui,
121-
},
122-
}
123-
124-
start := time.Now()
125-
if code := c.Run(args); code != 0 {
126-
t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
98+
// Run in a goroutine. We still try to catch any errors and
99+
// get them on the error channel.
100+
errCh := make(chan string, 1)
101+
go func() {
102+
if code := c.Run(args); code != 0 {
103+
errCh <- ui.OutputWriter.String()
104+
}
105+
}()
106+
select {
107+
case <-time.After(1000 * time.Millisecond):
108+
case err := <-errCh:
109+
t.Fatalf("err: %s", err)
127110
}
128-
elapsed := time.Since(start).Seconds()
129111

130-
// This test should take exactly one second, plus some minor amount of execution time.
131-
if elapsed < 1 || elapsed > 1.2 {
132-
t.Fatalf("bad: %f\n\n%s", elapsed, ui.ErrorWriter.String())
112+
// The total in flight should equal the parallelism
113+
if rc := atomic.LoadUint64(&runCount); rc != par {
114+
t.Fatalf("Expected parallelism: %d, got: %d", par, rc)
133115
}
134-
135116
}
136117

137118
func TestApply_configInvalid(t *testing.T) {

‎command/test-fixtures/parallelism/main.tf‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
1-
resource "test_instance" "foo1" {
2-
ami = "bar"
3-
4-
// shell has been configured to sleep for one second
5-
provisioner "shell" {}
6-
}
7-
8-
resource "test_instance" "foo2" {
9-
ami = "bar"
10-
11-
// shell has been configured to sleep for one second
12-
provisioner "shell" {}
13-
}
1+
resource "test_instance" "foo1" {}
2+
resource "test_instance" "foo2" {}
3+
resource "test_instance" "foo3" {}
4+
resource "test_instance" "foo4" {}
5+
resource "test_instance" "foo5" {}
6+
resource "test_instance" "foo6" {}
7+
resource "test_instance" "foo7" {}
8+
resource "test_instance" "foo8" {}
9+
resource "test_instance" "foo9" {}
10+
resource "test_instance" "foo10" {}

‎terraform/graph_walk_context.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package terraform
22

33
import (
44
"fmt"
5+
"log"
56
"sync"
67

78
"github.com/hashicorp/errwrap"
@@ -95,6 +96,8 @@ func (w *ContextGraphWalker) EnterPath(path []string) EvalContext {
9596
}
9697

9798
func (w *ContextGraphWalker) EnterEvalTree(v dag.Vertex, n EvalNode) EvalNode {
99+
log.Printf("[INFO] Entering eval tree: %s", dag.VertexName(v))
100+
98101
// Acquire a lock on the semaphore
99102
w.Context.parallelSem.Acquire()
100103

@@ -105,6 +108,8 @@ func (w *ContextGraphWalker) EnterEvalTree(v dag.Vertex, n EvalNode) EvalNode {
105108

106109
func (w *ContextGraphWalker) ExitEvalTree(
107110
v dag.Vertex, output interface{}, err error) error {
111+
log.Printf("[INFO] Exiting eval tree: %s", dag.VertexName(v))
112+
108113
// Release the semaphore
109114
w.Context.parallelSem.Release()
110115

‎terraform/resource_provider_mock.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,14 @@ func (p *MockResourceProvider) Apply(
118118
info *InstanceInfo,
119119
state *InstanceState,
120120
diff *InstanceDiff) (*InstanceState, error) {
121+
// We only lock while writing data. Reading is fine
121122
p.Lock()
122-
defer p.Unlock()
123-
124123
p.ApplyCalled = true
125124
p.ApplyInfo = info
126125
p.ApplyState = state
127126
p.ApplyDiff = diff
127+
p.Unlock()
128+
128129
if p.ApplyFn != nil {
129130
return p.ApplyFn(info, state, diff)
130131
}

0 commit comments

Comments
 (0)