Merge pull request docker#9539 from docker/nicksieger/9527

Fix ps ports output; includes unit test

Author: glours

Makefile

@@ -55,6 +55,7 @@ e2e-compose-standalone: ## Run End to end local tests in standalone mode. Set E2
 mocks:
 	mockgen -destination pkg/mocks/mock_docker_cli.go -package mocks github.com/docker/cli/cli/command Cli
 	mockgen -destination pkg/mocks/mock_docker_api.go -package mocks github.com/docker/docker/client APIClient
+	mockgen -destination pkg/mocks/mock_docker_compose_api.go -package mocks -source=./pkg/api/api.go Service
 
 .PHONY: e2e
 e2e: e2e-compose e2e-compose-standalone ## Run end to end local tests in both modes. Set E2E_TEST=TestName to run a single test

cmd/compose/ps.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/docker/compose/v2/cmd/formatter"
 	"github.com/docker/compose/v2/pkg/utils"
+	"github.com/docker/docker/api/types"
 
 	formatter2 "github.com/docker/cli/cli/command/formatter"
 	"github.com/pkg/errors"
@@ -146,7 +147,7 @@ SERVICES:
 func writter(containers []api.ContainerSummary) func(w io.Writer) {
 	return func(w io.Writer) {
 		for _, container := range containers {
-			ports := DisplayablePorts(container)
+			ports := displayablePorts(container)
 			status := container.State
 			if status == "running" && container.Health != "" {
 				status = fmt.Sprintf("%s (%s)", container.State, container.Health)
@@ -178,72 +179,20 @@ func hasStatus(c api.ContainerSummary, statuses []string) bool {
 	return false
 }
 
-type portRange struct {
-	pStart   int
-	pEnd     int
-	tStart   int
-	tEnd     int
-	IP       string
-	protocol string
-}
-
-func (pr portRange) String() string {
-	var (
-		pub string
-		tgt string
-	)
-
-	if pr.pEnd > pr.pStart {
-		pub = fmt.Sprintf("%s:%d-%d->", pr.IP, pr.pStart, pr.pEnd)
-	} else if pr.pStart > 0 {
-		pub = fmt.Sprintf("%s:%d->", pr.IP, pr.pStart)
-	}
-	if pr.tEnd > pr.tStart {
-		tgt = fmt.Sprintf("%d-%d", pr.tStart, pr.tEnd)
-	} else {
-		tgt = fmt.Sprintf("%d", pr.tStart)
-	}
-	return fmt.Sprintf("%s%s/%s", pub, tgt, pr.protocol)
-}
-
-// DisplayablePorts is copy pasted from https://github.com/docker/cli/pull/581/files
-func DisplayablePorts(c api.ContainerSummary) string {
+func displayablePorts(c api.ContainerSummary) string {
 	if c.Publishers == nil {
 		return ""
 	}
 
-	sort.Sort(c.Publishers)
-
-	pr := portRange{}
-	ports := []string{}
-	for _, p := range c.Publishers {
-		prIsRange := pr.tEnd != pr.tStart
-		tOverlaps := p.TargetPort <= pr.tEnd
-
-		// Start a new port-range if:
-		// - the protocol is different from the current port-range
-		// - published or target port are not consecutive to the current port-range
-		// - the current port-range is a _range_, and the target port overlaps with the current range's target-ports
-		if p.Protocol != pr.protocol || p.URL != pr.IP || p.PublishedPort-pr.pEnd > 1 || p.TargetPort-pr.tEnd > 1 || prIsRange && tOverlaps {
-			// start a new port-range, and print the previous port-range (if any)
-			if pr.pStart > 0 {
-				ports = append(ports, pr.String())
-			}
-			pr = portRange{
-				pStart:   p.PublishedPort,
-				pEnd:     p.PublishedPort,
-				tStart:   p.TargetPort,
-				tEnd:     p.TargetPort,
-				protocol: p.Protocol,
-				IP:       p.URL,
-			}
-			continue
+	ports := make([]types.Port, len(c.Publishers))
+	for i, pub := range c.Publishers {
+		ports[i] = types.Port{
+			IP:          pub.URL,
+			PrivatePort: uint16(pub.TargetPort),
+			PublicPort:  uint16(pub.PublishedPort),
+			Type:        pub.Protocol,
 		}
-		pr.pEnd = p.PublishedPort
-		pr.tEnd = p.TargetPort
-	}
-	if pr.tStart > 0 {
-		ports = append(ports, pr.String())
 	}
-	return strings.Join(ports, ", ")
+
+	return formatter2.DisplayablePorts(ports)
 }

cmd/compose/ps_test.go

@@ -0,0 +1,84 @@
+/*
+   Copyright 2020 Docker Compose CLI authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package compose
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/docker/compose/v2/pkg/api"
+	"github.com/docker/compose/v2/pkg/mocks"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPsPretty(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"))
+	if err != nil {
+		t.Fatal("could not create output file")
+	}
+	defer func() { _ = f.Close() }()
+
+	os.Stdout = f
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	backend := mocks.NewMockService(ctrl)
+	backend.EXPECT().
+		Ps(gomock.Eq(ctx), gomock.Any(), gomock.Any()).
+		DoAndReturn(func(ctx context.Context, projectName string, options api.PsOptions) ([]api.ContainerSummary, error) {
+			return []api.ContainerSummary{
+				{
+					ID:   "abc123",
+					Name: "ABC",
+					Publishers: api.PortPublishers{
+						{
+							TargetPort:    8080,
+							PublishedPort: 8080,
+							Protocol:      "tcp",
+						},
+						{
+							TargetPort:    8443,
+							PublishedPort: 8443,
+							Protocol:      "tcp",
+						},
+					},
+				},
+			}, nil
+		}).AnyTimes()
+
+	opts := psOptions{projectOptions: &projectOptions{ProjectName: "test"}}
+	err = runPs(ctx, backend, nil, opts)
+	assert.NoError(t, err)
+
+	_, err = f.Seek(0, 0)
+	assert.NoError(t, err)
+
+	output := make([]byte, 256)
+	_, err = f.Read(output)
+	assert.NoError(t, err)
+
+	assert.Contains(t, string(output), "8080/tcp, 8443/tcp")
+}