align --format flag and UX with docker cli

ndeloof · ndeloof · commit 8c39b5b7fd42 · 2022-12-14T22:53:43.000+01:00
Signed-off-by: Nicolas De Loof &lt;nicolas.deloof@gmail.com&gt;
diff --git a/cmd/compose/images.go b/cmd/compose/images.go
@@ -35,7 +35,8 @@ import (
 
 type imageOptions struct {
 	*projectOptions
-	Quiet bool
+	Quiet  bool
+	Format string
 }
 
 func imagesCommand(p *projectOptions, backend api.Service) *cobra.Command {
@@ -50,6 +51,7 @@ func imagesCommand(p *projectOptions, backend api.Service) *cobra.Command {
 		}),
 		ValidArgsFunction: completeServiceNames(p),
 	}
+	imgCmd.Flags().StringVar(&opts.Format, "format", "table", "Format the output. Values: [table | json].")
 	imgCmd.Flags().BoolVarP(&opts.Quiet, "quiet", "q", false, "Only display IDs")
 	return imgCmd
 }
@@ -88,7 +90,7 @@ func runImages(ctx context.Context, backend api.Service, opts imageOptions, serv
 		return images[i].ContainerName < images[j].ContainerName
 	})
 
-	return formatter.Print(images, formatter.PRETTY, os.Stdout,
+	return formatter.Print(images, opts.Format, os.Stdout,
 		func(w io.Writer) {
 			for _, img := range images {
 				id := stringid.TruncateID(img.ID)
@@ -104,5 +106,5 @@ func runImages(ctx context.Context, backend api.Service, opts imageOptions, serv
 				_, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", img.ContainerName, repo, tag, id, size)
 			}
 		},
-		"Container", "Repository", "Tag", "Image Id", "Size")
+		"CONTAINER", "REPOSITORY", "TAG", "IMAGE ID", "SIZE")
 }
diff --git a/cmd/compose/list.go b/cmd/compose/list.go
@@ -49,7 +49,7 @@ func listCommand(backend api.Service) *cobra.Command {
 		Args:              cobra.NoArgs,
 		ValidArgsFunction: noCompletion(),
 	}
-	lsCmd.Flags().StringVar(&lsOpts.Format, "format", "pretty", "Format the output. Values: [pretty | json].")
+	lsCmd.Flags().StringVar(&lsOpts.Format, "format", "table", "Format the output. Values: [table | json].")
 	lsCmd.Flags().BoolVarP(&lsOpts.Quiet, "quiet", "q", false, "Only display IDs.")
 	lsCmd.Flags().Var(&lsOpts.Filter, "filter", "Filter output based on conditions provided.")
 	lsCmd.Flags().BoolVarP(&lsOpts.All, "all", "a", false, "Show all stopped Compose projects")
diff --git a/cmd/compose/ps.go b/cmd/compose/ps.go
@@ -83,7 +83,7 @@ func psCommand(p *projectOptions, backend api.Service) *cobra.Command {
 		ValidArgsFunction: completeServiceNames(p),
 	}
 	flags := psCmd.Flags()
-	flags.StringVar(&opts.Format, "format", "pretty", "Format the output. Values: [pretty | json]")
+	flags.StringVar(&opts.Format, "format", "table", "Format the output. Values: [table | json]")
 	flags.StringVar(&opts.Filter, "filter", "", "Filter services by a property (supported filters: status).")
 	flags.StringArrayVar(&opts.Status, "status", []string{}, "Filter services by status. Values: [paused | restarting | removing | running | dead | created | exited]")
 	flags.BoolVarP(&opts.Quiet, "quiet", "q", false, "Only display IDs")
diff --git a/cmd/compose/ps_test.go b/cmd/compose/ps_test.go
@@ -28,14 +28,15 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestPsPretty(t *testing.T) {
+func TestPsTable(t *testing.T) {
 	ctx := context.Background()
 	origStdout := os.Stdout
 	t.Cleanup(func() {
 		os.Stdout = origStdout
 	})
 	dir := t.TempDir()
-	f, err := os.Create(filepath.Join(dir, "output.txt"))
+	out := filepath.Join(dir, "output.txt")
+	f, err := os.Create(out)
 	if err != nil {
 		t.Fatal("could not create output file")
 	}
@@ -51,8 +52,9 @@ func TestPsPretty(t *testing.T) {
 		DoAndReturn(func(ctx context.Context, projectName string, options api.PsOptions) ([]api.ContainerSummary, error) {
 			return []api.ContainerSummary{
 				{
-					ID:   "abc123",
-					Name: "ABC",
+					ID:    "abc123",
+					Name:  "ABC",
+					Image: "foo/bar",
 					Publishers: api.PortPublishers{
 						{
 							TargetPort:    8080,
@@ -76,8 +78,7 @@ func TestPsPretty(t *testing.T) {
 	_, err = f.Seek(0, 0)
 	assert.NoError(t, err)
 
-	output := make([]byte, 256)
-	_, err = f.Read(output)
+	output, err := os.ReadFile(out)
 	assert.NoError(t, err)
 
 	assert.Contains(t, string(output), "8080/tcp, 8443/tcp")
diff --git a/cmd/formatter/consts.go b/cmd/formatter/consts.go
@@ -17,10 +17,13 @@
 package formatter
 
 const (
-	// JSON is the constant for Json formats on list commands
+	// JSON Print in JSON format
 	JSON = "json"
 	// TemplateLegacyJSON the legacy json formatting value using go template
 	TemplateLegacyJSON = "{{json.}}"
 	// PRETTY is the constant for default formats on list commands
+	// Deprecated: use TABLE
 	PRETTY = "pretty"
+	// TABLE Print output in table format with column headers (default)
+	TABLE = "table"
 )
diff --git a/cmd/formatter/formatter.go b/cmd/formatter/formatter.go
@@ -30,7 +30,7 @@ import (
 // Print prints formatted lists in different formats
 func Print(toJSON interface{}, format string, outWriter io.Writer, writerFn func(w io.Writer), headers ...string) error {
 	switch strings.ToLower(format) {
-	case PRETTY, "":
+	case TABLE, PRETTY, "":
 		return PrintPrettySection(outWriter, writerFn, headers...)
 	case TemplateLegacyJSON:
 		switch reflect.TypeOf(toJSON).Kind() {
diff --git a/docs/reference/compose_images.md b/docs/reference/compose_images.md
@@ -7,6 +7,7 @@ List images used by the created containers
 
 | Name | Type | Default | Description |
 | --- | --- | --- | --- |
+| `--format` | `string` | `table` | Format the output. Values: [table \| json]. |
 | `-q`, `--quiet` |  |  | Only display IDs |
 
 
diff --git a/docs/reference/compose_ls.md b/docs/reference/compose_ls.md
@@ -9,7 +9,7 @@ List running compose projects
 | --- | --- | --- | --- |
 | `-a`, `--all` |  |  | Show all stopped Compose projects |
 | `--filter` | `filter` |  | Filter output based on conditions provided. |
-| `--format` | `string` | `pretty` | Format the output. Values: [pretty \| json]. |
+| `--format` | `string` | `table` | Format the output. Values: [table \| json]. |
 | `-q`, `--quiet` |  |  | Only display IDs. |
 
 
diff --git a/docs/reference/compose_ps.md b/docs/reference/compose_ps.md
@@ -9,7 +9,7 @@ List containers
 | --- | --- | --- | --- |
 | `-a`, `--all` |  |  | Show all stopped containers (including those created by the run command) |
 | [`--filter`](#filter) | `string` |  | Filter services by a property (supported filters: status). |
-| [`--format`](#format) | `string` | `pretty` | Format the output. Values: [pretty \| json] |
+| [`--format`](#format) | `string` | `table` | Format the output. Values: [table \| json] |
 | `-q`, `--quiet` |  |  | Only display IDs |
 | `--services` |  |  | Display services |
 | [`--status`](#status) | `stringArray` |  | Filter services by status. Values: [paused \| restarting \| removing \| running \| dead \| created \| exited] |
diff --git a/docs/reference/docker_compose_images.yaml b/docs/reference/docker_compose_images.yaml
@@ -5,6 +5,16 @@ usage: docker compose images [OPTIONS] [SERVICE...]
 pname: docker compose
 plink: docker_compose.yaml
 options:
+    - option: format
+      value_type: string
+      default_value: table
+      description: 'Format the output. Values: [table | json].'
+      deprecated: false
+      hidden: false
+      experimental: false
+      experimentalcli: false
+      kubernetes: false
+      swarm: false
     - option: quiet
       shorthand: q
       value_type: bool
diff --git a/docs/reference/docker_compose_ls.yaml b/docs/reference/docker_compose_ls.yaml
@@ -27,8 +27,8 @@ options:
       swarm: false
     - option: format
       value_type: string
-      default_value: pretty
-      description: 'Format the output. Values: [pretty | json].'
+      default_value: table
+      description: 'Format the output. Values: [table | json].'
       deprecated: false
       hidden: false
       experimental: false
diff --git a/docs/reference/docker_compose_ps.yaml b/docs/reference/docker_compose_ps.yaml
@@ -38,8 +38,8 @@ options:
       swarm: false
     - option: format
       value_type: string
-      default_value: pretty
-      description: 'Format the output. Values: [pretty | json]'
+      default_value: table
+      description: 'Format the output. Values: [table | json]'
       details_url: '#format'
       deprecated: false
       hidden: false
diff --git a/pkg/compose/ps_test.go b/pkg/compose/ps_test.go
@@ -61,10 +61,13 @@ func TestPs(t *testing.T) {
 	containers, err := tested.Ps(ctx, strings.ToLower(testProject), compose.PsOptions{})
 
 	expected := []compose.ContainerSummary{
-		{ID: "123", Name: "123", Project: strings.ToLower(testProject), Service: "service1", State: "running", Health: "healthy", Publishers: nil},
-		{ID: "456", Name: "456", Project: strings.ToLower(testProject), Service: "service1", State: "running", Health: "", Publishers: []compose.PortPublisher{{URL: "localhost", TargetPort: 90,
-			PublishedPort: 80}}},
-		{ID: "789", Name: "789", Project: strings.ToLower(testProject), Service: "service2", State: "exited", Health: "", ExitCode: 130, Publishers: nil},
+		{ID: "123", Name: "123", Image: "foo", Project: strings.ToLower(testProject), Service: "service1",
+			State: "running", Health: "healthy", Publishers: nil},
+		{ID: "456", Name: "456", Image: "foo", Project: strings.ToLower(testProject), Service: "service1",
+			State: "running", Health: "",
+			Publishers: []compose.PortPublisher{{URL: "localhost", TargetPort: 90, PublishedPort: 80}}},
+		{ID: "789", Name: "789", Image: "foo", Project: strings.ToLower(testProject), Service: "service2",
+			State: "exited", Health: "", ExitCode: 130, Publishers: nil},
 	}
 	assert.NilError(t, err)
 	assert.DeepEqual(t, containers, expected)
@@ -74,6 +77,7 @@ func containerDetails(service string, id string, status string, health string, e
 	container := moby.Container{
 		ID:     id,
 		Names:  []string{"/" + id},
+		Image:  "foo",
 		Labels: containerLabels(service, false),
 		State:  status,
 	}
diff --git a/pkg/e2e/compose_test.go b/pkg/e2e/compose_test.go
@@ -88,13 +88,11 @@ func TestLocalComposeUp(t *testing.T) {
 
 	t.Run("check healthcheck output", func(t *testing.T) {
 		c.WaitForCmdResult(t, c.NewDockerComposeCmd(t, "-p", projectName, "ps", "--format", "json"),
-			StdoutContains(`"Name":"compose-e2e-demo-web-1","Command":"/dispatcher","Project":"compose-e2e-demo","Service":"web","State":"running","Health":"healthy"`),
+			IsHealthy(projectName+"-web-1"),
 			5*time.Second, 1*time.Second)
 
 		res := c.RunDockerComposeCmd(t, "-p", projectName, "ps")
-		res.Assert(t, icmd.Expected{Out: `NAME                       COMMAND                  SERVICE             STATUS              PORTS`})
-		res.Assert(t, icmd.Expected{Out: `compose-e2e-demo-web-1     "/dispatcher"            web                 running (healthy)   0.0.0.0:90->80/tcp`})
-		res.Assert(t, icmd.Expected{Out: `compose-e2e-demo-db-1      "docker-entrypoint.s…"   db                  running             5432/tcp`})
+		assertServiceStatus(t, projectName, "web", "(healthy)", res.Stdout())
 	})
 
 	t.Run("images", func(t *testing.T) {
diff --git a/pkg/e2e/cp_test.go b/pkg/e2e/cp_test.go
@@ -45,7 +45,7 @@ func TestCopy(t *testing.T) {
 
 	t.Run("make sure service is running", func(t *testing.T) {
 		res := c.RunDockerComposeCmd(t, "-p", projectName, "ps")
-		res.Assert(t, icmd.Expected{Out: `nginx               running`})
+		assertServiceStatus(t, projectName, "nginx", "Up", res.Stdout())
 	})
 
 	t.Run("copy to container copies the file to the all containers by default", func(t *testing.T) {
diff --git a/pkg/e2e/framework.go b/pkg/e2e/framework.go
@@ -17,6 +17,7 @@
 package e2e
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -314,6 +315,27 @@ func StdoutContains(expected string) func(*icmd.Result) bool {
 	}
 }
 
+func IsHealthy(service string) func(res *icmd.Result) bool {
+	return func(res *icmd.Result) bool {
+		type state struct {
+			Name   string `json:"name"`
+			Health string `json:"health"`
+		}
+
+		ps := []state{}
+		err := json.Unmarshal([]byte(res.Stdout()), &ps)
+		if err != nil {
+			return false
+		}
+		for _, state := range ps {
+			if state.Name == service && state.Health == "healthy" {
+				return true
+			}
+		}
+		return false
+	}
+}
+
 // WaitForCmdResult try to execute a cmd until resulting output matches given predicate
 func (c *CLI) WaitForCmdResult(
 	t testing.TB,
diff --git a/pkg/e2e/ps_test.go b/pkg/e2e/ps_test.go
@@ -40,7 +40,7 @@ func TestPs(t *testing.T) {
 
 	assert.Contains(t, res.Combined(), "Container e2e-ps-busybox-1  Started", res.Combined())
 
-	t.Run("pretty", func(t *testing.T) {
+	t.Run("table", func(t *testing.T) {
 		res = c.RunDockerComposeCmd(t, "-f", "./fixtures/ps-test/compose.yaml", "--project-name", projectName, "ps")
 		lines := strings.Split(res.Stdout(), "\n")
 		assert.Equal(t, 4, len(lines))
diff --git a/pkg/e2e/restart_test.go b/pkg/e2e/restart_test.go
@@ -26,16 +26,17 @@ import (
 	"gotest.tools/v3/assert"
 )
 
+func assertServiceStatus(t *testing.T, projectName, service, status string, ps string) {
+	// match output with random spaces like:
+	// e2e-start-stop-db-1      alpine:latest "echo hello"     db	1 minutes ago	Exited (0) 1 minutes ago
+	regx := fmt.Sprintf("%s-%s-1.+%s\\s+.+%s.+", projectName, service, service, status)
+	testify.Regexp(t, regx, ps)
+}
+
 func TestRestart(t *testing.T) {
 	c := NewParallelCLI(t)
 	const projectName = "e2e-restart"
 
-	getServiceRegx := func(service string, status string) string {
-		// match output with random spaces like:
-		// e2e-start-stop-db-1      "echo hello"     db      running
-		return fmt.Sprintf("%s-%s-1.+%s\\s+%s", projectName, service, service, status)
-	}
-
 	t.Run("Up a project", func(t *testing.T) {
 		// This is just to ensure the containers do NOT exist
 		c.RunDockerComposeCmd(t, "--project-name", projectName, "down")
@@ -48,15 +49,15 @@ func TestRestart(t *testing.T) {
 			StdoutContains(`"State":"exited"`), 10*time.Second, 1*time.Second)
 
 		res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps", "-a")
-		testify.Regexp(t, getServiceRegx("restart", "exited"), res.Stdout())
+		assertServiceStatus(t, projectName, "restart", "Exited", res.Stdout())
 
 		c.RunDockerComposeCmd(t, "-f", "./fixtures/restart-test/compose.yaml", "--project-name", projectName, "restart")
 
 		// Give the same time but it must NOT exit
 		time.Sleep(time.Second)
 
 		res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps")
-		testify.Regexp(t, getServiceRegx("restart", "running"), res.Stdout())
+		assertServiceStatus(t, projectName, "restart", "Up", res.Stdout())
 
 		// Clean up
 		c.RunDockerComposeCmd(t, "--project-name", projectName, "down")
diff --git a/pkg/e2e/start_stop_test.go b/pkg/e2e/start_stop_test.go
@@ -36,12 +36,6 @@ func TestStartStop(t *testing.T) {
 		return fmt.Sprintf("%s\\s+%s\\(%d\\)", projectName, status, 2)
 	}
 
-	getServiceRegx := func(service string, status string) string {
-		// match output with random spaces like:
-		// e2e-start-stop-db-1      "echo hello"       db          running
-		return fmt.Sprintf("%s-%s-1.+%s\\s+%s", projectName, service, service, status)
-	}
-
 	t.Run("Up a project", func(t *testing.T) {
 		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/start-stop/compose.yaml", "--project-name", projectName, "up",
 			"-d")
@@ -51,8 +45,8 @@ func TestStartStop(t *testing.T) {
 		testify.Regexp(t, getProjectRegx("running"), res.Stdout())
 
 		res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps")
-		testify.Regexp(t, getServiceRegx("simple", "running"), res.Stdout())
-		testify.Regexp(t, getServiceRegx("another", "running"), res.Stdout())
+		assertServiceStatus(t, projectName, "simple", "Up", res.Stdout())
+		assertServiceStatus(t, projectName, "another", "Up", res.Stdout())
 	})
 
 	t.Run("stop project", func(t *testing.T) {
@@ -68,8 +62,8 @@ func TestStartStop(t *testing.T) {
 		assert.Assert(t, !strings.Contains(res.Combined(), "e2e-start-stop-no-dependencies-words-1"), res.Combined())
 
 		res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps", "--all")
-		testify.Regexp(t, getServiceRegx("simple", "exited"), res.Stdout())
-		testify.Regexp(t, getServiceRegx("another", "exited"), res.Stdout())
+		assertServiceStatus(t, projectName, "simple", "Exited", res.Stdout())
+		assertServiceStatus(t, projectName, "another", "Exited", res.Stdout())
 	})
 
 	t.Run("start project", func(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,8 @@ import (`
`35`	`35`
`36`	`36`	`type imageOptions struct {`
`37`	`37`	`*projectOptions`
`38`		`- Quiet bool`
	`38`	`+ Quiet bool`
	`39`	`+ Format string`
`39`	`40`	`}`
`40`	`41`
`41`	`42`	`func imagesCommand(p projectOptions, backend api.Service) cobra.Command {`
`@@ -50,6 +51,7 @@ func imagesCommand(p projectOptions, backend api.Service) cobra.Command {`
`50`	`51`	`}),`
`51`	`52`	`ValidArgsFunction: completeServiceNames(p),`
`52`	`53`	`}`
	`54`	`+ imgCmd.Flags().StringVar(&opts.Format, "format", "table", "Format the output. Values: [table \| json].")`
`53`	`55`	`imgCmd.Flags().BoolVarP(&opts.Quiet, "quiet", "q", false, "Only display IDs")`
`54`	`56`	`return imgCmd`
`55`	`57`	`}`
`@@ -88,7 +90,7 @@ func runImages(ctx context.Context, backend api.Service, opts imageOptions, serv`
`88`	`90`	`return images[i].ContainerName < images[j].ContainerName`
`89`	`91`	`})`
`90`	`92`
`91`		`- return formatter.Print(images, formatter.PRETTY, os.Stdout,`
	`93`	`+ return formatter.Print(images, opts.Format, os.Stdout,`
`92`	`94`	`func(w io.Writer) {`
`93`	`95`	`for _, img := range images {`
`94`	`96`	`id := stringid.TruncateID(img.ID)`
`@@ -104,5 +106,5 @@ func runImages(ctx context.Context, backend api.Service, opts imageOptions, serv`
`104`	`106`	`_, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", img.ContainerName, repo, tag, id, size)`
`105`	`107`	`}`
`106`	`108`	`},`
`107`		`- "Container", "Repository", "Tag", "Image Id", "Size")`
	`109`	`+ "CONTAINER", "REPOSITORY", "TAG", "IMAGE ID", "SIZE")`
`108`	`110`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ func listCommand(backend api.Service) *cobra.Command {`
`49`	`49`	`Args: cobra.NoArgs,`
`50`	`50`	`ValidArgsFunction: noCompletion(),`
`51`	`51`	`}`
`52`		`- lsCmd.Flags().StringVar(&lsOpts.Format, "format", "pretty", "Format the output. Values: [pretty \| json].")`
	`52`	`+ lsCmd.Flags().StringVar(&lsOpts.Format, "format", "table", "Format the output. Values: [table \| json].")`
`53`	`53`	`lsCmd.Flags().BoolVarP(&lsOpts.Quiet, "quiet", "q", false, "Only display IDs.")`
`54`	`54`	`lsCmd.Flags().Var(&lsOpts.Filter, "filter", "Filter output based on conditions provided.")`
`55`	`55`	`lsCmd.Flags().BoolVarP(&lsOpts.All, "all", "a", false, "Show all stopped Compose projects")`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ func psCommand(p projectOptions, backend api.Service) cobra.Command {`
`83`	`83`	`ValidArgsFunction: completeServiceNames(p),`
`84`	`84`	`}`
`85`	`85`	`flags := psCmd.Flags()`
`86`		`- flags.StringVar(&opts.Format, "format", "pretty", "Format the output. Values: [pretty \| json]")`
	`86`	`+ flags.StringVar(&opts.Format, "format", "table", "Format the output. Values: [table \| json]")`
`87`	`87`	`flags.StringVar(&opts.Filter, "filter", "", "Filter services by a property (supported filters: status).")`
`88`	`88`	`flags.StringArrayVar(&opts.Status, "status", []string{}, "Filter services by status. Values: [paused \| restarting \| removing \| running \| dead \| created \| exited]")`
`89`	`89`	`flags.BoolVarP(&opts.Quiet, "quiet", "q", false, "Only display IDs")`