From d318c4112b1e4e00a4c9c78acc59e57955e38d6a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 9 May 2017 17:24:40 -0400 Subject: [PATCH] Reduce complexity of two formatters Signed-off-by: Daniel Nephin --- cli/command/formatter/custom_test.go | 7 +- cli/command/formatter/image.go | 165 +++++++++++++-------------- cli/command/formatter/image_test.go | 24 +++- cli/compose/loader/loader.go | 6 +- 4 files changed, 106 insertions(+), 96 deletions(-) diff --git a/cli/command/formatter/custom_test.go b/cli/command/formatter/custom_test.go index da42039dca..a9f6ccdac9 100644 --- a/cli/command/formatter/custom_test.go +++ b/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) { - t.Fatalf("Expected entries: %v, got: %v", expected, value) - } + assert.Equal(t, expMap, entriesMap) } diff --git a/cli/command/formatter/image.go b/cli/command/formatter/image.go index aaf8ff1aa1..e94785ef08 100644 --- a/cli/command/formatter/image.go +++ b/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: "", @@ -96,90 +96,9 @@ func imageFormat(ctx ImageContext, images []types.ImageSummary, format func(subC digest: "", }) } else { - repoTags := map[string][]string{} - 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: "", - }) - 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: "", - digest: dgst, - }) - } - } else { - images = append(images, &imageContext{ - trunc: ctx.Trunc, - i: image, - repo: repo, - tag: "", - }) - } - } + formatted = imageFormatTaggedAndDigest(ctx, image) } - 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, "") + 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, "", dgst) + } + } else { + addImage(repo, "", "") + + } + } + return images +} + type imageContext struct { HeaderContext trunc bool diff --git a/cli/command/formatter/image_test.go b/cli/command/formatter/image_test.go index be7b084e3c..20b73a52c1 100644 --- a/cli/command/formatter/image_test.go +++ b/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 { - t.Fatalf("Expected %s, was %s\n", c.expValue, v) + } else { + assert.Equal(t, c.expValue, v) } } } diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index 2fb2630087..2bc941e652 100644 --- a/cli/compose/loader/loader.go +++ b/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 nil, err - } - - return &cfg, nil + return &cfg, err } func interpolateConfig(configDict map[string]interface{}, lookupEnv template.Mapping) (map[string]map[string]interface{}, error) {