1. docker
  2. Commit 33c205be4fa8860280aea70f301f7c394c0983c1
Browse code

TestTransfer*: don't call t.Fatal from a goroutine

staticcheck go linter warns:

> distribution/xfer/transfer_test.go:37:2: SA2002: the goroutine calls T.Fatalf, which must be called in the same goroutine as the test (staticcheck)

What it doesn't say is why. The reason is, t.Fatalf() calls t.FailNow(),
which is expected to stop test execution right now. It does so by
calling runtime.Goexit(), which, unless called from a main goroutine,
does not stop test execution.

Anyway, long story short, if we don't care much about stopping the test
case immediately, we can just replace t.Fatalf() with t.Errorf() which
still marks the test case as failed, but won't stop it immediately.

This patch was tested to check that the test fails if any of the
goroutines call t.Errorf():

1. Failure in DoFunc ("transfer function not started ...") was tested by
decreading the NewTransferManager() argument:

- tm := NewTransferManager(5)
+ tm := NewTransferManager(2)

2. Failure "got unexpected progress value" was tested by injecting a random:

- if present && p.Current <= val {
+ if present && p.Current <= val || rand.Intn(100) > 80 {

3. Failure in DoFunc ("too many jobs running") was tested by increasing
the NewTransferManager() argument:

- tm := NewTransferManager(concurrencyLimit)
+ tm := NewTransferManager(concurrencyLimit + 1)

While at it:
* fix/amend some error messages
* use _ for unused arguments of DoFunc

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Kir Kolyshkin authored on 2019/08/07 07:37:07
Showing 1 changed files
distribution/xfer/transfer_test.go
History View file @ 33c205b
... ... 
@@ -10,11 +10,11 @@ import (
10 10
11 11 
 func TestTransfer(t *testing.T) {
12 12 
 	makeXferFunc := func(id string) DoFunc {
13 
-		return func(progressChan chan<- progress.Progress, start <-chan struct{}, inactive chan<- struct{}) Transfer {
13 
+		return func(progressChan chan<- progress.Progress, start <-chan struct{}, _ chan<- struct{}) Transfer {
14 14 
 			select {
15 15 
 			case <-start:
16 16 
 			default:
17 
-				t.Fatalf("transfer function not started even though concurrency limit not reached")
17 
+				t.Errorf("%s: transfer function not started even though concurrency limit not reached", id)
18 18 
 			}
19 19
20 20 
 			xfer := NewTransfer()
... ... 
@@ -38,7 +38,7 @@ func TestTransfer(t *testing.T) {
38 38 
 		for p := range progressChan {
39 39 
 			val, present := receivedProgress[p.ID]
40 40 
 			if present && p.Current <= val {
41 
-				t.Fatalf("got unexpected progress value: %d (expected %d)", p.Current, val+1)
41 
+				t.Errorf("%s: got unexpected progress value: %d (expected <= %d)", p.ID, p.Current, val)
42 42 
 			}
43 43 
 			receivedProgress[p.ID] = p.Current
44 44 
 		}
... ... 
@@ -72,13 +72,13 @@ func TestConcurrencyLimit(t *testing.T) {
72 72 
 	var runningJobs int32
73 73
74 74 
 	makeXferFunc := func(id string) DoFunc {
75 
-		return func(progressChan chan<- progress.Progress, start <-chan struct{}, inactive chan<- struct{}) Transfer {
75 
+		return func(progressChan chan<- progress.Progress, start <-chan struct{}, _ chan<- struct{}) Transfer {
76 76 
 			xfer := NewTransfer()
77 77 
 			go func() {
78 78 
 				<-start
79 79 
 				totalJobs := atomic.AddInt32(&runningJobs, 1)
80 80 
 				if int(totalJobs) > concurrencyLimit {
81 
-					t.Fatalf("too many jobs running")
81 
+					t.Errorf("%s: too many jobs running (%d > %d)", id, totalJobs, concurrencyLimit)
82 82 
 				}
83 83 
 				for i := 0; i <= 10; i++ {
84 84 
 					progressChan <- progress.Progress{ID: id, Action: "testing", Current: int64(i), Total: 10}
... ... 
@@ -137,7 +137,7 @@ func TestInactiveJobs(t *testing.T) {
137 137 
 				<-start
138 138 
 				totalJobs := atomic.AddInt32(&runningJobs, 1)
139 139 
 				if int(totalJobs) > concurrencyLimit {
140 
-					t.Fatalf("too many jobs running")
140 
+					t.Errorf("%s: too many jobs running (%d > %d)", id, totalJobs, concurrencyLimit)
141 141 
 				}
142 142 
 				for i := 0; i <= 10; i++ {
143 143 
 					progressChan <- progress.Progress{ID: id, Action: "testing", Current: int64(i), Total: 10}
... ... 
@@ -191,7 +191,7 @@ func TestWatchRelease(t *testing.T) {
191 191 
 	ready := make(chan struct{})
192 192
193 193 
 	makeXferFunc := func(id string) DoFunc {
194 
-		return func(progressChan chan<- progress.Progress, start <-chan struct{}, inactive chan<- struct{}) Transfer {
194 
+		return func(progressChan chan<- progress.Progress, start <-chan struct{}, _ chan<- struct{}) Transfer {
195 195 
 			xfer := NewTransfer()
196 196 
 			go func() {
197 197 
 				defer func() {
... ... 
@@ -280,7 +280,7 @@ func TestWatchRelease(t *testing.T) {
280 280
281 281 
 func TestWatchFinishedTransfer(t *testing.T) {
282 282 
 	makeXferFunc := func(id string) DoFunc {
283 
-		return func(progressChan chan<- progress.Progress, start <-chan struct{}, inactive chan<- struct{}) Transfer {
283 
+		return func(progressChan chan<- progress.Progress, _ <-chan struct{}, _ chan<- struct{}) Transfer {
284 284 
 			xfer := NewTransfer()
285 285 
 			go func() {
286 286 
 				// Finish immediately
... ... 
@@ -322,7 +322,7 @@ func TestDuplicateTransfer(t *testing.T) {
322 322 
 	var xferFuncCalls int32
323 323
324 324 
 	makeXferFunc := func(id string) DoFunc {
325 
-		return func(progressChan chan<- progress.Progress, start <-chan struct{}, inactive chan<- struct{}) Transfer {
325 
+		return func(progressChan chan<- progress.Progress, _ <-chan struct{}, _ chan<- struct{}) Transfer {
326 326 
 			atomic.AddInt32(&xferFuncCalls, 1)
327 327 
 			xfer := NewTransfer()
328 328 
 			go func() {