Make list-* commands consistent

This change ensures that all list-* commands show the same style response when there are no values to display. It is a simple print to stderr that says 'No foos to display.' This fixes https://bugs.launchpad.net/juju/+bug/1596687
sdg0919 · Sep 22, 2016 · 1a887db · 1a887db
1 parent f10fe4a
commit 1a887db
Show file tree

Hide file tree

Showing 26 changed files with 92 additions and 72 deletions.
diff --git a/cmd/juju/action/list.go b/cmd/juju/action/list.go
@@ -118,6 +118,10 @@ func (c *listCommand) Run(ctx *cmd.Context) error {
 	case "yaml", "json":
 		output = shortOutput
 	default:
+		if len(sortedNames) == 0 {
+			ctx.Infof("No actions defined for %s.", c.applicationTag.Id())
+			return nil
+		}
 		var list []listOutput
 		for _, name := range sortedNames {
 			list = append(list, listOutput{name, shortOutput[name]})
@@ -140,11 +144,6 @@ func (c *listCommand) printTabular(writer io.Writer, value interface{}) error {
 		return errors.New("unexpected value")
 	}
 
-	if len(list) == 0 {
-		fmt.Fprintf(writer, "No actions defined for %s", c.applicationTag.Id())
-		return nil
-	}
-
 	tw := output.TabWriter(writer)
 	fmt.Fprintf(tw, "%s\t%s\n", "ACTION", "DESCRIPTION")
 	for _, value := range list {

diff --git a/cmd/juju/action/list_test.go b/cmd/juju/action/list_test.go
@@ -6,6 +6,7 @@ package action_test
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/juju/cmd"
@@ -122,7 +123,7 @@ snapshot        Take a snapshot of the database.
 		should:          "work properly when no results found",
 		withArgs:        []string{validServiceId},
 		expectNoResults: true,
-		expectMessage:   "No actions defined for " + validServiceId,
+		expectMessage:   fmt.Sprintf("No actions defined for %s.\n", validServiceId),
 	}}
 
 	for i, t := range tests {
@@ -149,7 +150,7 @@ snapshot        Take a snapshot of the database.
 					if t.expectFullSchema {
 						checkFullSchema(c, t.withCharmActions, result)
 					} else if t.expectNoResults {
-						c.Check(string(result), gc.Matches, t.expectMessage+"(?sm).*")
+						c.Check(testing.Stderr(ctx), gc.Matches, t.expectMessage)
 					} else {
 						c.Check(testing.Stdout(ctx), gc.Equals, simpleOutput)
 					}

diff --git a/cmd/juju/backups/list.go b/cmd/juju/backups/list.go
@@ -64,7 +64,7 @@ func (c *listCommand) Run(ctx *cmd.Context) error {
 	}
 
 	if len(result.List) == 0 {
-		fmt.Fprintln(ctx.Stdout, "(no backups found)")
+		ctx.Infof("No backups to display.")
 		return nil
 	}
 

diff --git a/cmd/juju/block/list.go b/cmd/juju/block/list.go
@@ -85,6 +85,8 @@ func (c *listCommand) Run(ctx *cmd.Context) (err error) {
 	return c.listForModel(ctx)
 }
 
+const noBlocks = "No commands are currently disabled."
+
 func (c *listCommand) listForModel(ctx *cmd.Context) (err error) {
 	api, err := c.apiFunc(c)
 	if err != nil {
@@ -96,6 +98,10 @@ func (c *listCommand) listForModel(ctx *cmd.Context) (err error) {
 	if err != nil {
 		return errors.Trace(err)
 	}
+	if len(result) == 0 {
+		ctx.Infof(noBlocks)
+		return nil
+	}
 	return c.out.Write(ctx, formatBlockInfo(result))
 }
 
@@ -110,6 +116,10 @@ func (c *listCommand) listForController(ctx *cmd.Context) (err error) {
 	if err != nil {
 		return errors.Trace(err)
 	}
+	if len(result) == 0 {
+		ctx.Infof(noBlocks)
+		return nil
+	}
 	info, err := FormatModelBlockInfo(result)
 	if err != nil {
 		return errors.Trace(err)

diff --git a/cmd/juju/block/list_test.go b/cmd/juju/block/list_test.go
@@ -32,7 +32,7 @@ func (s *listCommandSuite) TestInit(c *gc.C) {
 func (s *listCommandSuite) TestListEmpty(c *gc.C) {
 	ctx, err := testing.RunCommand(c, block.NewListCommandForTest(&mockListClient{}, nil))
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(testing.Stdout(ctx), gc.Equals, "No commands are currently disabled.\n")
+	c.Assert(testing.Stderr(ctx), gc.Equals, "No commands are currently disabled.\n")
 }
 
 func (s *listCommandSuite) TestListError(c *gc.C) {

diff --git a/cmd/juju/cachedimages/list.go b/cmd/juju/cachedimages/list.go
@@ -119,7 +119,7 @@ func (c *listCommand) Run(ctx *cmd.Context) (err error) {
 	}
 	imageInfo := c.imageMetadataToImageInfo(results)
 	if len(imageInfo) == 0 {
-		fmt.Fprintf(ctx.Stdout, "no matching images found\n")
+		ctx.Infof("No images to display.")
 		return nil
 	}
 	fmt.Fprintf(ctx.Stdout, "Cached images:\n")

diff --git a/cmd/juju/cachedimages/list_test.go b/cmd/juju/cachedimages/list_test.go
@@ -59,7 +59,7 @@ func runListCommand(c *gc.C, args ...string) (*cmd.Context, error) {
 func (*listImagesCommandSuite) TestListImagesNone(c *gc.C) {
 	context, err := runListCommand(c, "--kind", "kvm")
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(testing.Stdout(context), gc.Equals, "no matching images found\n")
+	c.Assert(testing.Stderr(context), gc.Equals, "No images to display.\n")
 }
 
 func (*listImagesCommandSuite) TestListImagesFormatJson(c *gc.C) {

diff --git a/cmd/juju/commands/list_sshkeys.go b/cmd/juju/commands/list_sshkeys.go
@@ -84,6 +84,10 @@ func (c *listKeysCommand) Run(context *cmd.Context) error {
 	if result.Error != nil {
 		return result.Error
 	}
+	if len(result.Result) == 0 {
+		context.Infof("No keys to display.")
+		return nil
+	}
 	fmt.Fprintf(context.Stdout, "Keys used in model: %s\n", c.ConnectionName())
 	fmt.Fprintln(context.Stdout, strings.Join(result.Result, "\n"))
 	return nil

diff --git a/cmd/juju/romulus/listagreements/listagreements.go b/cmd/juju/romulus/listagreements/listagreements.go
@@ -87,8 +87,9 @@ func (c *listAgreementsCommand) Run(ctx *cmd.Context) error {
 	if err != nil {
 		return errors.Annotate(err, "failed to list user agreements")
 	}
-	if agreements == nil {
-		agreements = []wireformat.AgreementResponse{}
+	if len(agreements) == 0 {
+		ctx.Infof("No agreements to display.")
+		return nil
 	}
 	err = c.out.Write(ctx, agreements)
 	if err != nil {

diff --git a/cmd/juju/romulus/listagreements/listagreements_test.go b/cmd/juju/romulus/listagreements/listagreements_test.go
@@ -61,7 +61,8 @@ const (
 func (s *listAgreementsSuite) TestGetUsersAgreements(c *gc.C) {
 	ctx, err := cmdtesting.RunCommand(c, listagreements.NewListAgreementsCommand())
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(cmdtesting.Stdout(ctx), gc.Equals, "[]\n")
+	c.Assert(cmdtesting.Stdout(ctx), gc.Equals, "")
+	c.Assert(cmdtesting.Stderr(ctx), gc.Equals, "No agreements to display.\n")
 	c.Assert(s.client.called, jc.IsTrue)
 
 	s.client.setError("well, this is embarassing")
@@ -91,13 +92,8 @@ func (s *listAgreementsSuite) TestGetUsersAgreements(c *gc.C) {
 }
 
 func (s *listAgreementsSuite) TestGetUsersAgreementsWithTermOwner(c *gc.C) {
-	ctx, err := cmdtesting.RunCommand(c, listagreements.NewListAgreementsCommand())
-	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(cmdtesting.Stdout(ctx), gc.Equals, "[]\n")
-	c.Assert(s.client.called, jc.IsTrue)
-
 	s.client.setError("well, this is embarassing")
-	ctx, err = cmdtesting.RunCommand(c, listagreements.NewListAgreementsCommand())
+	ctx, err := cmdtesting.RunCommand(c, listagreements.NewListAgreementsCommand())
 	c.Assert(err, gc.ErrorMatches, "failed to list user agreements: well, this is embarassing")
 	c.Assert(s.client.called, jc.IsTrue)
 

diff --git a/cmd/juju/romulus/listplans/list_plans.go b/cmd/juju/romulus/listplans/list_plans.go
@@ -137,6 +137,11 @@ func (c *ListPlansCommand) Run(ctx *cmd.Context) (rErr error) {
 		}
 		output[i] = outputPlan
 	}
+
+	if len(output) == 0 && c.out.Name() == "tabular" {
+		ctx.Infof("No plans to display.")
+	}
+
 	err = c.out.Write(ctx, output)
 	if err != nil {
 		return errors.Trace(err)

diff --git a/cmd/juju/storage/filesystemlistformatters.go b/cmd/juju/storage/filesystemlistformatters.go
@@ -10,21 +10,11 @@ import (
 	"strings"
 
 	"github.com/dustin/go-humanize"
-	"github.com/juju/errors"
 	"github.com/juju/juju/cmd/output"
 )
 
 // formatFilesystemListTabular writes a tabular summary of filesystem instances.
-func formatFilesystemListTabular(writer io.Writer, value interface{}) error {
-	infos, ok := value.(map[string]FilesystemInfo)
-	if !ok {
-		return errors.Errorf("expected value of type %T, got %T", infos, value)
-	}
-	formatFilesystemListTabularTyped(writer, infos)
-	return nil
-}
-
-func formatFilesystemListTabularTyped(writer io.Writer, infos map[string]FilesystemInfo) {
+func formatFilesystemListTabular(writer io.Writer, infos map[string]FilesystemInfo) error {
 	tw := output.TabWriter(writer)
 
 	print := func(values ...string) {
@@ -76,7 +66,7 @@ func formatFilesystemListTabularTyped(writer io.Writer, infos map[string]Filesys
 		)
 	}
 
-	tw.Flush()
+	return tw.Flush()
 }
 
 type filesystemAttachmentInfo struct {

diff --git a/cmd/juju/storage/list.go b/cmd/juju/storage/list.go
@@ -85,7 +85,8 @@ func (c *listCommand) Run(ctx *cmd.Context) (err error) {
 	if err != nil {
 		return err
 	}
-	if output == nil {
+	if output == nil && c.out.Name() == "tabular" {
+		ctx.Infof("No storage to display.")
 		return nil
 	}
 	return c.out.Write(ctx, output)
@@ -123,8 +124,7 @@ func (c *listCommand) generateListOutput(ctx *cmd.Context, api StorageListAPI) (
 }
 
 func formatListTabular(writer io.Writer, value interface{}) error {
-
-	switch value.(type) {
+	switch value := value.(type) {
 	case map[string]StorageInfo:
 		return formatStorageListTabular(writer, value)
 

diff --git a/cmd/juju/storage/listformatters.go b/cmd/juju/storage/listformatters.go
@@ -10,17 +10,11 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/juju/errors"
 	"github.com/juju/juju/cmd/output"
 )
 
 // formatListTabular writes a tabular summary of storage instances.
-func formatStorageListTabular(writer io.Writer, value interface{}) error {
-	storageInfo, ok := value.(map[string]StorageInfo)
-	if !ok {
-		return errors.Errorf("expected value of type %T, got %T", storageInfo, value)
-	}
-
+func formatStorageListTabular(writer io.Writer, storageInfo map[string]StorageInfo) error {
 	tw := output.TabWriter(writer)
 	p := func(values ...interface{}) {
 		for _, v := range values {

diff --git a/cmd/juju/storage/poollist.go b/cmd/juju/storage/poollist.go
@@ -104,6 +104,7 @@ func (c *poolListCommand) Run(ctx *cmd.Context) (err error) {
 		return err
 	}
 	if len(result) == 0 {
+		ctx.Infof("No storage pools to display.")
 		return nil
 	}
 	output := formatPoolInfo(result)

diff --git a/cmd/juju/storage/volumelistformatters.go b/cmd/juju/storage/volumelistformatters.go
@@ -10,21 +10,11 @@ import (
 	"strings"
 
 	"github.com/dustin/go-humanize"
-	"github.com/juju/errors"
 	"github.com/juju/juju/cmd/output"
 )
 
 // formatVolumeListTabular returns a tabular summary of volume instances.
-func formatVolumeListTabular(writer io.Writer, value interface{}) error {
-	infos, ok := value.(map[string]VolumeInfo)
-	if !ok {
-		return errors.Errorf("expected value of type %T, got %T", infos, value)
-	}
-	formatVolumeListTabularTyped(writer, infos)
-	return nil
-}
-
-func formatVolumeListTabularTyped(writer io.Writer, infos map[string]VolumeInfo) {
+func formatVolumeListTabular(writer io.Writer, infos map[string]VolumeInfo) error {
 	tw := output.TabWriter(writer)
 
 	print := func(values ...string) {
@@ -76,8 +66,7 @@ func formatVolumeListTabularTyped(writer io.Writer, infos map[string]VolumeInfo)
 		)
 	}
 
-	tw.Flush()
-	return
+	return tw.Flush()
 }
 
 type volumeAttachmentInfo struct {

diff --git a/cmd/juju/subnet/list.go b/cmd/juju/subnet/list.go
@@ -103,9 +103,9 @@ func (c *listCommand) Run(ctx *cmd.Context) error {
 		// Display a nicer message in case no subnets were found.
 		if len(subnets) == 0 {
 			if c.SpaceName != "" || c.ZoneName != "" {
-				ctx.Infof("no subnets found matching requested criteria")
+				ctx.Infof("No subnets found matching requested criteria.")
 			} else {
-				ctx.Infof("no subnets to display")
+				ctx.Infof("No subnets to display.")
 			}
 			return nil
 		}

diff --git a/cmd/juju/subnet/list_test.go b/cmd/juju/subnet/list_test.go
@@ -223,7 +223,7 @@ func (s *ListSuite) TestRunWhenNoneMatchSucceeds(c *gc.C) {
 	s.api.Subnets = s.api.Subnets[0:0]
 
 	s.AssertRunSucceeds(c,
-		`no subnets found matching requested criteria\n`,
+		`No subnets found matching requested criteria.\n`,
 		"", // empty stdout.
 		"--space", "default",
 	)
@@ -237,7 +237,7 @@ func (s *ListSuite) TestRunWhenNoSubnetsExistSucceeds(c *gc.C) {
 	s.api.Subnets = s.api.Subnets[0:0]
 
 	s.AssertRunSucceeds(c,
-		`no subnets to display\n`,
+		`No subnets to display.\n`,
 		"", // empty stdout.
 	)
 

diff --git a/cmd/juju/user/list.go b/cmd/juju/user/list.go
@@ -131,6 +131,10 @@ func (c *listCommand) modelUsers(ctx *cmd.Context) error {
 	if err != nil {
 		return err
 	}
+	if len(result) == 0 {
+		ctx.Infof("No users to display.")
+		return nil
+	}
 	return c.out.Write(ctx, common.ModelUserInfoFromParams(result, c.clock.Now()))
 }
 
@@ -148,6 +152,11 @@ func (c *listCommand) controllerUsers(ctx *cmd.Context) error {
 		return err
 	}
 
+	if len(result) == 0 {
+		ctx.Infof("No users to display.")
+		return nil
+	}
+
 	return c.out.Write(ctx, c.apiUsersToUserInfoSlice(result))
 }
 

diff --git a/featuretests/cmd_juju_subnet_test.go b/featuretests/cmd_juju_subnet_test.go
@@ -150,7 +150,7 @@ func (s *cmdSubnetSuite) TestSubnetListNoResults(c *gc.C) {
 	context := s.Run(c, expectedSuccess, "list-subnets")
 	s.AssertOutput(c, context,
 		"", // no stdout output
-		"no subnets to display\n",
+		"No subnets to display.\n",
 	)
 }
 

diff --git a/featuretests/storage_test.go b/featuretests/storage_test.go
@@ -279,7 +279,7 @@ block:
 func (s *cmdStorageSuite) TestListPoolsNameNoMatch(c *gc.C) {
 	stdout, stderr, err := runPoolList(c, "--name", "cranky")
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(stderr, gc.Equals, "")
+	c.Assert(stderr, gc.Equals, "No storage pools to display.\n")
 	c.Assert(stdout, gc.Equals, "")
 }
 

diff --git a/payload/status/list.go b/payload/status/list.go
@@ -99,6 +99,11 @@ func (c *ListCommand) Run(ctx *cmd.Context) error {
 		fmt.Fprintf(ctx.Stderr, "%v\n", err)
 	}
 
+	if len(payloads) == 0 {
+		ctx.Infof("No payloads to display.")
+		return nil
+	}
+
 	// Note that we do not worry about c.CompatVersion for payloads...
 	formatter := newListFormatter(payloads)
 	formatted := formatter.format()

diff --git a/payload/status/list_test.go b/payload/status/list_test.go
@@ -95,12 +95,8 @@ func (s *listSuite) TestNoPayloads(c *gc.C) {
 	code, stdout, stderr := runList(c, command)
 	c.Assert(code, gc.Equals, 0)
 
-	c.Check(stdout, gc.Equals, `
-[Unit Payloads]
-UNIT  MACHINE  PAYLOAD-CLASS  STATUS  TYPE  ID  TAGS  
-
-`[1:])
-	c.Check(stderr, gc.Equals, "")
+	c.Check(stderr, gc.Equals, "No payloads to display.\n")
+	c.Check(stdout, gc.Equals, "")
 }
 
 func (s *listSuite) TestPatternsOkay(c *gc.C) {

diff --git a/resource/cmd/list_charm_resources.go b/resource/cmd/list_charm_resources.go
@@ -116,6 +116,12 @@ func (c *ListCharmResourcesCommand) Run(ctx *cmd.Context) error {
 	if len(resources) != 1 {
 		return errors.New("got bad data from charm store")
 	}
+	res := resources[0]
+
+	if len(res) == 0 && c.out.Name() == "tabular" {
+		ctx.Infof("No resources to display.")
+		return nil
+	}
 
 	// Note that we do not worry about c.CompatVersion
 	// for show-charm-resources...