chore: Add test coverage for successors

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

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

@@ -17,14 +17,13 @@ package status
 
 import (
 	"context"
+	"oras.land/oras/internal/graph"
 	"sync"
 
-	"oras.land/oras/cmd/oras/internal/output"
-	"oras.land/oras/internal/descriptor"
-
 	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.
@@ -66,7 +65,7 @@ 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])
-		successors, err := descriptor.Successors(ctx, desc, fetcher, DeduplicatedFilter(committed))
+		successors, err := graph.FilteredSuccessors(ctx, desc, fetcher, DeduplicatedFilter(committed))
 		if err != nil {
 			return err
 		}

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

@@ -17,15 +17,14 @@ package status
 
 import (
 	"context"
+	"oras.land/oras/internal/graph"
 	"os"
 	"sync"
 
-	"oras.land/oras/cmd/oras/internal/display/status/track"
-	"oras.land/oras/internal/descriptor"
-
 	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/display/status/track"
 )
 
 // TTYPushHandler handles TTY status output for push command.
@@ -70,7 +69,7 @@ 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])
-		successors, err := descriptor.Successors(ctx, desc, fetcher, DeduplicatedFilter(committed))
+		successors, err := graph.FilteredSuccessors(ctx, desc, fetcher, DeduplicatedFilter(committed))
 		if err != nil {
 			return err
 		}

cmd/oras/internal/output/print.go

@@ -53,8 +53,9 @@ func (p *Printer) Println(a ...any) error {
 	if err != nil {
 		err = fmt.Errorf("display output error: %w", err)
 		_, _ = fmt.Fprint(p.err, err)
+		return err
 	}
-	return err
+	return nil
 }
 
 // Printf prints objects concurrent-safely with newline.

cmd/oras/root/cp.go

@@ -39,7 +39,6 @@ import (
 	oerrors "oras.land/oras/cmd/oras/internal/errors"
 	"oras.land/oras/cmd/oras/internal/option"
 	"oras.land/oras/cmd/oras/internal/output"
-	"oras.land/oras/internal/descriptor"
 	"oras.land/oras/internal/docker"
 	"oras.land/oras/internal/graph"
 	"oras.land/oras/internal/listener"
@@ -190,7 +189,7 @@ func doCopy(ctx context.Context, printer *output.Printer, src oras.ReadOnlyGraph
 		}
 		extendedCopyOptions.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
 			committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
-			successors, err := descriptor.Successors(ctx, desc, dst, status.DeduplicatedFilter(committed))
+			successors, err := graph.FilteredSuccessors(ctx, desc, dst, status.DeduplicatedFilter(committed))
 			if err != nil {
 				return err
 			}
@@ -219,7 +218,7 @@ func doCopy(ctx context.Context, printer *output.Printer, src oras.ReadOnlyGraph
 		}
 		extendedCopyOptions.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
 			committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
-			successors, err := descriptor.Successors(ctx, desc, tracked, status.DeduplicatedFilter(committed))
+			successors, err := graph.FilteredSuccessors(ctx, desc, tracked, status.DeduplicatedFilter(committed))
 			if err != nil {
 				return err
 			}

internal/descriptor/descriptor.go

@@ -16,12 +16,8 @@ limitations under the License.
 package descriptor
 
 import (
-	"context"
-
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
-	"oras.land/oras-go/v2/content"
-
 	"oras.land/oras/internal/docker"
 )
 
@@ -55,19 +51,3 @@ func GetTitleOrMediaType(desc ocispec.Descriptor) (name string, isTitle bool) {
 func GenerateContentKey(desc ocispec.Descriptor) string {
 	return desc.Digest.String() + desc.Annotations[ocispec.AnnotationTitle]
 }
-
-// Successors fetches successors and returns filtered ones.
-func Successors(ctx context.Context, desc ocispec.Descriptor, fetcher content.Fetcher, filter func(ocispec.Descriptor) bool) ([]ocispec.Descriptor, error) {
-	allSuccessors, err := content.Successors(ctx, fetcher, desc)
-	if err != nil {
-		return nil, err
-	}
-
-	var successors []ocispec.Descriptor
-	for _, s := range allSuccessors {
-		if filter(s) {
-			successors = append(successors, s)
-		}
-	}
-	return successors, nil
-}

internal/descriptor/descriptor_test.go

@@ -31,13 +31,13 @@ var (
 	}
 
 	imageDesc = ocispec.Descriptor{
-		MediaType: "application/vnd.oci.image.manifest.v1+json",
+		MediaType: ocispec.MediaTypeImageManifest,
 		Digest:    "sha256:2e0e0fe1fb3edbcdddad941c90d2b51e25a6bcd593e82545441a216de7bfa834",
 		Size:      474,
 	}
 
 	titledDesc = ocispec.Descriptor{
-		MediaType:   "application/vnd.oci.image.manifest.v1+json",
+		MediaType:   ocispec.MediaTypeImageManifest,
 		Digest:      "sha256:2e0e0fe1fb3edbcdddad941c90d2b51e25a6bcd593e82545441a216de7bfa834",
 		Size:        474,
 		Annotations: map[string]string{"org.opencontainers.image.title": "shaboozey"},
@@ -88,6 +88,6 @@ func TestDescriptor_GenerateContentKey(t *testing.T) {
 	expected := "sha256:2e0e0fe1fb3edbcdddad941c90d2b51e25a6bcd593e82545441a216de7bfa834shaboozey"
 	got := descriptor.GenerateContentKey(titledDesc)
 	if expected != got {
-		t.Fatalf("GetTitleOrMediaType() got %v, want %v", got, expected)
+		t.Fatalf("GenerateContentKey got %v, want %v", got, expected)
 	}
 }

internal/graph/graph.go

@@ -130,3 +130,19 @@ func FindPredecessors(ctx context.Context, src oras.ReadOnlyGraphTarget, descs [
 	}
 	return referrers, nil
 }
+
+// FilteredSuccessors fetches successors and returns filtered ones.
+func FilteredSuccessors(ctx context.Context, desc ocispec.Descriptor, fetcher content.Fetcher, filter func(ocispec.Descriptor) bool) ([]ocispec.Descriptor, error) {
+	allSuccessors, err := content.Successors(ctx, fetcher, desc)
+	if err != nil {
+		return nil, err
+	}
+
+	var successors []ocispec.Descriptor
+	for _, s := range allSuccessors {
+		if filter(s) {
+			successors = append(successors, s)
+		}
+	}
+	return successors, nil
+}

internal/graph/graph_test.go

@@ -19,32 +19,37 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"github.com/opencontainers/go-digest"
+	"oras.land/oras-go/v2/content/memory"
 	"reflect"
 	"testing"
 
-	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2/content"
-	"oras.land/oras-go/v2/content/memory"
 	"oras.land/oras/internal/docker"
 )
 
-type fetcher struct {
+type contentFetcher struct {
 	content.Fetcher
 }
 
-func TestSuccessors(t *testing.T) {
+func newTestFetcher(t *testing.T) (subject, config, ociImage, dockerImage, index ocispec.Descriptor, fetcher content.Fetcher) {
 	var blobs [][]byte
-	var descs []ocispec.Descriptor
-	appendBlob := func(mediaType string, blob []byte) {
+	ctx := context.Background()
+	memoryStorage := memory.New()
+	appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor {
 		blobs = append(blobs, blob)
-		descs = append(descs, ocispec.Descriptor{
+		desc := ocispec.Descriptor{
 			MediaType: mediaType,
 			Digest:    digest.FromBytes(blob),
 			Size:      int64(len(blob)),
-		})
+		}
+		if err := memoryStorage.Push(ctx, desc, bytes.NewReader(blob)); err != nil {
+			t.Errorf("Error pushing %v\n", err)
+		}
+		return desc
 	}
-	generateImage := func(subject *ocispec.Descriptor, mediaType string, config ocispec.Descriptor, layers ...ocispec.Descriptor) {
+	generateImage := func(subject *ocispec.Descriptor, mediaType string, config ocispec.Descriptor, layers ...ocispec.Descriptor) ocispec.Descriptor {
 		manifest := ocispec.Manifest{
 			MediaType: mediaType,
 			Subject:   subject,
@@ -55,40 +60,32 @@ func TestSuccessors(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		appendBlob(mediaType, manifestJSON)
+		return appendBlob(mediaType, manifestJSON)
 	}
-	generateIndex := func(manifests ...ocispec.Descriptor) {
+	generateIndex := func(manifests ...ocispec.Descriptor) ocispec.Descriptor {
 		index := ocispec.Index{
 			Manifests: manifests,
 		}
-		manifestJSON, err := json.Marshal(index)
+		indexJSON, err := json.Marshal(index)
 		if err != nil {
 			t.Fatal(err)
 		}
-		appendBlob(ocispec.MediaTypeImageIndex, manifestJSON)
-	}
-	const (
-		subject = iota
-		config
-		ociImage
-		dockerImage
-		index
-	)
-	appendBlob(ocispec.MediaTypeImageLayer, []byte("blob"))
-	imageType := "test.image"
-	appendBlob(imageType, []byte("config content"))
-	generateImage(&descs[subject], ocispec.MediaTypeImageManifest, descs[config])
-	generateImage(&descs[subject], docker.MediaTypeManifest, descs[config])
-	generateIndex(descs[subject])
-	memory := memory.New()
-	ctx := context.Background()
-	for i := range descs {
-		if err := memory.Push(ctx, descs[i], bytes.NewReader(blobs[i])); err != nil {
-			t.Errorf("Error pushing %v\n", err)
-		}
+		return appendBlob(ocispec.MediaTypeImageIndex, indexJSON)
 	}
-	fetcher := &fetcher{Fetcher: memory}
 
+	subject = appendBlob(ocispec.MediaTypeImageLayer, []byte("blob"))
+	imageType := "test.image"
+	config = appendBlob(imageType, []byte("config content"))
+	ociImage = generateImage(&subject, ocispec.MediaTypeImageManifest, config)
+	dockerImage = generateImage(&subject, docker.MediaTypeManifest, config)
+	index = generateIndex(subject)
+
+	return subject, config, ociImage, dockerImage, index, &contentFetcher{Fetcher: memoryStorage}
+}
+
+func TestSuccessors(t *testing.T) {
+	subject, config, ociImage, dockerImage, index, fetcher := newTestFetcher(t)
+	ctx := context.Background()
 	type args struct {
 		ctx     context.Context
 		fetcher content.Fetcher
@@ -104,26 +101,69 @@ func TestSuccessors(t *testing.T) {
 	}{
 		{"should failed to get non-existent OCI image", args{ctx, fetcher, ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest}}, nil, nil, nil, true},
 		{"should failed to get non-existent docker image", args{ctx, fetcher, ocispec.Descriptor{MediaType: docker.MediaTypeManifest}}, nil, nil, nil, true},
-		{"should get success of a docker image", args{ctx, fetcher, descs[dockerImage]}, nil, &descs[subject], &descs[config], false},
-		{"should get success of an OCI image", args{ctx, fetcher, descs[ociImage]}, nil, &descs[subject], &descs[config], false},
-		{"should get success of an index", args{ctx, fetcher, descs[index]}, []ocispec.Descriptor{descs[subject]}, nil, nil, false},
+		{"should get success of a docker image", args{ctx, fetcher, dockerImage}, nil, &subject, &config, false},
+		{"should get success of an OCI image", args{ctx, fetcher, ociImage}, nil, &subject, &config, false},
+		{"should get success of an index", args{ctx, fetcher, index}, []ocispec.Descriptor{subject}, nil, nil, false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			gotNodes, gotSubject, gotConfig, err := Successors(tt.args.ctx, tt.args.fetcher, tt.args.node)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("Successors() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("FilteredSuccessors() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			if !reflect.DeepEqual(gotNodes, tt.wantNodes) {
-				t.Errorf("Successors() gotNodes = %v, want %v", gotNodes, tt.wantNodes)
+				t.Errorf("FilteredSuccessors() gotNodes = %v, want %v", gotNodes, tt.wantNodes)
 			}
 			if !reflect.DeepEqual(gotSubject, tt.wantSubject) {
-				t.Errorf("Successors() gotSubject = %v, want %v", gotSubject, tt.wantSubject)
+				t.Errorf("FilteredSuccessors() gotSubject = %v, want %v", gotSubject, tt.wantSubject)
 			}
 			if !reflect.DeepEqual(gotConfig, tt.wantConfig) {
-				t.Errorf("Successors() gotConfig = %v, want %v", gotConfig, tt.wantConfig)
+				t.Errorf("FilteredSuccessors() gotConfig = %v, want %v", gotConfig, tt.wantConfig)
 			}
 		})
 	}
 }
+
+func TestDescriptor_GetSuccessors(t *testing.T) {
+	subject, config, ociImage, _, _, fetcher := newTestFetcher(t)
+
+	allFilter := func(ocispec.Descriptor) bool {
+		return true
+	}
+	got, err := FilteredSuccessors(context.Background(), ociImage, fetcher, allFilter)
+	if nil != err {
+		t.Errorf("FilteredSuccessors unexpected error %v", err)
+	}
+	if len(got) != 2 {
+		t.Errorf("Expected 2 successors got %v", len(got))
+	}
+	if subject.Digest != got[0].Digest {
+		t.Errorf("FilteredSuccessors got %v, want %v", got[0], subject)
+	}
+	if config.Digest != got[1].Digest {
+		t.Errorf("FilteredSuccessors got %v, want %v", got[1], subject)
+	}
+
+	noConfig := func(desc ocispec.Descriptor) bool {
+		return desc.Digest != config.Digest
+	}
+	got, err = FilteredSuccessors(context.Background(), ociImage, fetcher, noConfig)
+	if nil != err {
+		t.Errorf("FilteredSuccessors unexpected error %v", err)
+	}
+	if len(got) != 1 {
+		t.Errorf("Expected 1 successors got %v", len(got))
+	}
+	if subject.Digest != got[0].Digest {
+		t.Errorf("FilteredSuccessors got %v, want %v", got[0], subject)
+	}
+
+	got, err = FilteredSuccessors(context.Background(), ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest}, fetcher, allFilter)
+	if nil == err {
+		t.Error("FilteredSuccessors expected error")
+	}
+	if got != nil {
+		t.Errorf("FilteredSuccessors unexpected %v", got)
+	}
+}