refactor: clean status prompts (#1409)

Signed-off-by: Billy Zha <[email protected]>
oras-project · Jun 17, 2024 · 2aa005c · 2aa005c
1 parent e0b7cdd
commit 2aa005c
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 24 deletions.
diff --git a/cmd/oras/internal/display/status/text.go b/cmd/oras/internal/display/status/text.go
@@ -18,9 +18,10 @@ package status
 import (
 	"context"
 	"io"
-	"oras.land/oras/cmd/oras/internal/output"
 	"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"
@@ -58,17 +59,17 @@ func (ph *TextPushHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetche
 	committed := &sync.Map{}
 	opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error {
 		committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
-		return ph.printer.PrintStatus(desc, promptExists)
+		return ph.printer.PrintStatus(desc, PushPromptExists)
 	}
 	opts.PreCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
-		return ph.printer.PrintStatus(desc, promptUploading)
+		return ph.printer.PrintStatus(desc, PushPromptUploading)
 	}
 	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(promptSkipped)); err != nil {
+		if err := output.PrintSuccessorStatus(ctx, desc, fetcher, committed, ph.printer.StatusPrinter(PushPromptSkipped)); err != nil {
 			return err
 		}
-		return ph.printer.PrintStatus(desc, promptUploaded)
+		return ph.printer.PrintStatus(desc, PushPromptUploaded)
 	}
 }
 

diff --git a/cmd/oras/internal/display/status/tty.go b/cmd/oras/internal/display/status/tty.go
@@ -17,10 +17,11 @@ package status
 
 import (
 	"context"
-	"oras.land/oras/cmd/oras/internal/output"
 	"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"
@@ -52,7 +53,7 @@ func (ph *TTYPushHandler) OnEmptyArtifact() error {
 
 // TrackTarget returns a tracked target.
 func (ph *TTYPushHandler) TrackTarget(gt oras.GraphTarget) (oras.GraphTarget, StopTrackTargetFunc, error) {
-	tracked, err := track.NewTarget(gt, promptUploading, promptUploaded, ph.tty)
+	tracked, err := track.NewTarget(gt, PushPromptUploading, PushPromptUploaded, ph.tty)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -65,12 +66,12 @@ func (ph *TTYPushHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetcher
 	committed := &sync.Map{}
 	opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error {
 		committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
-		return ph.tracked.Prompt(desc, promptExists)
+		return ph.tracked.Prompt(desc, PushPromptExists)
 	}
 	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, promptSkipped)
+			return ph.tracked.Prompt(d, PushPromptSkipped)
 		})
 	}
 }

diff --git a/cmd/oras/internal/display/status/tty_test.go b/cmd/oras/internal/display/status/tty_test.go
@@ -20,8 +20,10 @@ package status
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/opencontainers/go-digest"
@@ -33,8 +35,9 @@ import (
 )
 
 var (
-	memStore *memory.Store
-	memDesc  ocispec.Descriptor
+	memStore     *memory.Store
+	memDesc      ocispec.Descriptor
+	manifestDesc ocispec.Descriptor
 )
 
 func TestMain(m *testing.M) {
@@ -55,6 +58,34 @@ func TestMain(m *testing.M) {
 		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()
 }
 
@@ -84,15 +115,22 @@ func TestTTYPushHandler_TrackTarget(t *testing.T) {
 	// test
 	_, fn, err := ph.TrackTarget(store)
 	if err != nil {
-		t.Error("TrackTarget() should not return an error")
+		t.Fatal("TrackTarget() should not return an error")
 	}
 	defer func() {
 		if err := fn(); err != nil {
 			t.Fatal(err)
 		}
 	}()
 	if ttyPushHandler, ok := ph.(*TTYPushHandler); !ok {
-		t.Errorf("TrackTarget() should return a *TTYPushHandler, got %T", ttyPushHandler)
+		t.Fatalf("TrackTarget() should return a *TTYPushHandler, got %T", ttyPushHandler)
+	}
+}
+
+func TestTTYPushHandler_TrackTarget_invalidTTY(t *testing.T) {
+	ph := NewTTYPushHandler(os.Stdin)
+	if _, _, err := ph.TrackTarget(nil); err == nil {
+		t.Error("TrackTarget() should return an error for non-tty file")
 	}
 }
 
@@ -106,24 +144,24 @@ func TestTTYPushHandler_UpdateCopyOptions(t *testing.T) {
 	ph := NewTTYPushHandler(slave)
 	gt, _, err := ph.TrackTarget(memory.New())
 	if err != nil {
-		t.Errorf("TrackTarget() should not return an error: %v", err)
+		t.Fatalf("TrackTarget() should not return an error: %v", err)
 	}
 	// test
 	opts := oras.CopyGraphOptions{}
 	ph.UpdateCopyOptions(&opts, memStore)
-	if err := oras.CopyGraph(context.Background(), memStore, gt, memDesc, opts); err != nil {
-		t.Errorf("CopyGraph() should not return an error: %v", err)
+	if err := oras.CopyGraph(context.Background(), memStore, gt, manifestDesc, opts); err != nil {
+		t.Fatalf("CopyGraph() should not return an error: %v", err)
 	}
-	if err := oras.CopyGraph(context.Background(), memStore, gt, memDesc, opts); err != nil {
-		t.Errorf("CopyGraph() should not return an error: %v", err)
+	if err := oras.CopyGraph(context.Background(), memStore, gt, manifestDesc, opts); err != nil {
+		t.Fatalf("CopyGraph() should not return an error: %v", err)
 	}
 	if tracked, ok := gt.(track.GraphTarget); !ok {
-		t.Errorf("TrackTarget() should return a *track.GraphTarget, got %T", tracked)
+		t.Fatalf("TrackTarget() should return a *track.GraphTarget, got %T", tracked)
 	} else {
 		tracked.Close()
 	}
 	// validate
-	if err = testutils.MatchPty(pty, slave, "Exists", memDesc.MediaType, "100.00%", memDesc.Digest.String()); err != nil {
+	if err = testutils.MatchPty(pty, slave, "Exists", manifestDesc.MediaType, "100.00%", manifestDesc.Digest.String()); err != nil {
 		t.Fatal(err)
 	}
 }

diff --git a/cmd/oras/internal/display/status/utils.go b/cmd/oras/internal/display/status/utils.go
@@ -31,8 +31,12 @@ const (
 	PullPromptSkipped     = "Skipped    "
 	PullPromptRestored    = "Restored   "
 	PullPromptDownloaded  = "Downloaded "
-	promptUploaded        = "Uploaded "
-	promptUploading       = "Uploading"
-	promptSkipped         = "Skipped  "
-	promptExists          = "Exists   "
+)
+
+// Prompts for push/attach events.
+const (
+	PushPromptUploaded  = "Uploaded "
+	PushPromptUploading = "Uploading"
+	PushPromptSkipped   = "Skipped  "
+	PushPromptExists    = "Exists   "
 )