Skip to content

Commit

Permalink
Standardize table printing (getporter#1876)
Browse files Browse the repository at this point in the history 
* Standardize table printing

Only use a single table printing library, with support for proper column
wrapping when data is a column is very long. We have been having trouble
with column data being so wide the table is unreadable when the terminal
forces the text to wrap.

This also updates all of Porter to use the single library, which has a
slightly different output (it adds a line under the table headers). When
I went through and updated all the expected console output tests, I took
the opportuntity to use the improved test.CompareToGoldenFile helper,
which makes syncing the expected test output much easier.

Signed-off-by: Carolyn Van Slyck <[email protected]>

* Update code to use new table printer

Update tests to use test.CompareGoldenFile

Signed-off-by: Carolyn Van Slyck <[email protected]>

* Improve test.CompareGoldenFile

When the golden file for a test doesn't exist, create it. This makes it
easier to get started using golden files without having to create the
empty file first. You can just edit the test to use that helper and it
will make the file for you when run with mage updatetestfiles

Signed-off-by: Carolyn Van Slyck <[email protected]>
Signed-off-by: joshuabezaleel <[email protected]>
  • Loading branch information
@carolynvs @joshuabezaleel
carolynvs authored and joshuabezaleel committed Feb 8, 2022
1 parent 8060f2e commit 0c3d075
Show file tree
Hide file tree
Showing 34 changed files with 319 additions and 180 deletions.
4 changes: 2 additions & 2 deletions pkg/porter/credentials.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ func (p *Porter) PrintCredentials(opts ListOptions) error {
}

printCredRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
cr, ok := v.(credentials.CredentialSet)
if !ok {
return nil
}
return []interface{}{cr.Namespace, cr.Name, tp.Format(cr.Modified)}
return []string{cr.Namespace, cr.Name, tp.Format(cr.Modified)}
}
return printer.PrintTable(p.Out, creds, printCredRow,
"NAMESPACE", "NAME", "MODIFIED")
Expand Down
77 changes: 31 additions & 46 deletions pkg/porter/credentials_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,37 +83,36 @@ func TestGenerateBadNameProvided(t *testing.T) {
}

type CredentialsListTest struct {
name string
format printer.Format
wantContains []string
errorMsg string
name string
format printer.Format
wantOutput string
errorMsg string
}

func TestCredentialsList_None(t *testing.T) {
testcases := []CredentialsListTest{
{
name: "invalid format",
format: "wingdings",
wantContains: []string{},
errorMsg: "invalid format: wingdings",
name: "invalid format",
format: "wingdings",
errorMsg: "invalid format: wingdings",
},
{
name: "json",
format: printer.FormatJson,
wantContains: []string{"[]\n"},
errorMsg: "",
name: "json",
format: printer.FormatJson,
wantOutput: "testdata/credentials/list-output.json",
errorMsg: "",
},
{
name: "yaml",
format: printer.FormatYaml,
wantContains: []string{"[]\n"},
errorMsg: "",
name: "yaml",
format: printer.FormatYaml,
wantOutput: "testdata/credentials/list-output.yaml",
errorMsg: "",
},
{
name: "plaintext",
format: printer.FormatPlaintext,
wantContains: []string{"NAME MODIFIED\n"},
errorMsg: "",
name: "plaintext",
format: printer.FormatPlaintext,
wantOutput: "testdata/credentials/list-output.txt",
errorMsg: "",
},
}

Expand All @@ -129,11 +128,8 @@ func TestCredentialsList_None(t *testing.T) {
require.Equal(t, err.Error(), tc.errorMsg)
} else {
require.NoError(t, err, "no error should have existed")
}

gotOutput := p.TestConfig.TestContext.GetOutput()
for _, contains := range tc.wantContains {
require.Contains(t, gotOutput, contains)
gotOutput := p.TestConfig.TestContext.GetOutput()
test.CompareGoldenFile(t, tc.wantOutput, gotOutput)
}
})
}
Expand All @@ -142,28 +138,19 @@ func TestCredentialsList_None(t *testing.T) {
func TestPorter_PrintCredentials(t *testing.T) {
testcases := []CredentialsListTest{
{
name: "json",
format: printer.FormatJson,
wantContains: []string{"\"namespace\": \"dev\",\n \"name\": \"kool-kreds\""},
errorMsg: "",
name: "json",
format: printer.FormatJson,
wantOutput: "testdata/credentials/show-output.json",
},
{
name: "yaml",
format: printer.FormatYaml,
wantContains: []string{"namespace: dev\n name: kool-kreds"},
errorMsg: "",
name: "yaml",
format: printer.FormatYaml,
wantOutput: "testdata/credentials/show-output.yaml",
},
{
name: "plaintext",
format: printer.FormatPlaintext,
wantContains: []string{"NAMESPACE NAME MODIFIED\ndev kool-kreds 2019-06-24"},
errorMsg: "",
},
{
name: "error",
format: printer.FormatPlaintext,
wantContains: []string{},
errorMsg: "",
name: "plaintext",
format: printer.FormatPlaintext,
wantOutput: "testdata/credentials/show-output.txt",
},
}

Expand All @@ -181,9 +168,7 @@ func TestPorter_PrintCredentials(t *testing.T) {
require.NoError(t, err)

gotOutput := p.TestConfig.TestContext.GetOutput()
for _, contains := range tc.wantContains {
require.Contains(t, gotOutput, contains)
}
test.CompareGoldenFile(t, tc.wantOutput, gotOutput)
})
}
}
Expand Down
21 changes: 11 additions & 10 deletions pkg/porter/explain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"sort"
"strconv"
"strings"

"get.porter.sh/porter/pkg/cnab"
Expand Down Expand Up @@ -349,12 +350,12 @@ func (p *Porter) printCredentialsExplainBlock(bun *PrintableBundle) error {
}
func (p *Porter) printCredentialsExplainTable(bun *PrintableBundle) error {
printCredRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
c, ok := v.(PrintableCredential)
if !ok {
return nil
}
return []interface{}{c.Name, c.Description, c.Required, c.ApplyTo}
return []string{c.Name, c.Description, strconv.FormatBool(c.Required), c.ApplyTo}
}
return printer.PrintTable(p.Out, bun.Credentials, printCredRow, "Name", "Description", "Required", "Applies To")
}
Expand All @@ -375,12 +376,12 @@ func (p *Porter) printParametersExplainBlock(bun *PrintableBundle) error {
}
func (p *Porter) printParametersExplainTable(bun *PrintableBundle) error {
printParamRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
p, ok := v.(PrintableParameter)
if !ok {
return nil
}
return []interface{}{p.Name, p.Description, p.Type, p.Default, p.Required, p.ApplyTo}
return []string{p.Name, p.Description, fmt.Sprintf("%v", p.Type), fmt.Sprintf("%v", p.Default), strconv.FormatBool(p.Required), p.ApplyTo}
}
return printer.PrintTable(p.Out, bun.Parameters, printParamRow, "Name", "Description", "Type", "Default", "Required", "Applies To")
}
Expand All @@ -402,12 +403,12 @@ func (p *Porter) printOutputsExplainBlock(bun *PrintableBundle) error {

func (p *Porter) printOutputsExplainTable(bun *PrintableBundle) error {
printOutputRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
o, ok := v.(PrintableOutput)
if !ok {
return nil
}
return []interface{}{o.Name, o.Description, o.Type, o.ApplyTo}
return []string{o.Name, o.Description, fmt.Sprintf("%v", o.Type), o.ApplyTo}
}
return printer.PrintTable(p.Out, bun.Outputs, printOutputRow, "Name", "Description", "Type", "Applies To")
}
Expand All @@ -429,12 +430,12 @@ func (p *Porter) printActionsExplainBlock(bun *PrintableBundle) error {

func (p *Porter) printActionsExplainTable(bun *PrintableBundle) error {
printActionRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
a, ok := v.(PrintableAction)
if !ok {
return nil
}
return []interface{}{a.Name, a.Description, a.Modifies, a.Stateless}
return []string{a.Name, a.Description, strconv.FormatBool(a.Modifies), strconv.FormatBool(a.Stateless)}
}
return printer.PrintTable(p.Out, bun.Actions, printActionRow, "Name", "Description", "Modifies Installation", "Stateless")
}
Expand All @@ -457,12 +458,12 @@ func (p *Porter) printDependenciesExplainBlock(bun *PrintableBundle) error {

func (p *Porter) printDependenciesExplainTable(bun *PrintableBundle) error {
printDependencyRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
o, ok := v.(PrintableDependency)
if !ok {
return nil
}
return []interface{}{o.Alias, o.Reference}
return []string{o.Alias, o.Reference}
}
return printer.PrintTable(p.Out, bun.Dependencies, printDependencyRow, "Alias", "Reference")
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/porter/explain_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package porter

import (
"fmt"
"io/ioutil"
"testing"

"get.porter.sh/porter/pkg/cnab"
"get.porter.sh/porter/pkg/test"
"github.com/cnabio/cnab-go/bundle"
"github.com/cnabio/cnab-go/bundle/definition"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -41,9 +41,7 @@ func TestExplain_generateTable(t *testing.T) {
err = p.printBundleExplain(opts, pb)
assert.NoError(t, err)
gotOutput := p.TestConfig.TestContext.GetOutput()
expected, err := ioutil.ReadFile("testdata/explain/expected-table-output.txt")
require.NoError(t, err)
assert.Equal(t, string(expected), gotOutput)
test.CompareGoldenFile(t, "testdata/explain/expected-table-output.txt", gotOutput)
}

func TestExplain_generateJSON(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/porter/inspect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,12 @@ func (p *Porter) printInvocationImageInspectBlock(bun *InspectableBundle) error

func (p *Porter) printInvocationImageInspectTable(bun *InspectableBundle) error {
printInvocationImageRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
ii, ok := v.(PrintableInvocationImage)
if !ok {
return nil
}
return []interface{}{ii.Image, ii.ImageType, ii.Digest, ii.Original}
return []string{ii.Image, ii.ImageType, ii.Digest, ii.Original}
}
return printer.PrintTable(p.Out, bun.InvocationImages, printInvocationImageRow, "Image", "Type", "Digest", "Original Image")
}
Expand All @@ -142,12 +142,12 @@ func (p *Porter) printImagesInspectBlock(bun *InspectableBundle) error {

func (p *Porter) printImagesInspectTable(bun *InspectableBundle) error {
printImageRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
pi, ok := v.(PrintableImage)
if !ok {
return nil
}
return []interface{}{pi.Name, pi.ImageType, pi.Image.Image, pi.Digest, pi.Original}
return []string{pi.Name, pi.ImageType, pi.Image.Image, pi.Digest, pi.Original}
}
return printer.PrintTable(p.Out, bun.Images, printImageRow, "Name", "Type", "Image", "Digest", "Original Image")
}
4 changes: 2 additions & 2 deletions pkg/porter/list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,12 @@ func (p *Porter) PrintInstallations(ctx context.Context, opts ListOptions) error
}

row :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
cl, ok := v.(DisplayInstallation)
if !ok {
return nil
}
return []interface{}{cl.Namespace, cl.Name, tp.Format(cl.Status.Created), tp.Format(cl.Status.Modified), cl.Status.Action, cl.Status.ResultStatus}
return []string{cl.Namespace, cl.Name, tp.Format(cl.Status.Created), tp.Format(cl.Status.Modified), cl.Status.Action, cl.Status.ResultStatus}
}
return printer.PrintTable(p.Out, displayInstallations, row,
"NAMESPACE", "NAME", "CREATED", "MODIFIED", "LAST ACTION", "LAST STATUS")
Expand Down
4 changes: 2 additions & 2 deletions pkg/porter/mixins.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ func (p *Porter) PrintMixins(opts PrintMixinsOptions) error {
switch opts.Format {
case printer.FormatPlaintext:
printMixinRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
m, ok := v.(mixin.Metadata)
if !ok {
return nil
}
return []interface{}{m.Name, m.VersionInfo.Version, m.VersionInfo.Author}
return []string{m.Name, m.VersionInfo.Version, m.VersionInfo.Author}
}
return printer.PrintTable(p.Out, mixins, printMixinRow, "Name", "Version", "Author")
case printer.FormatJson:
Expand Down
4 changes: 4 additions & 0 deletions pkg/porter/mixins/list-output.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---------------------------------
Name Version Author
---------------------------------
exec v1.0 Porter Authors
6 changes: 2 additions & 4 deletions pkg/porter/mixins_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"get.porter.sh/porter/pkg/mixin"
"get.porter.sh/porter/pkg/pkgmgmt"
"get.porter.sh/porter/pkg/printer"
"get.porter.sh/porter/pkg/test"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -24,11 +25,8 @@ func TestPorter_PrintMixins(t *testing.T) {
err := p.PrintMixins(opts)

require.Nil(t, err)
wantOutput := `Name Version Author
exec v1.0 Porter Authors
`
gotOutput := p.TestConfig.TestContext.GetOutput()
assert.Equal(t, wantOutput, gotOutput)
test.CompareGoldenFile(t, "mixins/list-output.txt", gotOutput)
}

func TestPorter_InstallMixin(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/porter/packages.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (p *Porter) PrintPackages(opts SearchOptions, list pkgmgmt.PackageList) err
switch opts.Format {
case printer.FormatPlaintext:
printMixinRow :=
func(v interface{}) []interface{} {
func(v interface{}) []string {
m, ok := v.(pkgmgmt.PackageListing)
if !ok {
return nil
Expand All @@ -85,7 +85,7 @@ func (p *Porter) PrintPackages(opts SearchOptions, list pkgmgmt.PackageList) err
} else {
urlType = "Unknown"
}
return []interface{}{m.Name, m.Description, m.Author, m.URL, urlType}
return []string{m.Name, m.Description, m.Author, m.URL, urlType}
}
return printer.PrintTable(p.Out, list, printMixinRow, "Name", "Description", "Author", "URL", "URL Type")
case printer.FormatJson:
Expand Down
Loading

0 comments on commit 0c3d075

Please sign in to comment.