From 0b2cc3b09250a3227255c57210df88421a24b603 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Fri, 26 Jul 2024 13:23:08 -0600 Subject: [PATCH 1/4] refactor: Successor getting with separation of concerns Signed-off-by: Terry Howe --- cmd/oras/internal/display/status/text.go | 20 ++- cmd/oras/internal/display/status/text_test.go | 131 ++++++++++++++++++ cmd/oras/internal/display/status/tty.go | 17 ++- cmd/oras/internal/display/status/tty_test.go | 62 --------- cmd/oras/internal/display/status/utils.go | 16 +++ cmd/oras/internal/output/print.go | 30 ---- cmd/oras/root/cp.go | 14 +- internal/descriptor/descriptor.go | 1 - internal/graph/graph.go | 16 +++ internal/graph/graph_test.go | 43 ++++++ 10 files changed, 244 insertions(+), 106 deletions(-) create mode 100644 cmd/oras/internal/display/status/text_test.go diff --git a/cmd/oras/internal/display/status/text.go b/cmd/oras/internal/display/status/text.go index c2f35af62..5e5b6c20d 100644 --- a/cmd/oras/internal/display/status/text.go +++ b/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) } diff --git a/cmd/oras/internal/display/status/text_test.go b/cmd/oras/internal/display/status/text_test.go new file mode 100644 index 000000000..47d9d6282 --- /dev/null +++ b/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") + } +} diff --git a/cmd/oras/internal/display/status/tty.go b/cmd/oras/internal/display/status/tty.go index fa7e4345c..9269717a1 100644 --- a/cmd/oras/internal/display/status/tty.go +++ b/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 } } diff --git a/cmd/oras/internal/display/status/tty_test.go b/cmd/oras/internal/display/status/tty_test.go index cc6fc6935..f611bcd33 100644 --- a/cmd/oras/internal/display/status/tty_test.go +++ b/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 { diff --git a/cmd/oras/internal/display/status/utils.go b/cmd/oras/internal/display/status/utils.go index 0b7f244e1..453a1acc6 100644 --- a/cmd/oras/internal/display/status/utils.go +++ b/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 + } +} diff --git a/cmd/oras/internal/output/print.go b/cmd/oras/internal/output/print.go index a3f6a42fc..d7e486866 100644 --- a/cmd/oras/internal/output/print.go +++ b/cmd/oras/internal/output/print.go @@ -16,7 +16,6 @@ limitations under the License. package output import ( - "context" "fmt" "io" "sync" @@ -24,12 +23,8 @@ import ( "oras.land/oras/internal/descriptor" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2/content" ) -// PrintFunc is the function type returned by StatusPrinter. -type PrintFunc func(ocispec.Descriptor) error - // Printer prints for status handlers. type Printer struct { out io.Writer @@ -92,28 +87,3 @@ func (p *Printer) PrintStatus(desc ocispec.Descriptor, status string) error { } return p.Println(status, descriptor.ShortDigest(desc), name) } - -// StatusPrinter returns a tracking function for transfer status. -func (p *Printer) StatusPrinter(status string) PrintFunc { - return func(desc ocispec.Descriptor) error { - return p.PrintStatus(desc, status) - } -} - -// PrintSuccessorStatus prints transfer status of successors. -func PrintSuccessorStatus(ctx context.Context, desc ocispec.Descriptor, fetcher content.Fetcher, committed *sync.Map, print PrintFunc) error { - successors, err := content.Successors(ctx, fetcher, desc) - if err != nil { - return err - } - for _, s := range successors { - name := s.Annotations[ocispec.AnnotationTitle] - if v, ok := committed.Load(s.Digest.String()); ok && v != name { - // Reprint status for deduplicated content - if err := print(s); err != nil { - return err - } - } - } - return nil -} diff --git a/cmd/oras/root/cp.go b/cmd/oras/root/cp.go index 5f04adf67..9f0fd6fa4 100644 --- a/cmd/oras/root/cp.go +++ b/cmd/oras/root/cp.go @@ -38,7 +38,6 @@ import ( "oras.land/oras/cmd/oras/internal/display/status/track" 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/docker" "oras.land/oras/internal/graph" "oras.land/oras/internal/listener" @@ -198,9 +197,16 @@ func doCopy(ctx context.Context, copyHandler status.CopyHandler, src oras.ReadOn } extendedCopyOptions.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) - return output.PrintSuccessorStatus(ctx, desc, tracked, committed, func(desc ocispec.Descriptor) error { - return tracked.Prompt(desc, promptSkipped) - }) + successors, err := graph.FilteredSuccessors(ctx, desc, tracked, status.DeduplicatedFilter(committed)) + if err != nil { + return err + } + for _, successor := range successors { + if err = tracked.Prompt(successor, promptSkipped); err != nil { + return err + } + } + return nil } extendedCopyOptions.OnMounted = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) diff --git a/internal/descriptor/descriptor.go b/internal/descriptor/descriptor.go index 46ad93249..84806fed2 100644 --- a/internal/descriptor/descriptor.go +++ b/internal/descriptor/descriptor.go @@ -18,7 +18,6 @@ package descriptor import ( "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras/internal/docker" ) diff --git a/internal/graph/graph.go b/internal/graph/graph.go index acaf42171..5041c2a2d 100644 --- a/internal/graph/graph.go +++ b/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 +} diff --git a/internal/graph/graph_test.go b/internal/graph/graph_test.go index 15efd94f9..9cd95b2ee 100644 --- a/internal/graph/graph_test.go +++ b/internal/graph/graph_test.go @@ -68,3 +68,46 @@ func TestSuccessors(t *testing.T) { }) } } + +func TestDescriptor_GetSuccessors(t *testing.T) { + mockFetcher := testutils.NewMockFetcher(t) + + allFilter := func(ocispec.Descriptor) bool { + return true + } + got, err := FilteredSuccessors(context.Background(), mockFetcher.OciImage, mockFetcher.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 mockFetcher.Subject.Digest != got[0].Digest { + t.Errorf("FilteredSuccessors got %v, want %v", got[0], mockFetcher.Subject) + } + if mockFetcher.Config.Digest != got[1].Digest { + t.Errorf("FilteredSuccessors got %v, want %v", got[1], mockFetcher.Subject) + } + + noConfig := func(desc ocispec.Descriptor) bool { + return desc.Digest != mockFetcher.Config.Digest + } + got, err = FilteredSuccessors(context.Background(), mockFetcher.OciImage, mockFetcher.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 mockFetcher.Subject.Digest != got[0].Digest { + t.Errorf("FilteredSuccessors got %v, want %v", got[0], mockFetcher.Subject) + } + + got, err = FilteredSuccessors(context.Background(), ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest}, mockFetcher.Fetcher, allFilter) + if nil == err { + t.Error("FilteredSuccessors expected error") + } + if got != nil { + t.Errorf("FilteredSuccessors unexpected %v", got) + } +} From 7fcc38173fde862afd94883ef35b0c949089b897 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 6 Aug 2024 05:34:07 +0000 Subject: [PATCH 2/4] test(unit): increase coverage Signed-off-by: Billy Zha --- cmd/oras/internal/display/status/text_test.go | 32 +++++++------ cmd/oras/internal/display/status/tty_test.go | 48 +++++++++++++++++++ 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/cmd/oras/internal/display/status/text_test.go b/cmd/oras/internal/display/status/text_test.go index 47d9d6282..ca05177ed 100644 --- a/cmd/oras/internal/display/status/text_test.go +++ b/cmd/oras/internal/display/status/text_test.go @@ -20,14 +20,15 @@ import ( "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" - "os" - "strings" - "testing" ) var ( @@ -36,37 +37,38 @@ var ( printer *output.Printer bogus ocispec.Descriptor memStore *memory.Store - memDesc ocispec.Descriptor + layerDesc ocispec.Descriptor manifestDesc ocispec.Descriptor + wantedError = fmt.Errorf("wanted error") ) func TestMain(m *testing.M) { // memory store for testing memStore = memory.New() - content := []byte("test") - r := bytes.NewReader(content) - memDesc = ocispec.Descriptor{ + layerContent := []byte("test") + r := bytes.NewReader(layerContent) + layerDesc = ocispec.Descriptor{ MediaType: "application/octet-stream", - Digest: digest.FromBytes(content), - Size: int64(len(content)), + Digest: digest.FromBytes(layerContent), + Size: int64(len(layerContent)), } - if err := memStore.Push(context.Background(), memDesc, r); err != nil { + if err := memStore.Push(context.Background(), layerDesc, r); err != nil { fmt.Println("Setup failed:", err) os.Exit(1) } - if err := memStore.Tag(context.Background(), memDesc, memDesc.Digest.String()); err != nil { + if err := memStore.Tag(context.Background(), layerDesc, layerDesc.Digest.String()); err != nil { fmt.Println("Setup failed:", err) os.Exit(1) } - layer1Desc := memDesc + layer1Desc := layerDesc layer1Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer1"} - layer2Desc := memDesc + layer2Desc := layerDesc layer2Desc.Annotations = map[string]string{ocispec.AnnotationTitle: "layer2"} manifest := ocispec.Manifest{ MediaType: ocispec.MediaTypeImageManifest, Layers: []ocispec.Descriptor{layer1Desc, layer2Desc}, - Config: memDesc, + Config: layerDesc, } manifestContent, err := json.Marshal(&manifest) if err != nil { @@ -82,7 +84,7 @@ func TestMain(m *testing.M) { fmt.Println("Setup failed:", err) os.Exit(1) } - if err := memStore.Tag(context.Background(), memDesc, memDesc.Digest.String()); err != nil { + if err := memStore.Tag(context.Background(), layerDesc, layerDesc.Digest.String()); err != nil { fmt.Println("Setup failed:", err) os.Exit(1) } diff --git a/cmd/oras/internal/display/status/tty_test.go b/cmd/oras/internal/display/status/tty_test.go index f611bcd33..9cb31f49b 100644 --- a/cmd/oras/internal/display/status/tty_test.go +++ b/cmd/oras/internal/display/status/tty_test.go @@ -16,10 +16,13 @@ limitations under the License. package status import ( + "context" + "io" "os" "testing" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2" ) func TestTTYPushHandler_OnFileLoading(t *testing.T) { @@ -63,3 +66,48 @@ func TestTTYPullHandler_OnNodeProcessing(t *testing.T) { t.Error("OnNodeProcessing() should not return an error") } } + +// 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{ + 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") + } +} From ecd9e7f3b083face7165000275205e8cf51f46bb Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Thu, 8 Aug 2024 11:53:55 -0600 Subject: [PATCH 3/4] 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 ++-- .../display/status/tty_console_test.go | 37 +-- 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 ++- 14 files changed, 321 insertions(+), 240 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_console_test.go b/cmd/oras/internal/display/status/tty_console_test.go index 3b7e853d4..426285f62 100644 --- a/cmd/oras/internal/display/status/tty_console_test.go +++ b/cmd/oras/internal/display/status/tty_console_test.go @@ -18,10 +18,7 @@ limitations under the License. package status import ( - "context" - "oras.land/oras-go/v2" "oras.land/oras-go/v2/content/memory" - "oras.land/oras/cmd/oras/internal/display/status/track" "oras.land/oras/internal/testutils" "testing" ) @@ -33,7 +30,7 @@ func TestTTYPushHandler_TrackTarget(t *testing.T) { t.Fatal(err) } defer slave.Close() - ph := NewTTYPushHandler(slave) + ph := NewTTYPushHandler(slave, mockFetcher.Fetcher) store := memory.New() // test _, fn, err := ph.TrackTarget(store) @@ -50,38 +47,6 @@ func TestTTYPushHandler_TrackTarget(t *testing.T) { } } -func TestTTYPushHandler_UpdateCopyOptions(t *testing.T) { - // prepare pty - pty, slave, err := testutils.NewPty() - if err != nil { - t.Fatal(err) - } - defer slave.Close() - ph := NewTTYPushHandler(slave) - gt, _, err := ph.TrackTarget(memory.New()) - if err != nil { - 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, manifestDesc, opts); err != nil { - t.Fatalf("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.Fatalf("TrackTarget() should return a *track.GraphTarget, got %T", tracked) - } else { - _ = tracked.Close() - } - // validate - if err = testutils.MatchPty(pty, slave, "Exists", manifestDesc.MediaType, "100.00%", manifestDesc.Digest.String()); err != nil { - t.Fatal(err) - } -} - func Test_TTYPullHandler_TrackTarget(t *testing.T) { src := memory.New() t.Run("has TTY", func(t *testing.T) { 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) } From ec11e039e28ff36bee08b7a0f99fcbf62bd8b6b4 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 9 Aug 2024 15:41:23 +0000 Subject: [PATCH 4/4] test(unit): increase coverage Signed-off-by: Billy Zha --- cmd/oras/internal/display/status/text_test.go | 24 ++++----- cmd/oras/internal/display/status/tty.go | 6 +-- cmd/oras/internal/display/status/tty_test.go | 47 ++++++++++++++-- internal/graph/graph_test.go | 10 ++-- internal/testutils/fetcher.go | 17 +++--- internal/testutils/prompt.go | 53 +++++++++++++++++++ 6 files changed, 127 insertions(+), 30 deletions(-) create mode 100644 internal/testutils/prompt.go diff --git a/cmd/oras/internal/display/status/text_test.go b/cmd/oras/internal/display/status/text_test.go index 628489ab2..b008501ec 100644 --- a/cmd/oras/internal/display/status/text_test.go +++ b/cmd/oras/internal/display/status/text_test.go @@ -45,7 +45,7 @@ func TestMain(m *testing.M) { func TestTextCopyHandler_OnMounted(t *testing.T) { defer builder.Reset() - expected := "Mounted 6f42718876ce oci-image" + expected := "Mounted 0b442c23c1dd oci-image" ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) if ch.OnMounted(ctx, mockFetcher.OciImage) != nil { t.Error("OnMounted() should not return an error") @@ -58,7 +58,7 @@ func TestTextCopyHandler_OnMounted(t *testing.T) { func TestTextCopyHandler_OnCopySkipped(t *testing.T) { defer builder.Reset() - expected := "Exists 6f42718876ce oci-image" + expected := "Exists 0b442c23c1dd oci-image" ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) if ch.OnCopySkipped(ctx, mockFetcher.OciImage) != nil { t.Error("OnCopySkipped() should not return an error") @@ -71,7 +71,7 @@ func TestTextCopyHandler_OnCopySkipped(t *testing.T) { func TestTextCopyHandler_PostCopy(t *testing.T) { defer builder.Reset() - expected := "Copied 6f42718876ce oci-image" + expected := "Copied 0b442c23c1dd oci-image" ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) if ch.PostCopy(ctx, mockFetcher.OciImage) != nil { t.Error("PostCopy() should not return an error") @@ -87,7 +87,7 @@ func TestTextCopyHandler_PostCopy(t *testing.T) { func TestTextCopyHandler_PreCopy(t *testing.T) { defer builder.Reset() - expected := "Copying 6f42718876ce oci-image" + expected := "Copying 0b442c23c1dd oci-image" ch := NewTextCopyHandler(printer, mockFetcher.Fetcher) if ch.PreCopy(ctx, mockFetcher.OciImage) != nil { t.Error("PreCopy() should not return an error") @@ -100,7 +100,7 @@ func TestTextCopyHandler_PreCopy(t *testing.T) { func TestTextPullHandler_OnNodeDownloaded(t *testing.T) { defer builder.Reset() - expected := "Downloaded 6f42718876ce oci-image" + expected := "Downloaded 0b442c23c1dd oci-image" ph := NewTextPullHandler(printer) if ph.OnNodeDownloaded(mockFetcher.OciImage) != nil { t.Error("OnNodeDownloaded() should not return an error") @@ -113,7 +113,7 @@ func TestTextPullHandler_OnNodeDownloaded(t *testing.T) { func TestTextPullHandler_OnNodeDownloading(t *testing.T) { defer builder.Reset() - expected := "Downloading 6f42718876ce oci-image" + expected := "Downloading 0b442c23c1dd oci-image" ph := NewTextPullHandler(printer) if ph.OnNodeDownloading(mockFetcher.OciImage) != nil { t.Error("OnNodeDownloading() should not return an error") @@ -126,7 +126,7 @@ func TestTextPullHandler_OnNodeDownloading(t *testing.T) { func TestTextPullHandler_OnNodeProcessing(t *testing.T) { defer builder.Reset() - expected := "Processing 6f42718876ce oci-image" + expected := "Processing 0b442c23c1dd oci-image" ph := NewTextPullHandler(printer) if ph.OnNodeProcessing(mockFetcher.OciImage) != nil { t.Error("OnNodeProcessing() should not return an error") @@ -139,7 +139,7 @@ func TestTextPullHandler_OnNodeProcessing(t *testing.T) { func TestTextPullHandler_OnNodeRestored(t *testing.T) { defer builder.Reset() - expected := "Restored 6f42718876ce oci-image" + expected := "Restored 0b442c23c1dd oci-image" ph := NewTextPullHandler(printer) if ph.OnNodeRestored(mockFetcher.OciImage) != nil { t.Error("OnNodeRestored() should not return an error") @@ -152,7 +152,7 @@ func TestTextPullHandler_OnNodeRestored(t *testing.T) { func TestTextPullHandler_OnNodeSkipped(t *testing.T) { defer builder.Reset() - expected := "Skipped 6f42718876ce oci-image" + expected := "Skipped 0b442c23c1dd oci-image" ph := NewTextPullHandler(printer) if ph.OnNodeSkipped(mockFetcher.OciImage) != nil { t.Error("OnNodeSkipped() should not return an error") @@ -165,7 +165,7 @@ func TestTextPullHandler_OnNodeSkipped(t *testing.T) { func TestTextPushHandler_OnCopySkipped(t *testing.T) { defer builder.Reset() - expected := "Exists 6f42718876ce oci-image" + expected := "Exists 0b442c23c1dd oci-image" ph := NewTextPushHandler(printer, mockFetcher.Fetcher) if ph.OnCopySkipped(ctx, mockFetcher.OciImage) != nil { t.Error("OnCopySkipped() should not return an error") @@ -204,7 +204,7 @@ func TestTextPushHandler_OnFileLoading(t *testing.T) { func TestTextPushHandler_PostCopy(t *testing.T) { defer builder.Reset() - expected := "Uploaded 6f42718876ce oci-image" + expected := "Uploaded 0b442c23c1dd oci-image" ph := NewTextPushHandler(printer, mockFetcher.Fetcher) if ph.PostCopy(ctx, mockFetcher.OciImage) != nil { t.Error("PostCopy() should not return an error") @@ -217,7 +217,7 @@ func TestTextPushHandler_PostCopy(t *testing.T) { func TestTextPushHandler_PreCopy(t *testing.T) { defer builder.Reset() - expected := "Uploading 6f42718876ce oci-image" + expected := "Uploading 0b442c23c1dd oci-image" ph := NewTextPushHandler(printer, mockFetcher.Fetcher) if ph.PreCopy(ctx, mockFetcher.OciImage) != nil { t.Error("PreCopy() should not return an error") diff --git a/cmd/oras/internal/display/status/tty.go b/cmd/oras/internal/display/status/tty.go index 615e078b7..78f639076 100644 --- 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/internal/graph" "os" "sync" + "oras.land/oras/internal/graph" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2" "oras.land/oras-go/v2/content" @@ -83,8 +84,7 @@ func (ph *TTYPushHandler) PostCopy(ctx context.Context, desc ocispec.Descriptor) return err } for _, successor := range successors { - err = ph.tracked.Prompt(successor, PushPromptSkipped) - if err != nil { + if err = ph.tracked.Prompt(successor, PushPromptSkipped); err != nil { return err } } diff --git a/cmd/oras/internal/display/status/tty_test.go b/cmd/oras/internal/display/status/tty_test.go index ed3767dd5..4f04394a8 100644 --- a/cmd/oras/internal/display/status/tty_test.go +++ b/cmd/oras/internal/display/status/tty_test.go @@ -16,12 +16,12 @@ limitations under the License. package status import ( - "context" + "errors" "os" + "sync" "testing" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras/internal/testutils" ) @@ -67,13 +67,52 @@ func TestTTYPullHandler_OnNodeProcessing(t *testing.T) { } } -func TestTTYPushHandler_errGetSuccessor(t *testing.T) { +func TestTTYPushHandler_PostCopy(t *testing.T) { + fetcher := testutils.NewMockFetcher() + committed := &sync.Map{} + committed.Store(fetcher.ImageLayer.Digest.String(), fetcher.ImageLayer.Annotations[ocispec.AnnotationTitle]) + ph := &TTYPushHandler{ + tracked: &testutils.PromptDiscarder{}, + committed: committed, + fetcher: fetcher.Fetcher, + } + if err := ph.PostCopy(ctx, fetcher.OciImage); err != nil { + t.Errorf("unexpected error from PostCopy(): %v", err) + } +} + +func TestTTYPushHandler_PostCopy_errGetSuccessor(t *testing.T) { errorFetcher := testutils.NewErrorFetcher() ph := NewTTYPushHandler(nil, errorFetcher) - err := ph.PostCopy(context.Background(), ocispec.Descriptor{ + err := ph.PostCopy(ctx, ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageManifest, }) if err.Error() != errorFetcher.ExpectedError.Error() { t.Errorf("PostCopy() should return expected error got %v", err.Error()) } } + +func TestTTYPushHandler_PostCopy_errPrompt(t *testing.T) { + fetcher := testutils.NewMockFetcher() + committed := &sync.Map{} + committed.Store(fetcher.ImageLayer.Digest.String(), fetcher.ImageLayer.Annotations[ocispec.AnnotationTitle]+"1") + wantedError := errors.New("wanted error") + ph := &TTYPushHandler{ + tracked: testutils.NewErrorPrompt(wantedError), + committed: committed, + fetcher: fetcher.Fetcher, + } + if err := ph.PostCopy(ctx, fetcher.OciImage); err != wantedError { + t.Errorf("PostCopy() should return expected error got %v", err) + } +} + +func TestTTYPushHandler_OnCopySkipped(t *testing.T) { + ph := &TTYPushHandler{ + tracked: &testutils.PromptDiscarder{}, + committed: &sync.Map{}, + } + if err := ph.OnCopySkipped(ctx, ocispec.Descriptor{}); err != nil { + t.Error("OnCopySkipped() should not return an error") + } +} diff --git a/internal/graph/graph_test.go b/internal/graph/graph_test.go index 7d61bc209..985616367 100644 --- a/internal/graph/graph_test.go +++ b/internal/graph/graph_test.go @@ -45,9 +45,9 @@ 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, mockFetcher.DockerImage}, nil, &mockFetcher.Subject, &mockFetcher.Config, false}, - {"should get success of an OCI image", args{ctx, fetcher, mockFetcher.OciImage}, nil, &mockFetcher.Subject, &mockFetcher.Config, false}, - {"should get success of an index", args{ctx, fetcher, mockFetcher.Index}, []ocispec.Descriptor{mockFetcher.Subject}, nil, nil, false}, + {"should get successors of a docker image", args{ctx, fetcher, mockFetcher.DockerImage}, nil, nil, &mockFetcher.Config, false}, + {"should get successors of an OCI image", args{ctx, fetcher, mockFetcher.OciImage}, []ocispec.Descriptor{mockFetcher.ImageLayer}, &mockFetcher.Subject, &mockFetcher.Config, false}, + {"should get successors of an index", args{ctx, fetcher, mockFetcher.Index}, []ocispec.Descriptor{mockFetcher.Subject}, nil, nil, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -79,7 +79,7 @@ func TestDescriptor_GetSuccessors(t *testing.T) { if nil != err { t.Errorf("FilteredSuccessors unexpected error %v", err) } - if len(got) != 2 { + if len(got) != 3 { t.Errorf("Expected 2 successors got %v", len(got)) } if mockFetcher.Subject.Digest != got[0].Digest { @@ -96,7 +96,7 @@ func TestDescriptor_GetSuccessors(t *testing.T) { if nil != err { t.Errorf("FilteredSuccessors unexpected error %v", err) } - if len(got) != 1 { + if len(got) != 2 { t.Errorf("Expected 1 successors got %v", len(got)) } if mockFetcher.Subject.Digest != got[0].Digest { diff --git a/internal/testutils/fetcher.go b/internal/testutils/fetcher.go index 9f3dc7cd1..57b070fde 100644 --- a/internal/testutils/fetcher.go +++ b/internal/testutils/fetcher.go @@ -20,9 +20,10 @@ import ( "context" "encoding/json" "fmt" + "io" + "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" @@ -45,12 +46,14 @@ func (f *ErrorFetcher) Fetch(context.Context, ocispec.Descriptor) (io.ReadCloser return nil, f.ExpectedError } +// MockFetcher implements content.Fetcher and populates a memory store. type MockFetcher struct { store *memory.Store Fetcher content.Fetcher Subject ocispec.Descriptor Config ocispec.Descriptor OciImage ocispec.Descriptor + ImageLayer ocispec.Descriptor DockerImage ocispec.Descriptor Index ocispec.Descriptor } @@ -58,12 +61,14 @@ type MockFetcher struct { // NewMockFetcher creates a MockFetcher and populates it. 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.ImageLayer = mockFetcher.PushBlob(ocispec.MediaTypeImageLayer, []byte("layer content")) + mockFetcher.ImageLayer.Annotations = map[string]string{ocispec.AnnotationTitle: "layer"} + mockFetcher.Subject = mockFetcher.PushOCIImage(nil, mockFetcher.Config) + mockFetcher.OciImage = mockFetcher.PushOCIImage(&mockFetcher.Subject, mockFetcher.Config, mockFetcher.ImageLayer) mockFetcher.OciImage.Annotations = map[string]string{ocispec.AnnotationTitle: "oci-image"} - mockFetcher.DockerImage = mockFetcher.PushDockerImage(&mockFetcher.Subject, mockFetcher.Config) + mockFetcher.DockerImage = mockFetcher.PushDockerImage(mockFetcher.Config) mockFetcher.Index = mockFetcher.PushIndex(mockFetcher.Subject) mockFetcher.Fetcher = mockFetcher.store return mockFetcher @@ -102,8 +107,8 @@ func (mf *MockFetcher) PushOCIImage(subject *ocispec.Descriptor, config ocispec. } // PushDockerImage pushes the given subject, config and layers as a Docker image. -func (mf *MockFetcher) PushDockerImage(subject *ocispec.Descriptor, config ocispec.Descriptor, layers ...ocispec.Descriptor) ocispec.Descriptor { - return mf.pushImage(subject, docker.MediaTypeManifest, config, layers...) +func (mf *MockFetcher) PushDockerImage(config ocispec.Descriptor, layers ...ocispec.Descriptor) ocispec.Descriptor { + return mf.pushImage(nil, docker.MediaTypeManifest, config, layers...) } // PushIndex pushes the manifests as an index. diff --git a/internal/testutils/prompt.go b/internal/testutils/prompt.go new file mode 100644 index 000000000..f40763d94 --- /dev/null +++ b/internal/testutils/prompt.go @@ -0,0 +1,53 @@ +/* +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 testutils + +import ( + "io" + + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2" +) + +// PromptDiscarder mocks trackable GraphTarget with discarded prompt. +type PromptDiscarder struct { + oras.GraphTarget + io.Closer +} + +// Prompt discards the prompt. +func (p *PromptDiscarder) Prompt(ocispec.Descriptor, string) error { + return nil +} + +// ErrorPrompt mocks an errored prompt. +type ErrorPrompt struct { + oras.GraphTarget + io.Closer + wanted error +} + +// NewErrorPrompt creates an error prompt. +func NewErrorPrompt(err error) *ErrorPrompt { + return &ErrorPrompt{ + wanted: err, + } +} + +// Prompt mocks an errored prompt. +func (e *ErrorPrompt) Prompt(ocispec.Descriptor, string) error { + return e.wanted +}