refactor: Successor getting with separation of concerns

Signed-off-by: Terry Howe <terrylhowe@gmail.com>

.gitignore

@@ -36,6 +36,7 @@ debug
 
 # Custom
 coverage.txt
+coverage-all.txt
 test/e2e/coverage.txt
 **/covcounters.*
 **/covmeta.*

Makefile

@@ -33,7 +33,8 @@ LDFLAGS += -X $(PROJECT_PKG)/internal/version.GitTreeState=${GIT_DIRTY}
 
 .PHONY: test
 test: tidy vendor check-encoding  ## tidy and run tests
-	$(GO_EXE) test -race -v -coverprofile=coverage.txt -covermode=atomic -coverpkg=./... ./...
+	$(GO_EXE) test -race -v -coverprofile=coverage-all.txt -covermode=atomic -coverpkg=./... ./...
+	@grep -v testutil coverage-all.txt >coverage.txt; rm -f coverage-all.txt
 
 .PHONY: teste2e
 teste2e:  ## run end to end tests

cmd/oras/internal/display/status/progress/manager_test.go

@@ -22,7 +22,7 @@ import (
 	"testing"
 
 	"oras.land/oras/cmd/oras/internal/display/status/console"
-	"oras.land/oras/cmd/oras/internal/display/status/console/testutils"
+	"oras.land/oras/internal/testutils"
 )
 
 func Test_manager_render(t *testing.T) {

cmd/oras/internal/display/status/progress/status_test.go

@@ -23,8 +23,8 @@ import (
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras/cmd/oras/internal/display/status/console"
-	"oras.land/oras/cmd/oras/internal/display/status/console/testutils"
 	"oras.land/oras/cmd/oras/internal/display/status/progress/humanize"
+	"oras.land/oras/internal/testutils"
 )
 
 func Test_status_String(t *testing.T) {

cmd/oras/internal/display/status/text.go

@@ -17,13 +17,13 @@ package status
 
 import (
 	"context"
+	"oras.land/oras/internal/graph"
 	"sync"
 
-	"oras.land/oras/cmd/oras/internal/output"
-
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2"
 	"oras.land/oras-go/v2/content"
+	"oras.land/oras/cmd/oras/internal/output"
 )
 
 // TextPushHandler handles text status output for push events.
@@ -65,9 +65,15 @@ func (ph *TextPushHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetche
 	}
 	opts.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
 		committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
-		if err := output.PrintSuccessorStatus(ctx, desc, fetcher, committed, ph.printer.StatusPrinter(PushPromptSkipped)); err != nil {
+		successors, err := graph.FilteredSuccessors(ctx, desc, fetcher, DeduplicatedFilter(committed))
+		if err != nil {
 			return err
 		}
+		for _, successor := range successors {
+			if err = ph.printer.PrintStatus(successor, PushPromptSkipped); err != nil {
+				return err
+			}
+		}
 		return ph.printer.PrintStatus(desc, PushPromptUploaded)
 	}
 }
@@ -149,9 +155,15 @@ func (ch *TextCopyHandler) PreCopy(_ context.Context, desc ocispec.Descriptor) e
 // PostCopy implements PostCopy of CopyHandler.
 func (ch *TextCopyHandler) PostCopy(ctx context.Context, desc ocispec.Descriptor) error {
 	ch.committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
-	if err := output.PrintSuccessorStatus(ctx, desc, ch.fetcher, ch.committed, ch.printer.StatusPrinter(copyPromptSkipped)); err != nil {
+	successors, err := graph.FilteredSuccessors(ctx, desc, ch.fetcher, DeduplicatedFilter(ch.committed))
+	if err != nil {
 		return err
 	}
+	for _, successor := range successors {
+		if err = ch.printer.PrintStatus(successor, copyPromptSkipped); err != nil {
+			return err
+		}
+	}
 	return ch.printer.PrintStatus(desc, copyPromptCopied)
 }
 

cmd/oras/internal/display/status/text_test.go

@@ -0,0 +1,131 @@
+/*
+Copyright The ORAS 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 status
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"github.com/opencontainers/go-digest"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"oras.land/oras-go/v2/content/memory"
+	"oras.land/oras/cmd/oras/internal/output"
+	"oras.land/oras/internal/testutils"
+	"os"
+	"strings"
+	"testing"
+)
+
+var (
+	ctx          context.Context
+	builder      *strings.Builder
+	printer      *output.Printer
+	bogus        ocispec.Descriptor
+	memStore     *memory.Store
+	memDesc      ocispec.Descriptor
+	manifestDesc ocispec.Descriptor
+)
+
+func TestMain(m *testing.M) {
+	// memory store for testing
+	memStore = memory.New()
+	content := []byte("test")
+	r := bytes.NewReader(content)
+	memDesc = ocispec.Descriptor{
+		MediaType: "application/octet-stream",
+		Digest:    digest.FromBytes(content),
+		Size:      int64(len(content)),
+	}
+	if err := memStore.Push(context.Background(), memDesc, r); err != nil {
+		fmt.Println("Setup failed:", err)
+		os.Exit(1)
+	}
+	if err := memStore.Tag(context.Background(), memDesc, memDesc.Digest.String()); err != nil {
+		fmt.Println("Setup failed:", err)
+		os.Exit(1)
+	}
+
+	layer1Desc := memDesc
+	layer1Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer1"}
+	layer2Desc := memDesc
+	layer2Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer2"}
+	manifest := ocispec.Manifest{
+		MediaType: ocispec.MediaTypeImageManifest,
+		Layers:    []ocispec.Descriptor{layer1Desc, layer2Desc},
+		Config:    memDesc,
+	}
+	manifestContent, err := json.Marshal(&manifest)
+	if err != nil {
+		fmt.Println("Setup failed:", err)
+		os.Exit(1)
+	}
+	manifestDesc = ocispec.Descriptor{
+		MediaType: manifest.MediaType,
+		Size:      int64(len(manifestContent)),
+		Digest:    digest.FromBytes(manifestContent),
+	}
+	if err := memStore.Push(context.Background(), manifestDesc, strings.NewReader(string(manifestContent))); err != nil {
+		fmt.Println("Setup failed:", err)
+		os.Exit(1)
+	}
+	if err := memStore.Tag(context.Background(), memDesc, memDesc.Digest.String()); err != nil {
+		fmt.Println("Setup failed:", err)
+		os.Exit(1)
+	}
+
+	ctx = context.Background()
+	builder = &strings.Builder{}
+	printer = output.NewPrinter(builder, os.Stderr, false)
+	bogus = ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest}
+	m.Run()
+}
+
+func TestTextCopyHandler_OnMounted(t *testing.T) {
+	fetcher := testutils.NewMockFetcher(t)
+	ch := NewTextCopyHandler(printer, fetcher.Fetcher)
+	if ch.OnMounted(ctx, fetcher.OciImage) != nil {
+		t.Error("OnMounted() should not return an error")
+	}
+
+}
+
+func TestTextCopyHandler_OnCopySkipped(t *testing.T) {
+	fetcher := testutils.NewMockFetcher(t)
+	ch := NewTextCopyHandler(printer, fetcher.Fetcher)
+	if ch.OnCopySkipped(ctx, fetcher.OciImage) != nil {
+		t.Error("OnCopySkipped() should not return an error")
+	}
+}
+
+func TestTextCopyHandler_PostCopy(t *testing.T) {
+	fetcher := testutils.NewMockFetcher(t)
+	ch := NewTextCopyHandler(printer, fetcher.Fetcher)
+	if ch.PostCopy(ctx, fetcher.OciImage) != nil {
+		t.Error("PostCopy() should not return an error")
+	}
+	if ch.PostCopy(ctx, bogus) == nil {
+		t.Error("PostCopy() should return an error")
+	}
+}
+
+func TestTextCopyHandler_PreCopy(t *testing.T) {
+	fetcher := testutils.NewMockFetcher(t)
+	ch := NewTextCopyHandler(printer, fetcher.Fetcher)
+	if ch.PreCopy(ctx, fetcher.OciImage) != nil {
+		t.Error("PreCopy() should not return an error")
+	}
+}

cmd/oras/internal/display/status/track/target_test.go

@@ -28,7 +28,7 @@ import (
 	"oras.land/oras-go/v2"
 	"oras.land/oras-go/v2/content/memory"
 	"oras.land/oras-go/v2/registry/remote"
-	"oras.land/oras/cmd/oras/internal/display/status/console/testutils"
+	"oras.land/oras/internal/testutils"
 )
 
 type testReferenceGraphTarget struct {

cmd/oras/internal/display/status/tty.go

@@ -17,11 +17,10 @@ package status
 
 import (
 	"context"
+	"oras.land/oras/internal/graph"
 	"os"
 	"sync"
 
-	"oras.land/oras/cmd/oras/internal/output"
-
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2"
 	"oras.land/oras-go/v2/content"
@@ -70,9 +69,17 @@ func (ph *TTYPushHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetcher
 	}
 	opts.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
 		committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
-		return output.PrintSuccessorStatus(ctx, desc, fetcher, committed, func(d ocispec.Descriptor) error {
-			return ph.tracked.Prompt(d, PushPromptSkipped)
-		})
+		successors, err := graph.FilteredSuccessors(ctx, desc, fetcher, DeduplicatedFilter(committed))
+		if err != nil {
+			return err
+		}
+		for _, successor := range successors {
+			err = ph.tracked.Prompt(successor, PushPromptSkipped)
+			if err != nil {
+				return err
+			}
+		}
+		return nil
 	}
 }
 

cmd/oras/internal/display/status/tty_console_test.go

@@ -21,8 +21,8 @@ import (
 	"context"
 	"oras.land/oras-go/v2"
 	"oras.land/oras-go/v2/content/memory"
-	"oras.land/oras/cmd/oras/internal/display/status/console/testutils"
 	"oras.land/oras/cmd/oras/internal/display/status/track"
+	"oras.land/oras/internal/testutils"
 	"testing"
 )
 

cmd/oras/internal/display/status/tty_test.go

@@ -16,74 +16,12 @@ limitations under the License.
 package status
 
 import (
-	"bytes"
-	"context"
-	"encoding/json"
-	"fmt"
 	"os"
-	"strings"
 	"testing"
 
-	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
-	"oras.land/oras-go/v2/content/memory"
 )
 
-var (
-	memStore     *memory.Store
-	memDesc      ocispec.Descriptor
-	manifestDesc ocispec.Descriptor
-)
-
-func TestMain(m *testing.M) {
-	// memory store for testing
-	memStore = memory.New()
-	content := []byte("test")
-	r := bytes.NewReader(content)
-	memDesc = ocispec.Descriptor{
-		MediaType: "application/octet-stream",
-		Digest:    digest.FromBytes(content),
-		Size:      int64(len(content)),
-	}
-	if err := memStore.Push(context.Background(), memDesc, r); err != nil {
-		fmt.Println("Setup failed:", err)
-		os.Exit(1)
-	}
-	if err := memStore.Tag(context.Background(), memDesc, memDesc.Digest.String()); err != nil {
-		fmt.Println("Setup failed:", err)
-		os.Exit(1)
-	}
-
-	layer1Desc := memDesc
-	layer1Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer1"}
-	layer2Desc := memDesc
-	layer2Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer2"}
-	manifest := ocispec.Manifest{
-		MediaType: ocispec.MediaTypeImageManifest,
-		Layers:    []ocispec.Descriptor{layer1Desc, layer2Desc},
-		Config:    memDesc,
-	}
-	manifestContent, err := json.Marshal(&manifest)
-	if err != nil {
-		fmt.Println("Setup failed:", err)
-		os.Exit(1)
-	}
-	manifestDesc = ocispec.Descriptor{
-		MediaType: manifest.MediaType,
-		Size:      int64(len(manifestContent)),
-		Digest:    digest.FromBytes(manifestContent),
-	}
-	if err := memStore.Push(context.Background(), manifestDesc, strings.NewReader(string(manifestContent))); err != nil {
-		fmt.Println("Setup failed:", err)
-		os.Exit(1)
-	}
-	if err := memStore.Tag(context.Background(), memDesc, memDesc.Digest.String()); err != nil {
-		fmt.Println("Setup failed:", err)
-		os.Exit(1)
-	}
-	m.Run()
-}
-
 func TestTTYPushHandler_OnFileLoading(t *testing.T) {
 	ph := NewTTYPushHandler(os.Stdout)
 	if ph.OnFileLoading("test") != nil {

cmd/oras/internal/display/status/utils.go

@@ -15,6 +15,12 @@ limitations under the License.
 
 package status
 
+import (
+	"sync"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+)
+
 // Prompts for pull events.
 const (
 	PullPromptDownloading = "Downloading"
@@ -41,3 +47,13 @@ const (
 	copyPromptSkipped = "Skipped"
 	copyPromptMounted = "Mounted"
 )
+
+// DeduplicatedFilter filters out deduplicated descriptors.
+func DeduplicatedFilter(committed *sync.Map) func(desc ocispec.Descriptor) bool {
+	return func(desc ocispec.Descriptor) bool {
+		name := desc.Annotations[ocispec.AnnotationTitle]
+		v, ok := committed.Load(desc.Digest.String())
+		// committed but not printed == deduplicated
+		return ok && v != name
+	}
+}

cmd/oras/internal/option/common_unix_test.go

@@ -20,7 +20,7 @@ package option
 import (
 	"testing"
 
-	"oras.land/oras/cmd/oras/internal/display/status/console/testutils"
+	"oras.land/oras/internal/testutils"
 )
 
 func TestCommon_parseTTY(t *testing.T) {