From 97ba0a1546d3ad9de74fcfbccc7b37d6057c83c6 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 11 Nov 2024 19:05:40 +0800 Subject: [PATCH] increase test coverage Signed-off-by: Xiaoxuan Wang --- cmd/oras/root/manifest/index/update.go | 9 +- cmd/oras/root/manifest/index/update_test.go | 226 +++++++++++++------- 2 files changed, 149 insertions(+), 86 deletions(-) diff --git a/cmd/oras/root/manifest/index/update.go b/cmd/oras/root/manifest/index/update.go index 659adf6f0..e82204519 100644 --- a/cmd/oras/root/manifest/index/update.go +++ b/cmd/oras/root/manifest/index/update.go @@ -29,7 +29,6 @@ import ( "oras.land/oras/cmd/oras/internal/argument" "oras.land/oras/cmd/oras/internal/command" "oras.land/oras/cmd/oras/internal/display" - "oras.land/oras/cmd/oras/internal/display/metadata" "oras.land/oras/cmd/oras/internal/display/status" oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" @@ -123,11 +122,11 @@ func updateIndex(cmd *cobra.Command, opts updateOptions) error { if err != nil { return err } - manifests, err = addManifests(ctx, displayStatus, displayMetadata, manifests, target, opts.addArguments) + manifests, err = addManifests(ctx, displayStatus, manifests, target, opts.addArguments) if err != nil { return err } - manifests, err = mergeIndexes(ctx, displayStatus, displayMetadata, manifests, target, opts.mergeArguments) + manifests, err = mergeIndexes(ctx, displayStatus, manifests, target, opts.mergeArguments) if err != nil { return err } @@ -173,7 +172,7 @@ func fetchIndex(ctx context.Context, handler status.ManifestIndexUpdateHandler, return index, nil } -func addManifests(ctx context.Context, displayStatus status.ManifestIndexUpdateHandler, displayMetadata metadata.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, addArguments []string) ([]ocispec.Descriptor, error) { +func addManifests(ctx context.Context, displayStatus status.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, addArguments []string) ([]ocispec.Descriptor, error) { for _, manifestRef := range addArguments { if err := displayStatus.OnFetching(manifestRef); err != nil { return nil, err @@ -199,7 +198,7 @@ func addManifests(ctx context.Context, displayStatus status.ManifestIndexUpdateH return manifests, nil } -func mergeIndexes(ctx context.Context, displayStatus status.ManifestIndexUpdateHandler, displayMetadata metadata.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, mergeArguments []string) ([]ocispec.Descriptor, error) { +func mergeIndexes(ctx context.Context, displayStatus status.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, mergeArguments []string) ([]ocispec.Descriptor, error) { for _, indexRef := range mergeArguments { if err := displayStatus.OnFetching(indexRef); err != nil { return nil, err diff --git a/cmd/oras/root/manifest/index/update_test.go b/cmd/oras/root/manifest/index/update_test.go index bc46c1884..63171f28b 100644 --- a/cmd/oras/root/manifest/index/update_test.go +++ b/cmd/oras/root/manifest/index/update_test.go @@ -18,7 +18,6 @@ package index import ( "context" "fmt" - "os" "reflect" "testing" @@ -26,7 +25,6 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2" "oras.land/oras/cmd/oras/internal/display/status" - "oras.land/oras/cmd/oras/internal/output" ) // // for quick drafting @@ -37,6 +35,77 @@ import ( // return 0, fmt.Errorf("test error") // } +type testUpdateDisplayStatus struct { + onFetchingError bool + onFetchedError bool + onIndexPackedError bool + onIndexPushedError bool + onManifestRemovedError bool + onManifestAddedError bool + onIndexMergedError bool +} + +func (tds *testUpdateDisplayStatus) OnFetching(manifestRef string) error { + if tds.onFetchingError { + return fmt.Errorf("OnFetching error") + } + return nil +} + +func (tds *testUpdateDisplayStatus) OnFetched(manifestRef string, desc ocispec.Descriptor) error { + if tds.onFetchedError { + return fmt.Errorf("OnFetched error") + } + return nil +} + +func (tds *testUpdateDisplayStatus) OnIndexPacked(desc ocispec.Descriptor) error { + if tds.onIndexPackedError { + return fmt.Errorf("error") + } + return nil +} + +func (tds *testUpdateDisplayStatus) OnIndexPushed(path string) error { + if tds.onIndexPushedError { + return fmt.Errorf("error") + } + return nil +} + +func (tds *testUpdateDisplayStatus) OnManifestRemoved(digest digest.Digest) error { + if tds.onManifestRemovedError { + return fmt.Errorf("OnManifestRemoved error") + } + return nil +} + +func (tds *testUpdateDisplayStatus) OnManifestAdded(manifestRef string, desc ocispec.Descriptor) error { + if tds.onManifestAddedError { + return fmt.Errorf("error") + } + return nil +} + +func (tds *testUpdateDisplayStatus) OnIndexMerged(indexRef string, desc ocispec.Descriptor) error { + if tds.onIndexMergedError { + return fmt.Errorf("error") + } + return nil +} + +func NewTestUpdateDisplayStatus(onFetching, onFetched, onIndexPacked, onIndexPushed, onManifestRemoved, onManifestAdded, onIndexMerged bool) status.ManifestIndexUpdateHandler { + return &testUpdateDisplayStatus{ + onFetchingError: onFetching, + onFetchedError: onFetched, + onIndexPackedError: onIndexPacked, + onIndexPushedError: onIndexPushed, + onManifestRemovedError: onManifestRemoved, + onManifestAddedError: onManifestAdded, + onIndexMergedError: onIndexMerged, + } +} + var ( A = ocispec.Descriptor{ MediaType: ocispec.MediaTypeImageManifest, @@ -69,7 +138,7 @@ func Test_doRemoveManifests(t *testing.T) { name: "remove one matched item", manifests: []ocispec.Descriptor{A, B, C}, digestSet: map[digest.Digest]bool{B.Digest: false}, - displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)), + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, false, false, false), indexRef: "test01", want: []ocispec.Descriptor{A, C}, wantErr: false, @@ -78,7 +147,7 @@ func Test_doRemoveManifests(t *testing.T) { name: "remove all matched items", manifests: []ocispec.Descriptor{A, B, A, C, A, A, A}, digestSet: map[digest.Digest]bool{A.Digest: false}, - displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)), + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, false, false, false), indexRef: "test02", want: []ocispec.Descriptor{B, C}, wantErr: false, @@ -87,7 +156,7 @@ func Test_doRemoveManifests(t *testing.T) { name: "remove correctly when there is only one item", manifests: []ocispec.Descriptor{A}, digestSet: map[digest.Digest]bool{A.Digest: false}, - displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)), + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, false, false, false), indexRef: "test03", want: []ocispec.Descriptor{}, wantErr: false, @@ -96,7 +165,7 @@ func Test_doRemoveManifests(t *testing.T) { name: "remove multiple distinct manifests", manifests: []ocispec.Descriptor{A, B, C}, digestSet: map[digest.Digest]bool{A.Digest: false, C.Digest: false}, - displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)), + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, false, false, false), indexRef: "test04", want: []ocispec.Descriptor{B}, wantErr: false, @@ -105,8 +174,8 @@ func Test_doRemoveManifests(t *testing.T) { name: "remove multiple duplicate manifests", manifests: []ocispec.Descriptor{A, B, C, C, B, A, B}, digestSet: map[digest.Digest]bool{A.Digest: false, C.Digest: false}, - displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)), - indexRef: "test04", + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, false, false, false), + indexRef: "test05", want: []ocispec.Descriptor{B, B, B}, wantErr: false, }, @@ -114,8 +183,17 @@ func Test_doRemoveManifests(t *testing.T) { name: "return error when deleting a nonexistent item", manifests: []ocispec.Descriptor{A, C}, digestSet: map[digest.Digest]bool{B.Digest: false}, - displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)), - indexRef: "test04", + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, false, false, false), + indexRef: "test06", + want: nil, + wantErr: true, + }, + { + name: "handler error", + manifests: []ocispec.Descriptor{A, B, C}, + digestSet: map[digest.Digest]bool{B.Digest: false}, + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, true, false, false), + indexRef: "test07", want: nil, wantErr: true, }, @@ -143,77 +221,6 @@ func Test_doRemoveManifests(t *testing.T) { } } -type testUpdateDisplayStatus struct { - onFetchingError bool - onFetchedError bool - onIndexPackedError bool - onIndexPushedError bool - onManifestRemovedError bool - onManifestAddedError bool - onIndexMergedError bool -} - -func (tds *testUpdateDisplayStatus) OnFetching(manifestRef string) error { - if tds.onFetchingError { - return fmt.Errorf("OnFetching error") - } - return nil -} - -func (tds *testUpdateDisplayStatus) OnFetched(manifestRef string, desc ocispec.Descriptor) error { - if tds.onFetchedError { - return fmt.Errorf("OnFetched error") - } - return nil -} - -func (tds *testUpdateDisplayStatus) OnIndexPacked(desc ocispec.Descriptor) error { - if tds.onIndexPackedError { - return fmt.Errorf("error") - } - return nil -} - -func (tds *testUpdateDisplayStatus) OnIndexPushed(path string) error { - if tds.onIndexPushedError { - return fmt.Errorf("error") - } - return nil -} - -func (tds *testUpdateDisplayStatus) OnManifestRemoved(digest digest.Digest) error { - if tds.onManifestRemovedError { - return fmt.Errorf("error") - } - return nil -} - -func (tds *testUpdateDisplayStatus) OnManifestAdded(manifestRef string, desc ocispec.Descriptor) error { - if tds.onManifestAddedError { - return fmt.Errorf("error") - } - return nil -} - -func (tds *testUpdateDisplayStatus) OnIndexMerged(indexRef string, desc ocispec.Descriptor) error { - if tds.onIndexMergedError { - return fmt.Errorf("error") - } - return nil -} - -func NewTestUpdateDisplayStatus(onFetching, onFetched, onIndexPacked, onIndexPushed, onManifestRemoved, onManifestAdded, onIndexMerged bool) status.ManifestIndexUpdateHandler { - return &testUpdateDisplayStatus{ - onFetchingError: onFetching, - onFetchedError: onFetched, - onIndexPackedError: onIndexPacked, - onIndexPushedError: onIndexPushed, - onManifestRemovedError: onManifestRemoved, - onManifestAddedError: onManifestAdded, - onIndexMergedError: onIndexMerged, - } -} - func Test_fetchIndex(t *testing.T) { testContext := context.Background() tests := []struct { @@ -266,3 +273,60 @@ func Test_fetchIndex(t *testing.T) { }) } } + +func Test_mergeIndexes(t *testing.T) { + testContext := context.Background() + tests := []struct { + name string + ctx context.Context + displayStatus status.ManifestIndexUpdateHandler + manifests []ocispec.Descriptor + target oras.ReadOnlyTarget + mergeArguments []string + want []ocispec.Descriptor + wantErr bool + }{ + { + name: "OnFetching error", + ctx: testContext, + displayStatus: NewTestUpdateDisplayStatus(true, false, false, false, false, false, false), + manifests: []ocispec.Descriptor{}, + target: NewTestReadOnlyTarget("index"), + mergeArguments: []string{"test"}, + want: nil, + wantErr: true, + }, + { + name: "OnFetched error", + ctx: testContext, + displayStatus: NewTestUpdateDisplayStatus(false, true, false, false, false, false, false), + manifests: []ocispec.Descriptor{}, + target: NewTestReadOnlyTarget("index"), + mergeArguments: []string{"test"}, + want: nil, + wantErr: true, + }, + { + name: "unmarshall error", + ctx: testContext, + displayStatus: NewTestUpdateDisplayStatus(false, false, false, false, false, false, false), + manifests: []ocispec.Descriptor{}, + target: NewTestReadOnlyTarget("index"), + mergeArguments: []string{"test"}, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := mergeIndexes(tt.ctx, tt.displayStatus, tt.manifests, tt.target, tt.mergeArguments) + if (err != nil) != tt.wantErr { + t.Errorf("mergeIndexes() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("mergeIndexes() = %v, want %v", got, tt.want) + } + }) + } +}