Cleanup pkg/jsonmessage progress tests

Signed-off-by: Daniel Nephin <dnephin@docker.com>

pkg/jsonmessage/jsonmessage.go

@@ -40,21 +40,17 @@ type JSONProgress struct {
 	// If true, don't show xB/yB
 	HideCounts bool   `json:"hidecounts,omitempty"`
 	Units      string `json:"units,omitempty"`
+	nowFunc    func() time.Time
+	winSize    int
 }
 
 func (p *JSONProgress) String() string {
 	var (
-		width       = 200
+		width       = p.width()
 		pbBox       string
 		numbersBox  string
 		timeLeftBox string
 	)
-
-	ws, err := term.GetWinsize(p.terminalFd)
-	if err == nil {
-		width = int(ws.Width)
-	}
-
 	if p.Current <= 0 && p.Total <= 0 {
 		return ""
 	}
@@ -103,7 +99,7 @@ func (p *JSONProgress) String() string {
 	}
 
 	if p.Current > 0 && p.Start > 0 && percentage < 50 {
-		fromStart := time.Now().UTC().Sub(time.Unix(p.Start, 0))
+		fromStart := p.now().Sub(time.Unix(p.Start, 0))
 		perEntry := fromStart / time.Duration(p.Current)
 		left := time.Duration(p.Total-p.Current) * perEntry
 		left = (left / time.Second) * time.Second
@@ -115,6 +111,28 @@ func (p *JSONProgress) String() string {
 	return pbBox + numbersBox + timeLeftBox
 }
 
+// shim for testing
+func (p *JSONProgress) now() time.Time {
+	if p.nowFunc == nil {
+		p.nowFunc = func() time.Time {
+			return time.Now().UTC()
+		}
+	}
+	return p.nowFunc()
+}
+
+// shim for testing
+func (p *JSONProgress) width() int {
+	if p.winSize != 0 {
+		return p.winSize
+	}
+	ws, err := term.GetWinsize(p.terminalFd)
+	if err == nil {
+		return int(ws.Width)
+	}
+	return 200
+}
+
 // JSONMessage defines a message struct. It describes
 // the created time, where it from, status, ID of the
 // message. It's used for docker events.

pkg/jsonmessage/jsonmessage_test.go

@@ -15,84 +15,102 @@ import (
 
 func TestError(t *testing.T) {
 	je := JSONError{404, "Not found"}
-	if je.Error() != "Not found" {
+	assert.Assert(t, is.Error(&je, "Not found"))
-		t.Fatalf("Expected 'Not found' got '%s'", je.Error())
-	}
 }
 
-func TestProgress(t *testing.T) {
+func TestProgressString(t *testing.T) {
-	termsz, err := term.GetWinsize(0)
+	type expected struct {
-	if err != nil {
+		short string
-		// we can safely ignore the err here
+		long  string
-		termsz = nil
-	}
-	jp := JSONProgress{}
-	if jp.String() != "" {
-		t.Fatalf("Expected empty string, got '%s'", jp.String())
 	}
 
-	expected := "      1B"
+	shortAndLong := func(short, long string) expected {
-	jp2 := JSONProgress{Current: 1}
+		return expected{short: short, long: long}
-	if jp2.String() != expected {
-		t.Fatalf("Expected %q, got %q", expected, jp2.String())
 	}
 
-	expectedStart := "[==========>                                        ]      20B/100B"
+	start := time.Date(2017, 12, 3, 15, 10, 1, 0, time.UTC)
-	if termsz != nil && termsz.Width <= 110 {
+	timeAfter := func(delta time.Duration) func() time.Time {
-		expectedStart = "    20B/100B"
+		return func() time.Time {
+			return start.Add(delta)
 		}
-	jp3 := JSONProgress{Current: 20, Total: 100, Start: time.Now().Unix()}
-	// Just look at the start of the string
-	// (the remaining time is really hard to test -_-)
-	if jp3.String()[:len(expectedStart)] != expectedStart {
-		t.Fatalf("Expected to start with %q, got %q", expectedStart, jp3.String())
 	}
 
-	expected = "[=========================>                         ]      50B/100B"
+	var testcases = []struct {
-	if termsz != nil && termsz.Width <= 110 {
+		name     string
-		expected = "    50B/100B"
+		progress JSONProgress
-	}
+		expected expected
-	jp4 := JSONProgress{Current: 50, Total: 100}
+	}{
-	if jp4.String() != expected {
+		{
-		t.Fatalf("Expected %q, got %q", expected, jp4.String())
+			name: "no progress",
+		},
+		{
+			name:     "progress 1",
+			progress: JSONProgress{Current: 1},
+			expected: shortAndLong("      1B", "      1B"),
+		},
+		{
+			name: "some progress with a start time",
+			progress: JSONProgress{
+				Current: 20,
+				Total:   100,
+				Start:   start.Unix(),
+				nowFunc: timeAfter(time.Second),
+			},
+			expected: shortAndLong(
+				"     20B/100B 4s",
+				"[==========>                                        ]      20B/100B 4s",
+			),
+		},
+		{
+			name:     "some progress without a start time",
+			progress: JSONProgress{Current: 50, Total: 100},
+			expected: shortAndLong(
+				"     50B/100B",
+				"[=========================>                         ]      50B/100B",
+			),
+		},
+		{
+			name:     "current more than total is not negative gh#7136",
+			progress: JSONProgress{Current: 50, Total: 40},
+			expected: shortAndLong(
+				"     50B",
+				"[==================================================>]      50B",
+			),
+		},
+		{
+			name:     "with units",
+			progress: JSONProgress{Current: 50, Total: 100, Units: "units"},
+			expected: shortAndLong(
+				"50/100 units",
+				"[=========================>                         ] 50/100 units",
+			),
+		},
+		{
+			name:     "current more than total with units is not negative ",
+			progress: JSONProgress{Current: 50, Total: 40, Units: "units"},
+			expected: shortAndLong(
+				"50 units",
+				"[==================================================>] 50 units",
+			),
+		},
+		{
+			name:     "hide counts",
+			progress: JSONProgress{Current: 50, Total: 100, HideCounts: true},
+			expected: shortAndLong(
+				"",
+				"[=========================>                         ] ",
+			),
+		},
 	}
 
-	// this number can't be negative gh#7136
+	for _, testcase := range testcases {
-	expected = "[==================================================>]      50B"
+		t.Run(testcase.name, func(t *testing.T) {
-	if termsz != nil && termsz.Width <= 110 {
+			testcase.progress.winSize = 100
-		expected = "    50B"
+			assert.Equal(t, testcase.progress.String(), testcase.expected.short)
-	}
-	jp5 := JSONProgress{Current: 50, Total: 40}
-	if jp5.String() != expected {
-		t.Fatalf("Expected %q, got %q", expected, jp5.String())
-	}
 
-	expected = "[=========================>                         ] 50/100 units"
+			testcase.progress.winSize = 200
-	if termsz != nil && termsz.Width <= 110 {
+			assert.Equal(t, testcase.progress.String(), testcase.expected.long)
-		expected = "    50/100 units"
+		})
-	}
-	jp6 := JSONProgress{Current: 50, Total: 100, Units: "units"}
-	if jp6.String() != expected {
-		t.Fatalf("Expected %q, got %q", expected, jp6.String())
-	}
-
-	// this number can't be negative
-	expected = "[==================================================>] 50 units"
-	if termsz != nil && termsz.Width <= 110 {
-		expected = "    50 units"
-	}
-	jp7 := JSONProgress{Current: 50, Total: 40, Units: "units"}
-	if jp7.String() != expected {
-		t.Fatalf("Expected %q, got %q", expected, jp7.String())
-	}
-
-	expected = "[=========================>                         ] "
-	if termsz != nil && termsz.Width <= 110 {
-		expected = ""
-	}
-	jp8 := JSONProgress{Current: 50, Total: 100, HideCounts: true}
-	if jp8.String() != expected {
-		t.Fatalf("Expected %q, got %q", expected, jp8.String())
 	}
 }
 

pkg/streamformatter/streamformatter_test.go

@@ -8,6 +8,8 @@ import (
 	"testing"
 
 	"github.com/docker/docker/pkg/jsonmessage"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/gotestyourself/gotestyourself/assert"
 	is "github.com/gotestyourself/gotestyourself/assert/cmp"
 )
@@ -58,30 +60,31 @@ func TestJsonProgressFormatterFormatProgress(t *testing.T) {
 		Total:   30,
 		Start:   1,
 	}
-	res := sf.formatProgress("id", "action", jsonProgress, &AuxFormatter{Writer: &bytes.Buffer{}})
+	aux := "aux message"
+	res := sf.formatProgress("id", "action", jsonProgress, aux)
 	msg := &jsonmessage.JSONMessage{}
 
 	assert.NilError(t, json.Unmarshal(res, msg))
-	assert.Check(t, is.Equal("id", msg.ID))
-	assert.Check(t, is.Equal("action", msg.Status))
 
-	// jsonProgress will always be in the format of:
+	rawAux := json.RawMessage(`"` + aux + `"`)
-	// [=========================>                         ]      15B/30B 412910h51m30s
+	expected := &jsonmessage.JSONMessage{
-	// The last entry '404933h7m11s' is the timeLeftBox.
+		ID:       "id",
-	// However, the timeLeftBox field may change as jsonProgress.String() depends on time.Now().
+		Status:   "action",
-	// Therefore, we have to strip the timeLeftBox from the strings to do the comparison.
+		Aux:      &rawAux,
-
+		Progress: jsonProgress,
-	// Compare the jsonProgress strings before the timeLeftBox
-	expectedProgress := "[=========================>                         ]      15B/30B"
-	// if terminal column is <= 110, expectedProgressShort is expected.
-	expectedProgressShort := "      15B/30B"
-	if !(strings.HasPrefix(msg.ProgressMessage, expectedProgress) ||
-		strings.HasPrefix(msg.ProgressMessage, expectedProgressShort)) {
-		t.Fatalf("ProgressMessage without the timeLeftBox must be %s or %s, got: %s",
-			expectedProgress, expectedProgressShort, msg.ProgressMessage)
 	}
+	assert.DeepEqual(t, msg, expected, cmpJSONMessageOpt())
+}
 
-	assert.Check(t, is.DeepEqual(jsonProgress, msg.Progress))
+func cmpJSONMessageOpt() cmp.Option {
+	progressMessagePath := func(path cmp.Path) bool {
+		return path.String() == "ProgressMessage"
+	}
+	return cmp.Options{
+		cmpopts.IgnoreUnexported(jsonmessage.JSONProgress{}),
+		// Ignore deprecated property that is a derivative of Progress
+		cmp.FilterPath(progressMessagePath, cmp.Ignore()),
+	}
 }
 
 func TestJsonProgressFormatterFormatStatus(t *testing.T) {