From 788d416939995d8d1455886ffffec78c68f41b28 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Thu, 8 Aug 2024 11:53:55 -0600 Subject: [PATCH] Increase test coverage for handlers Signed-off-by: Terry Howe --- cmd/oras/internal/display/handler.go | 12 +- cmd/oras/internal/display/handler_test.go | 7 +- cmd/oras/internal/display/status/discard.go | 19 +- cmd/oras/internal/display/status/interface.go | 5 +- cmd/oras/internal/display/status/text.go | 58 +++-- cmd/oras/internal/display/status/text_test.go | 240 ++++++++++++------ cmd/oras/internal/display/status/tty.go | 52 ++-- cmd/oras/internal/display/status/tty_test.go | 56 +--- cmd/oras/root/attach.go | 13 +- cmd/oras/root/attach_test.go | 10 +- cmd/oras/root/push.go | 16 +- internal/graph/graph_test.go | 4 +- internal/testutils/fetcher.go | 32 ++- 13 files changed, 320 insertions(+), 204 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index d1c8e09fb..2f7428585 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -37,12 +37,12 @@ import ( ) // NewPushHandler returns status and metadata handlers for push command. -func NewPushHandler(printer *output.Printer, format option.Format, tty *os.File) (status.PushHandler, metadata.PushHandler, error) { +func NewPushHandler(printer *output.Printer, format option.Format, tty *os.File, fetcher fetcher.Fetcher) (status.PushHandler, metadata.PushHandler, error) { var statusHandler status.PushHandler if tty != nil { - statusHandler = status.NewTTYPushHandler(tty) + statusHandler = status.NewTTYPushHandler(tty, fetcher) } else if format.Type == option.FormatTypeText.Name { - statusHandler = status.NewTextPushHandler(printer) + statusHandler = status.NewTextPushHandler(printer, fetcher) } else { statusHandler = status.NewDiscardHandler() } @@ -62,12 +62,12 @@ func NewPushHandler(printer *output.Printer, format option.Format, tty *os.File) } // NewAttachHandler returns status and metadata handlers for attach command. -func NewAttachHandler(printer *output.Printer, format option.Format, tty *os.File) (status.AttachHandler, metadata.AttachHandler, error) { +func NewAttachHandler(printer *output.Printer, format option.Format, tty *os.File, fetcher fetcher.Fetcher) (status.AttachHandler, metadata.AttachHandler, error) { var statusHandler status.AttachHandler if tty != nil { - statusHandler = status.NewTTYAttachHandler(tty) + statusHandler = status.NewTTYAttachHandler(tty, fetcher) } else if format.Type == option.FormatTypeText.Name { - statusHandler = status.NewTextAttachHandler(printer) + statusHandler = status.NewTextAttachHandler(printer, fetcher) } else { statusHandler = status.NewDiscardHandler() } diff --git a/cmd/oras/internal/display/handler_test.go b/cmd/oras/internal/display/handler_test.go index 9f73e7248..f1016e6f0 100644 --- a/cmd/oras/internal/display/handler_test.go +++ b/cmd/oras/internal/display/handler_test.go @@ -16,6 +16,7 @@ limitations under the License. package display import ( + "oras.land/oras/internal/testutils" "os" "testing" @@ -24,16 +25,18 @@ import ( ) func TestNewPushHandler(t *testing.T) { + mockFetcher := testutils.NewMockFetcher() printer := output.NewPrinter(os.Stdout, os.Stderr, false) - _, _, err := NewPushHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout) + _, _, err := NewPushHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, mockFetcher.Fetcher) if err != nil { t.Errorf("NewPushHandler() error = %v, want nil", err) } } func TestNewAttachHandler(t *testing.T) { + mockFetcher := testutils.NewMockFetcher() printer := output.NewPrinter(os.Stdout, os.Stderr, false) - _, _, err := NewAttachHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout) + _, _, err := NewAttachHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, mockFetcher.Fetcher) if err != nil { t.Errorf("NewAttachHandler() error = %v, want nil", err) } diff --git a/cmd/oras/internal/display/status/discard.go b/cmd/oras/internal/display/status/discard.go index 91fe6229c..50b45b8f3 100644 --- a/cmd/oras/internal/display/status/discard.go +++ b/cmd/oras/internal/display/status/discard.go @@ -16,9 +16,10 @@ limitations under the License. package status import ( + "context" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2" - "oras.land/oras-go/v2/content" ) func discardStopTrack() error { @@ -48,8 +49,20 @@ func (DiscardHandler) TrackTarget(gt oras.GraphTarget) (oras.GraphTarget, StopTr return gt, discardStopTrack, nil } -// UpdateCopyOptions updates the copy options for the artifact push. -func (DiscardHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetcher content.Fetcher) {} +// OnCopySkipped is called when an object already exists. +func (DiscardHandler) OnCopySkipped(_ context.Context, _ ocispec.Descriptor) error { + return nil +} + +// PreCopy implements PreCopy of CopyHandler. +func (DiscardHandler) PreCopy(_ context.Context, _ ocispec.Descriptor) error { + return nil +} + +// PostCopy implements PostCopy of CopyHandler. +func (DiscardHandler) PostCopy(_ context.Context, _ ocispec.Descriptor) error { + return nil +} // OnNodeDownloading implements PullHandler. func (DiscardHandler) OnNodeDownloading(desc ocispec.Descriptor) error { diff --git a/cmd/oras/internal/display/status/interface.go b/cmd/oras/internal/display/status/interface.go index a254af38e..c2f0bd8b8 100644 --- a/cmd/oras/internal/display/status/interface.go +++ b/cmd/oras/internal/display/status/interface.go @@ -19,7 +19,6 @@ import ( "context" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2" - "oras.land/oras-go/v2/content" ) // StopTrackTargetFunc is the function type to stop tracking a target. @@ -30,7 +29,9 @@ type PushHandler interface { OnFileLoading(name string) error OnEmptyArtifact() error TrackTarget(gt oras.GraphTarget) (oras.GraphTarget, StopTrackTargetFunc, error) - UpdateCopyOptions(opts *oras.CopyGraphOptions, fetcher content.Fetcher) + OnCopySkipped(ctx context.Context, desc ocispec.Descriptor) error + PreCopy(ctx context.Context, desc ocispec.Descriptor) error + PostCopy(ctx context.Context, desc ocispec.Descriptor) error } // AttachHandler handles text status output for attach command. diff --git a/cmd/oras/internal/display/status/text.go b/cmd/oras/internal/display/status/text.go index 5e5b6c20d..2ac3a6235 100644 --- a/cmd/oras/internal/display/status/text.go +++ b/cmd/oras/internal/display/status/text.go @@ -28,14 +28,19 @@ import ( // TextPushHandler handles text status output for push events. type TextPushHandler struct { - printer *output.Printer + printer *output.Printer + committed *sync.Map + fetcher content.Fetcher } // NewTextPushHandler returns a new handler for push command. -func NewTextPushHandler(printer *output.Printer) PushHandler { - return &TextPushHandler{ - printer: printer, +func NewTextPushHandler(printer *output.Printer, fetcher content.Fetcher) PushHandler { + tch := TextPushHandler{ + printer: printer, + fetcher: fetcher, + committed: &sync.Map{}, } + return &tch } // OnFileLoading is called when a file is being prepared for upload. @@ -53,34 +58,35 @@ func (ph *TextPushHandler) TrackTarget(gt oras.GraphTarget) (oras.GraphTarget, S return gt, discardStopTrack, nil } -// UpdateCopyOptions adds status update to the copy options. -func (ph *TextPushHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetcher content.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.printer.PrintStatus(desc, PushPromptExists) - } - opts.PreCopy = func(ctx context.Context, desc ocispec.Descriptor) error { - return ph.printer.PrintStatus(desc, PushPromptUploading) +// OnCopySkipped is called when an object already exists. +func (ph *TextPushHandler) OnCopySkipped(_ context.Context, desc ocispec.Descriptor) error { + ph.committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) + return ph.printer.PrintStatus(desc, PushPromptExists) +} + +// PreCopy implements PreCopy of CopyHandler. +func (ph *TextPushHandler) PreCopy(_ context.Context, desc ocispec.Descriptor) error { + return ph.printer.PrintStatus(desc, PushPromptUploading) +} + +// PostCopy implements PostCopy of CopyHandler. +func (ph *TextPushHandler) PostCopy(ctx context.Context, desc ocispec.Descriptor) error { + ph.committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) + successors, err := graph.FilteredSuccessors(ctx, desc, ph.fetcher, DeduplicatedFilter(ph.committed)) + if err != nil { + return err } - opts.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error { - committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) - successors, err := graph.FilteredSuccessors(ctx, desc, fetcher, DeduplicatedFilter(committed)) - if err != nil { + for _, successor := range successors { + if err = ph.printer.PrintStatus(successor, PushPromptExists); 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) } + return ph.printer.PrintStatus(desc, PushPromptUploaded) } // NewTextAttachHandler returns a new handler for attach command. -func NewTextAttachHandler(printer *output.Printer) AttachHandler { - return NewTextPushHandler(printer) +func NewTextAttachHandler(printer *output.Printer, fetcher content.Fetcher) AttachHandler { + return NewTextPushHandler(printer, fetcher) } // TextPullHandler handles text status output for pull events. @@ -160,7 +166,7 @@ func (ch *TextCopyHandler) PostCopy(ctx context.Context, desc ocispec.Descriptor return err } for _, successor := range successors { - if err = ch.printer.PrintStatus(successor, copyPromptSkipped); err != nil { + if err = ch.printer.PrintStatus(successor, copyPromptExists); err != nil { return err } } diff --git a/cmd/oras/internal/display/status/text_test.go b/cmd/oras/internal/display/status/text_test.go index ca05177ed..628489ab2 100644 --- a/cmd/oras/internal/display/status/text_test.go +++ b/cmd/oras/internal/display/status/text_test.go @@ -16,79 +16,26 @@ 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" "oras.land/oras/cmd/oras/internal/output" "oras.land/oras/internal/testutils" ) var ( - ctx context.Context - builder *strings.Builder - printer *output.Printer - bogus ocispec.Descriptor - memStore *memory.Store - layerDesc ocispec.Descriptor - manifestDesc ocispec.Descriptor - wantedError = fmt.Errorf("wanted error") + ctx context.Context + builder *strings.Builder + printer *output.Printer + bogus ocispec.Descriptor + mockFetcher testutils.MockFetcher ) func TestMain(m *testing.M) { - // memory store for testing - memStore = memory.New() - layerContent := []byte("test") - r := bytes.NewReader(layerContent) - layerDesc = ocispec.Descriptor{ - MediaType: "application/octet-stream", - Digest: digest.FromBytes(layerContent), - Size: int64(len(layerContent)), - } - if err := memStore.Push(context.Background(), layerDesc, r); err != nil { - fmt.Println("Setup failed:", err) - os.Exit(1) - } - if err := memStore.Tag(context.Background(), layerDesc, layerDesc.Digest.String()); err != nil { - fmt.Println("Setup failed:", err) - os.Exit(1) - } - - layer1Desc := layerDesc - layer1Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer1"} - layer2Desc := layerDesc - layer2Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer2"} - manifest := ocispec.Manifest{ - MediaType: ocispec.MediaTypeImageManifest, - Layers: []ocispec.Descriptor{layer1Desc, layer2Desc}, - Config: layerDesc, - } - 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(), layerDesc, layerDesc.Digest.String()); err != nil { - fmt.Println("Setup failed:", err) - os.Exit(1) - } - + mockFetcher = testutils.NewMockFetcher() ctx = context.Background() builder = &strings.Builder{} printer = output.NewPrinter(builder, os.Stderr, false) @@ -97,37 +44,186 @@ func TestMain(m *testing.M) { } func TestTextCopyHandler_OnMounted(t *testing.T) { - fetcher := testutils.NewMockFetcher(t) - ch := NewTextCopyHandler(printer, fetcher.Fetcher) - if ch.OnMounted(ctx, fetcher.OciImage) != nil { + defer builder.Reset() + expected := "Mounted 6f42718876ce oci-image" + ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) + if ch.OnMounted(ctx, mockFetcher.OciImage) != nil { t.Error("OnMounted() should not return an error") } - + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } } func TestTextCopyHandler_OnCopySkipped(t *testing.T) { - fetcher := testutils.NewMockFetcher(t) - ch := NewTextCopyHandler(printer, fetcher.Fetcher) - if ch.OnCopySkipped(ctx, fetcher.OciImage) != nil { + defer builder.Reset() + expected := "Exists 6f42718876ce oci-image" + ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) + if ch.OnCopySkipped(ctx, mockFetcher.OciImage) != nil { t.Error("OnCopySkipped() should not return an error") } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } } func TestTextCopyHandler_PostCopy(t *testing.T) { - fetcher := testutils.NewMockFetcher(t) - ch := NewTextCopyHandler(printer, fetcher.Fetcher) - if ch.PostCopy(ctx, fetcher.OciImage) != nil { + defer builder.Reset() + expected := "Copied 6f42718876ce oci-image" + ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) + if ch.PostCopy(ctx, mockFetcher.OciImage) != nil { t.Error("PostCopy() should not return an error") } if ch.PostCopy(ctx, bogus) == nil { t.Error("PostCopy() should return an error") } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } } func TestTextCopyHandler_PreCopy(t *testing.T) { - fetcher := testutils.NewMockFetcher(t) - ch := NewTextCopyHandler(printer, fetcher.Fetcher) - if ch.PreCopy(ctx, fetcher.OciImage) != nil { + defer builder.Reset() + expected := "Copying 6f42718876ce oci-image" + ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) + if ch.PreCopy(ctx, mockFetcher.OciImage) != nil { + t.Error("PreCopy() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPullHandler_OnNodeDownloaded(t *testing.T) { + defer builder.Reset() + expected := "Downloaded 6f42718876ce oci-image" + ph := NewTextPullHandler(printer) + if ph.OnNodeDownloaded(mockFetcher.OciImage) != nil { + t.Error("OnNodeDownloaded() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPullHandler_OnNodeDownloading(t *testing.T) { + defer builder.Reset() + expected := "Downloading 6f42718876ce oci-image" + ph := NewTextPullHandler(printer) + if ph.OnNodeDownloading(mockFetcher.OciImage) != nil { + t.Error("OnNodeDownloading() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPullHandler_OnNodeProcessing(t *testing.T) { + defer builder.Reset() + expected := "Processing 6f42718876ce oci-image" + ph := NewTextPullHandler(printer) + if ph.OnNodeProcessing(mockFetcher.OciImage) != nil { + t.Error("OnNodeProcessing() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPullHandler_OnNodeRestored(t *testing.T) { + defer builder.Reset() + expected := "Restored 6f42718876ce oci-image" + ph := NewTextPullHandler(printer) + if ph.OnNodeRestored(mockFetcher.OciImage) != nil { + t.Error("OnNodeRestored() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPullHandler_OnNodeSkipped(t *testing.T) { + defer builder.Reset() + expected := "Skipped 6f42718876ce oci-image" + ph := NewTextPullHandler(printer) + if ph.OnNodeSkipped(mockFetcher.OciImage) != nil { + t.Error("OnNodeSkipped() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPushHandler_OnCopySkipped(t *testing.T) { + defer builder.Reset() + expected := "Exists 6f42718876ce oci-image" + ph := NewTextPushHandler(printer, mockFetcher.Fetcher) + if ph.OnCopySkipped(ctx, mockFetcher.OciImage) != nil { + t.Error("OnCopySkipped() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPushHandler_OnEmptyArtifact(t *testing.T) { + defer builder.Reset() + expected := "Uploading empty artifact" + ph := NewTextPushHandler(printer, mockFetcher.Fetcher) + if ph.OnEmptyArtifact() != nil { + t.Error("OnEmptyArtifact() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPushHandler_OnFileLoading(t *testing.T) { + defer builder.Reset() + expected := "" + ph := NewTextPushHandler(printer, mockFetcher.Fetcher) + if ph.OnFileLoading("name") != nil { + t.Error("OnFileLoading() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPushHandler_PostCopy(t *testing.T) { + defer builder.Reset() + expected := "Uploaded 6f42718876ce oci-image" + ph := NewTextPushHandler(printer, mockFetcher.Fetcher) + if ph.PostCopy(ctx, mockFetcher.OciImage) != nil { + t.Error("PostCopy() should not return an error") + } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } +} + +func TestTextPushHandler_PreCopy(t *testing.T) { + defer builder.Reset() + expected := "Uploading 6f42718876ce oci-image" + ph := NewTextPushHandler(printer, mockFetcher.Fetcher) + if ph.PreCopy(ctx, mockFetcher.OciImage) != nil { t.Error("PreCopy() should not return an error") } + actual := strings.TrimSpace(builder.String()) + if expected != actual { + t.Error("Output does not match expected <" + expected + "> actual <" + actual + ">") + } } diff --git a/cmd/oras/internal/display/status/tty.go b/cmd/oras/internal/display/status/tty.go index 9269717a1..615e078b7 100644 --- a/cmd/oras/internal/display/status/tty.go +++ b/cmd/oras/internal/display/status/tty.go @@ -29,14 +29,18 @@ import ( // TTYPushHandler handles TTY status output for push command. type TTYPushHandler struct { - tty *os.File - tracked track.GraphTarget + tty *os.File + tracked track.GraphTarget + committed *sync.Map + fetcher content.Fetcher } // NewTTYPushHandler returns a new handler for push status events. -func NewTTYPushHandler(tty *os.File) PushHandler { +func NewTTYPushHandler(tty *os.File, fetcher content.Fetcher) PushHandler { return &TTYPushHandler{ - tty: tty, + tty: tty, + fetcher: fetcher, + committed: &sync.Map{}, } } @@ -60,32 +64,36 @@ func (ph *TTYPushHandler) TrackTarget(gt oras.GraphTarget) (oras.GraphTarget, St return tracked, tracked.Close, nil } -// UpdateCopyOptions adds TTY status output to the copy options. -func (ph *TTYPushHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetcher content.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, PushPromptExists) +// OnCopySkipped is called when an object already exists. +func (ph *TTYPushHandler) OnCopySkipped(_ context.Context, desc ocispec.Descriptor) error { + ph.committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) + return ph.tracked.Prompt(desc, PushPromptExists) +} + +// PreCopy implements PreCopy of CopyHandler. +func (ph *TTYPushHandler) PreCopy(_ context.Context, _ ocispec.Descriptor) error { + return nil +} + +// PostCopy implements PostCopy of CopyHandler. +func (ph *TTYPushHandler) PostCopy(ctx context.Context, desc ocispec.Descriptor) error { + ph.committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) + successors, err := graph.FilteredSuccessors(ctx, desc, ph.fetcher, DeduplicatedFilter(ph.committed)) + if err != nil { + return err } - opts.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error { - committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) - successors, err := graph.FilteredSuccessors(ctx, desc, fetcher, DeduplicatedFilter(committed)) + for _, successor := range successors { + err = ph.tracked.Prompt(successor, PushPromptSkipped) if err != nil { return err } - for _, successor := range successors { - err = ph.tracked.Prompt(successor, PushPromptSkipped) - if err != nil { - return err - } - } - return nil } + return nil } // NewTTYAttachHandler returns a new handler for attach status events. -func NewTTYAttachHandler(tty *os.File) AttachHandler { - return NewTTYPushHandler(tty) +func NewTTYAttachHandler(tty *os.File, fetcher content.Fetcher) AttachHandler { + return NewTTYPushHandler(tty, fetcher) } // TTYPullHandler handles TTY status output for pull events. diff --git a/cmd/oras/internal/display/status/tty_test.go b/cmd/oras/internal/display/status/tty_test.go index 9cb31f49b..ed3767dd5 100644 --- a/cmd/oras/internal/display/status/tty_test.go +++ b/cmd/oras/internal/display/status/tty_test.go @@ -17,30 +17,30 @@ package status import ( "context" - "io" "os" "testing" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2" + + "oras.land/oras/internal/testutils" ) func TestTTYPushHandler_OnFileLoading(t *testing.T) { - ph := NewTTYPushHandler(os.Stdout) + ph := NewTTYPushHandler(os.Stdout, mockFetcher.Fetcher) if ph.OnFileLoading("test") != nil { t.Error("OnFileLoading() should not return an error") } } func TestTTYPushHandler_OnEmptyArtifact(t *testing.T) { - ph := NewTTYAttachHandler(os.Stdout) + ph := NewTTYAttachHandler(os.Stdout, mockFetcher.Fetcher) if ph.OnEmptyArtifact() != nil { t.Error("OnEmptyArtifact() should not return an error") } } func TestTTYPushHandler_TrackTarget_invalidTTY(t *testing.T) { - ph := NewTTYPushHandler(os.Stdin) + ph := NewTTYPushHandler(os.Stdin, mockFetcher.Fetcher) if _, _, err := ph.TrackTarget(nil); err == nil { t.Error("TrackTarget() should return an error for non-tty file") } @@ -67,47 +67,13 @@ func TestTTYPullHandler_OnNodeProcessing(t *testing.T) { } } -// ErrorFetcher implements content.Fetcher. -type ErrorFetcher struct{} - -// Fetch returns an error. -func (f *ErrorFetcher) Fetch(context.Context, ocispec.Descriptor) (io.ReadCloser, error) { - return nil, wantedError -} - func TestTTYPushHandler_errGetSuccessor(t *testing.T) { - ph := NewTTYPushHandler(nil) - opts := oras.CopyGraphOptions{} - ph.UpdateCopyOptions(&opts, &ErrorFetcher{}) - if err := opts.PostCopy(context.Background(), ocispec.Descriptor{ + errorFetcher := testutils.NewErrorFetcher() + ph := NewTTYPushHandler(nil, errorFetcher) + err := ph.PostCopy(context.Background(), ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageManifest, - }); err != wantedError { - t.Error("PostCopy() should return expected error") - } -} - -// ErrorPromptor mocks trackable GraphTarget. -type ErrorPromptor struct { - oras.GraphTarget - io.Closer -} - -// Prompt mocks an errored prompt. -func (p *ErrorPromptor) Prompt(ocispec.Descriptor, string) error { - return wantedError -} - -func TestTTYPushHandler_errPrompt(t *testing.T) { - ph := TTYPushHandler{ - tracked: &ErrorPromptor{}, - } - opts := oras.CopyGraphOptions{} - ph.UpdateCopyOptions(&opts, memStore) - if err := opts.OnCopySkipped(ctx, layerDesc); err != wantedError { - t.Error("OnCopySkipped() should return expected error") - } - // test - if err := opts.PostCopy(context.Background(), manifestDesc); err != wantedError { - t.Error("PostCopy() should return expected error") + }) + if err.Error() != errorFetcher.ExpectedError.Error() { + t.Errorf("PostCopy() should return expected error got %v", err.Error()) } } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 88f829779..4c094862a 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -119,11 +119,6 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder func runAttach(cmd *cobra.Command, opts *attachOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - displayStatus, displayMetadata, err := display.NewAttachHandler(opts.Printer, opts.Format, opts.TTY) - if err != nil { - return err - } - annotations, err := opts.LoadManifestAnnotations() if err != nil { return err @@ -156,6 +151,10 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { if err != nil { return fmt.Errorf("failed to resolve %s: %w", opts.Reference, err) } + displayStatus, displayMetadata, err := display.NewAttachHandler(opts.Printer, opts.Format, opts.TTY, store) + if err != nil { + return err + } descs, err := loadFiles(ctx, store, annotations, opts.FileRefs, displayStatus) if err != nil { return err @@ -168,7 +167,9 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { } graphCopyOptions := oras.DefaultCopyGraphOptions graphCopyOptions.Concurrency = opts.concurrency - displayStatus.UpdateCopyOptions(&graphCopyOptions, store) + graphCopyOptions.OnCopySkipped = displayStatus.OnCopySkipped + graphCopyOptions.PreCopy = displayStatus.PreCopy + graphCopyOptions.PostCopy = displayStatus.PostCopy packOpts := oras.PackManifestOptions{ Subject: &subject, diff --git a/cmd/oras/root/attach_test.go b/cmd/oras/root/attach_test.go index bbd443ee2..7bc666377 100644 --- a/cmd/oras/root/attach_test.go +++ b/cmd/oras/root/attach_test.go @@ -17,10 +17,11 @@ package root import ( "context" + "errors" "testing" "github.com/spf13/cobra" - "oras.land/oras/cmd/oras/internal/errors" + "oras.land/oras/cmd/oras/internal/option" ) @@ -31,12 +32,13 @@ func Test_runAttach_errType(t *testing.T) { // test opts := &attachOptions{ - Format: option.Format{ - Type: "unknown", + Packer: option.Packer{ + AnnotationFilePath: "/tmp/whatever", + ManifestAnnotations: []string{"one", "two"}, }, } got := runAttach(cmd, opts).Error() - want := errors.UnsupportedFormatTypeError(opts.Format.Type).Error() + want := errors.New("`--annotation` and `--annotation-file` cannot be both specified").Error() if got != want { t.Fatalf("got %v, want %v", got, want) } diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 56a066912..1eb11e523 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -151,10 +151,6 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t func runPush(cmd *cobra.Command, opts *pushOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - displayStatus, displayMetadata, err := display.NewPushHandler(opts.Printer, opts.Format, opts.TTY) - if err != nil { - return err - } annotations, err := opts.LoadManifestAnnotations() if err != nil { return err @@ -182,12 +178,17 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { desc.Annotations = packOpts.ConfigAnnotations packOpts.ConfigDescriptor = &desc } + memoryStore := memory.New() + union := contentutil.MultiReadOnlyTarget(memoryStore, store) + displayStatus, displayMetadata, err := display.NewPushHandler(opts.Printer, opts.Format, opts.TTY, union) + if err != nil { + return err + } descs, err := loadFiles(ctx, store, annotations, opts.FileRefs, displayStatus) if err != nil { return err } packOpts.Layers = descs - memoryStore := memory.New() pack := func() (ocispec.Descriptor, error) { root, err := oras.PackManifest(ctx, memoryStore, opts.PackVersion, opts.artifactType, packOpts) if err != nil { @@ -210,8 +211,9 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { } copyOptions := oras.DefaultCopyOptions copyOptions.Concurrency = opts.concurrency - union := contentutil.MultiReadOnlyTarget(memoryStore, store) - displayStatus.UpdateCopyOptions(©Options.CopyGraphOptions, union) + copyOptions.CopyGraphOptions.OnCopySkipped = displayStatus.OnCopySkipped + copyOptions.CopyGraphOptions.PreCopy = displayStatus.PreCopy + copyOptions.CopyGraphOptions.PostCopy = displayStatus.PostCopy copy := func(root ocispec.Descriptor) error { // add both pull and push scope hints for dst repository // to save potential push-scope token requests during copy diff --git a/internal/graph/graph_test.go b/internal/graph/graph_test.go index 9cd95b2ee..7d61bc209 100644 --- a/internal/graph/graph_test.go +++ b/internal/graph/graph_test.go @@ -27,7 +27,7 @@ import ( ) func TestSuccessors(t *testing.T) { - mockFetcher := testutils.NewMockFetcher(t) + mockFetcher := testutils.NewMockFetcher() fetcher := mockFetcher.Fetcher ctx := context.Background() type args struct { @@ -70,7 +70,7 @@ func TestSuccessors(t *testing.T) { } func TestDescriptor_GetSuccessors(t *testing.T) { - mockFetcher := testutils.NewMockFetcher(t) + mockFetcher := testutils.NewMockFetcher() allFilter := func(ocispec.Descriptor) bool { return true diff --git a/internal/testutils/fetcher.go b/internal/testutils/fetcher.go index a0f8b636d..9f3dc7cd1 100644 --- a/internal/testutils/fetcher.go +++ b/internal/testutils/fetcher.go @@ -19,16 +19,33 @@ import ( "bytes" "context" "encoding/json" + "fmt" "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "io" "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/content/memory" "oras.land/oras/internal/docker" - "testing" ) +// ErrorFetcher implements content.Fetcher. +type ErrorFetcher struct { + ExpectedError error +} + +// NewErrorFetcher create and error fetcher +func NewErrorFetcher() *ErrorFetcher { + return &ErrorFetcher{ + ExpectedError: fmt.Errorf("expected error"), + } +} + +// Fetch returns an error. +func (f *ErrorFetcher) Fetch(context.Context, ocispec.Descriptor) (io.ReadCloser, error) { + return nil, f.ExpectedError +} + type MockFetcher struct { - t *testing.T store *memory.Store Fetcher content.Fetcher Subject ocispec.Descriptor @@ -39,12 +56,13 @@ type MockFetcher struct { } // NewMockFetcher creates a MockFetcher and populates it. -func NewMockFetcher(t *testing.T) (mockFetcher MockFetcher) { - mockFetcher = MockFetcher{store: memory.New(), t: t} +func NewMockFetcher() (mockFetcher MockFetcher) { + mockFetcher = MockFetcher{store: memory.New()} mockFetcher.Subject = mockFetcher.PushBlob(ocispec.MediaTypeImageLayer, []byte("blob")) imageType := "test.image" mockFetcher.Config = mockFetcher.PushBlob(imageType, []byte("config content")) mockFetcher.OciImage = mockFetcher.PushOCIImage(&mockFetcher.Subject, mockFetcher.Config) + mockFetcher.OciImage.Annotations = map[string]string{ocispec.AnnotationTitle: "oci-image"} mockFetcher.DockerImage = mockFetcher.PushDockerImage(&mockFetcher.Subject, mockFetcher.Config) mockFetcher.Index = mockFetcher.PushIndex(mockFetcher.Subject) mockFetcher.Fetcher = mockFetcher.store @@ -59,7 +77,7 @@ func (mf *MockFetcher) PushBlob(mediaType string, blob []byte) ocispec.Descripto Size: int64(len(blob)), } if err := mf.store.Push(context.Background(), desc, bytes.NewReader(blob)); err != nil { - mf.t.Fatal(err) + panic(err) } return desc } @@ -73,7 +91,7 @@ func (mf *MockFetcher) pushImage(subject *ocispec.Descriptor, mediaType string, } manifestJSON, err := json.Marshal(manifest) if err != nil { - mf.t.Fatal(err) + panic(err) } return mf.PushBlob(mediaType, manifestJSON) } @@ -95,7 +113,7 @@ func (mf *MockFetcher) PushIndex(manifests ...ocispec.Descriptor) ocispec.Descri } indexJSON, err := json.Marshal(index) if err != nil { - mf.t.Fatal(err) + panic(err) } return mf.PushBlob(ocispec.MediaTypeImageIndex, indexJSON) }