Merge pull request #476 from dnephin/small-cleanup-to-two-formatters

Reduce complexity of image formatter

cli/command/formatter/custom_test.go

@@ -1,9 +1,10 @@
 package formatter
 
 import (
-	"reflect"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func compareMultipleValues(t *testing.T, value, expected string) {
@@ -22,7 +23,5 @@ func compareMultipleValues(t *testing.T, value, expected string) {
 		keyval := strings.Split(expected, "=")
 		expMap[keyval[0]] = keyval[1]
 	}
-	if !reflect.DeepEqual(expMap, entriesMap) {
+	assert.Equal(t, expMap, entriesMap)
-		t.Fatalf("Expected entries: %v, got: %v", expected, value)
-	}
 }

cli/command/formatter/image.go

@@ -86,9 +86,9 @@ func needDigest(ctx ImageContext) bool {
 
 func imageFormat(ctx ImageContext, images []types.ImageSummary, format func(subContext subContext) error) error {
 	for _, image := range images {
-		images := []*imageContext{}
+		formatted := []*imageContext{}
 		if isDangling(image) {
-			images = append(images, &imageContext{
+			formatted = append(formatted, &imageContext{
 				trunc:  ctx.Trunc,
 				i:      image,
 				repo:   "<none>",
@@ -96,90 +96,9 @@ func imageFormat(ctx ImageContext, images []types.ImageSummary, format func(subC
 				digest: "<none>",
 			})
 		} else {
-			repoTags := map[string][]string{}
+			formatted = imageFormatTaggedAndDigest(ctx, image)
-			repoDigests := map[string][]string{}
-
-			for _, refString := range image.RepoTags {
-				ref, err := reference.ParseNormalizedNamed(refString)
-				if err != nil {
-					continue
-				}
-				if nt, ok := ref.(reference.NamedTagged); ok {
-					familiarRef := reference.FamiliarName(ref)
-					repoTags[familiarRef] = append(repoTags[familiarRef], nt.Tag())
-				}
-			}
-			for _, refString := range image.RepoDigests {
-				ref, err := reference.ParseNormalizedNamed(refString)
-				if err != nil {
-					continue
-				}
-				if c, ok := ref.(reference.Canonical); ok {
-					familiarRef := reference.FamiliarName(ref)
-					repoDigests[familiarRef] = append(repoDigests[familiarRef], c.Digest().String())
-				}
-			}
-
-			for repo, tags := range repoTags {
-				digests := repoDigests[repo]
-
-				// Do not display digests as their own row
-				delete(repoDigests, repo)
-
-				if !needDigest(ctx) {
-					// Ignore digest references, just show tag once
-					digests = nil
-				}
-
-				for _, tag := range tags {
-					if len(digests) == 0 {
-						images = append(images, &imageContext{
-							trunc:  ctx.Trunc,
-							i:      image,
-							repo:   repo,
-							tag:    tag,
-							digest: "<none>",
-						})
-						continue
-					}
-					// Display the digests for each tag
-					for _, dgst := range digests {
-						images = append(images, &imageContext{
-							trunc:  ctx.Trunc,
-							i:      image,
-							repo:   repo,
-							tag:    tag,
-							digest: dgst,
-						})
-					}
-
-				}
-			}
-
-			// Show rows for remaining digest only references
-			for repo, digests := range repoDigests {
-				// If digests are displayed, show row per digest
-				if ctx.Digest {
-					for _, dgst := range digests {
-						images = append(images, &imageContext{
-							trunc:  ctx.Trunc,
-							i:      image,
-							repo:   repo,
-							tag:    "<none>",
-							digest: dgst,
-						})
-					}
-				} else {
-					images = append(images, &imageContext{
-						trunc: ctx.Trunc,
-						i:     image,
-						repo:  repo,
-						tag:   "<none>",
-					})
-				}
-			}
 		}
-		for _, imageCtx := range images {
+		for _, imageCtx := range formatted {
 			if err := format(imageCtx); err != nil {
 				return err
 			}
@@ -188,6 +107,82 @@ func imageFormat(ctx ImageContext, images []types.ImageSummary, format func(subC
 	return nil
 }
 
+func imageFormatTaggedAndDigest(ctx ImageContext, image types.ImageSummary) []*imageContext {
+	repoTags := map[string][]string{}
+	repoDigests := map[string][]string{}
+	images := []*imageContext{}
+
+	for _, refString := range image.RepoTags {
+		ref, err := reference.ParseNormalizedNamed(refString)
+		if err != nil {
+			continue
+		}
+		if nt, ok := ref.(reference.NamedTagged); ok {
+			familiarRef := reference.FamiliarName(ref)
+			repoTags[familiarRef] = append(repoTags[familiarRef], nt.Tag())
+		}
+	}
+	for _, refString := range image.RepoDigests {
+		ref, err := reference.ParseNormalizedNamed(refString)
+		if err != nil {
+			continue
+		}
+		if c, ok := ref.(reference.Canonical); ok {
+			familiarRef := reference.FamiliarName(ref)
+			repoDigests[familiarRef] = append(repoDigests[familiarRef], c.Digest().String())
+		}
+	}
+
+	addImage := func(repo, tag, digest string) {
+		image := &imageContext{
+			trunc:  ctx.Trunc,
+			i:      image,
+			repo:   repo,
+			tag:    tag,
+			digest: digest,
+		}
+		images = append(images, image)
+	}
+
+	for repo, tags := range repoTags {
+		digests := repoDigests[repo]
+
+		// Do not display digests as their own row
+		delete(repoDigests, repo)
+
+		if !needDigest(ctx) {
+			// Ignore digest references, just show tag once
+			digests = nil
+		}
+
+		for _, tag := range tags {
+			if len(digests) == 0 {
+				addImage(repo, tag, "<none>")
+				continue
+			}
+			// Display the digests for each tag
+			for _, dgst := range digests {
+				addImage(repo, tag, dgst)
+			}
+
+		}
+	}
+
+	// Show rows for remaining digest only references
+	for repo, digests := range repoDigests {
+		// If digests are displayed, show row per digest
+		if ctx.Digest {
+			for _, dgst := range digests {
+				addImage(repo, "<none>", dgst)
+			}
+		} else {
+			addImage(repo, "<none>", "")
+
+		}
+	}
+	return images
+}
+
 type imageContext struct {
 	HeaderContext
 	trunc  bool

cli/command/formatter/image_test.go

@@ -55,6 +55,26 @@ func TestImageContext(t *testing.T) {
 			i:      types.ImageSummary{},
 			digest: "sha256:d149ab53f8718e987c3a3024bb8aa0e2caadf6c0328f1d9d850b2a2a67f2819a",
 		}, "sha256:d149ab53f8718e987c3a3024bb8aa0e2caadf6c0328f1d9d850b2a2a67f2819a", ctx.Digest},
+		{
+			imageContext{
+				i: types.ImageSummary{Containers: 10},
+			}, "10", ctx.Containers,
+		},
+		{
+			imageContext{
+				i: types.ImageSummary{VirtualSize: 10000},
+			}, "10kB", ctx.VirtualSize,
+		},
+		{
+			imageContext{
+				i: types.ImageSummary{SharedSize: 10000},
+			}, "10kB", ctx.SharedSize,
+		},
+		{
+			imageContext{
+				i: types.ImageSummary{SharedSize: 5000, VirtualSize: 20000},
+			}, "15kB", ctx.UniqueSize,
+		},
 	}
 
 	for _, c := range cases {
@@ -62,8 +82,8 @@ func TestImageContext(t *testing.T) {
 		v := c.call()
 		if strings.Contains(v, ",") {
 			compareMultipleValues(t, v, c.expValue)
-		} else if v != c.expValue {
+		} else {
-			t.Fatalf("Expected %s, was %s\n", c.expValue, v)
+			assert.Equal(t, c.expValue, v)
 		}
 	}
 }

cli/compose/loader/loader.go

@@ -93,11 +93,7 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) {
 	}
 
 	cfg.Configs, err = LoadConfigObjs(config["configs"], configDetails.WorkingDir)
-	if err != nil {
+	return &cfg, err
-		return nil, err
-	}
-
-	return &cfg, nil
 }
 
 func interpolateConfig(configDict map[string]interface{}, lookupEnv template.Mapping) (map[string]map[string]interface{}, error) {