Browse code

Update TestUpdatePidsLimit to be more atomic

Create a new container for each subtest, so that individual
subtests are self-contained, and there's no need to execute
them in the exact order, or resetting the container in between.

This makes the test slower (6.54s vs 3.43s), but reduced the
difference by using `network=host`, which made a substantial
difference (without `network=host`, the test took more than
twice as long: 13.96s).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2019/02/25 00:26:37
Showing 2 changed files
... ...
@@ -7,7 +7,6 @@ import (
7 7
 	"testing"
8 8
 	"time"
9 9
 
10
-	"github.com/docker/docker/api/types"
11 10
 	containertypes "github.com/docker/docker/api/types/container"
12 11
 	"github.com/docker/docker/client"
13 12
 	"github.com/docker/docker/integration/internal/container"
... ...
@@ -114,8 +113,6 @@ func TestUpdatePidsLimit(t *testing.T) {
114 114
 	oldAPIclient := request.NewAPIClient(t, client.WithVersion("1.24"))
115 115
 	ctx := context.Background()
116 116
 
117
-	cID := container.Run(t, ctx, apiClient)
118
-
119 117
 	intPtr := func(i int64) *int64 {
120 118
 		return &i
121 119
 	}
... ...
@@ -123,33 +120,28 @@ func TestUpdatePidsLimit(t *testing.T) {
123 123
 	for _, test := range []struct {
124 124
 		desc     string
125 125
 		oldAPI   bool
126
+		initial  *int64
126 127
 		update   *int64
127 128
 		expect   int64
128 129
 		expectCg string
129 130
 	}{
130 131
 		{desc: "update from none", update: intPtr(32), expect: 32, expectCg: "32"},
131
-		{desc: "no change", update: nil, expectCg: "32"},
132
-		{desc: "update lower", update: intPtr(16), expect: 16, expectCg: "16"},
133
-		{desc: "update on old api ignores value", oldAPI: true, update: intPtr(10), expect: 16, expectCg: "16"},
134
-		{desc: "unset limit", update: intPtr(0), expect: 0, expectCg: "max"},
135
-		{desc: "reset", update: intPtr(32), expect: 32, expectCg: "32"},
136
-		{desc: "unset limit with minus one", update: intPtr(-1), expect: 0, expectCg: "max"},
137
-		{desc: "reset", update: intPtr(32), expect: 32, expectCg: "32"},
138
-		{desc: "unset limit with minus two", update: intPtr(-2), expect: 0, expectCg: "max"},
132
+		{desc: "no change", initial: intPtr(32), expect: 32, expectCg: "32"},
133
+		{desc: "update lower", initial: intPtr(32), update: intPtr(16), expect: 16, expectCg: "16"},
134
+		{desc: "update on old api ignores value", oldAPI: true, initial: intPtr(32), update: intPtr(16), expect: 32, expectCg: "32"},
135
+		{desc: "unset limit with zero", initial: intPtr(32), update: intPtr(0), expect: 0, expectCg: "max"},
136
+		{desc: "unset limit with minus one", initial: intPtr(32), update: intPtr(-1), expect: 0, expectCg: "max"},
137
+		{desc: "unset limit with minus two", initial: intPtr(32), update: intPtr(-2), expect: 0, expectCg: "max"},
139 138
 	} {
140 139
 		c := apiClient
141 140
 		if test.oldAPI {
142 141
 			c = oldAPIclient
143 142
 		}
144 143
 
145
-		var before types.ContainerJSON
146
-		if test.update == nil {
147
-			var err error
148
-			before, err = c.ContainerInspect(ctx, cID)
149
-			assert.NilError(t, err)
150
-		}
151
-
152 144
 		t.Run(test.desc, func(t *testing.T) {
145
+			// Using "network=host" to speed up creation (13.96s vs 6.54s)
146
+			cID := container.Run(t, ctx, apiClient, container.WithPidsLimit(test.initial), container.WithNetworkMode("host"))
147
+
153 148
 			_, err := c.ContainerUpdate(ctx, cID, containertypes.UpdateConfig{
154 149
 				Resources: containertypes.Resources{
155 150
 					PidsLimit: test.update,
... ...
@@ -160,13 +152,7 @@ func TestUpdatePidsLimit(t *testing.T) {
160 160
 			inspect, err := c.ContainerInspect(ctx, cID)
161 161
 			assert.NilError(t, err)
162 162
 			assert.Assert(t, inspect.HostConfig.Resources.PidsLimit != nil)
163
-
164
-			if test.update == nil {
165
-				assert.Assert(t, before.HostConfig.Resources.PidsLimit != nil)
166
-				assert.Equal(t, *before.HostConfig.Resources.PidsLimit, *inspect.HostConfig.Resources.PidsLimit)
167
-			} else {
168
-				assert.Equal(t, *inspect.HostConfig.Resources.PidsLimit, test.expect)
169
-			}
163
+			assert.Equal(t, *inspect.HostConfig.Resources.PidsLimit, test.expect)
170 164
 
171 165
 			ctx, cancel := context.WithTimeout(ctx, 60*time.Second)
172 166
 			defer cancel()
... ...
@@ -137,6 +137,16 @@ func WithAutoRemove(c *TestContainerConfig) {
137 137
 	c.HostConfig.AutoRemove = true
138 138
 }
139 139
 
140
+// WithPidsLimit sets the container's "pids-limit
141
+func WithPidsLimit(limit *int64) func(*TestContainerConfig) {
142
+	return func(c *TestContainerConfig) {
143
+		if c.HostConfig == nil {
144
+			c.HostConfig = &containertypes.HostConfig{}
145
+		}
146
+		c.HostConfig.PidsLimit = limit
147
+	}
148
+}
149
+
140 150
 // WithRestartPolicy sets container's restart policy
141 151
 func WithRestartPolicy(policy string) func(c *TestContainerConfig) {
142 152
 	return func(c *TestContainerConfig) {