From a68e8e29ce5b502fdbb86e95ce58bbab8cd92851 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Tue, 25 Jun 2024 14:48:05 +0000
Subject: [PATCH 01/13] add units tests for init and reject commands

Signed-off-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
---
 pkg/cli/commands/rpkg/init/command_test.go   | 111 ++++++++++++++++
 pkg/cli/commands/rpkg/reject/command_test.go | 131 +++++++++++++++++++
 2 files changed, 242 insertions(+)
 create mode 100644 pkg/cli/commands/rpkg/init/command_test.go
 create mode 100644 pkg/cli/commands/rpkg/reject/command_test.go

diff --git a/pkg/cli/commands/rpkg/init/command_test.go b/pkg/cli/commands/rpkg/init/command_test.go
new file mode 100644
index 00000000..f1a4e548
--- /dev/null
+++ b/pkg/cli/commands/rpkg/init/command_test.go
@@ -0,0 +1,111 @@
+// Copyright 2022 The kpt and Nephio 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 init
+
+import (
+	"context"
+	"io"
+	"os"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
+	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+)
+
+func createScheme() (*runtime.Scheme, error) {
+	scheme := runtime.NewScheme()
+
+	for _, api := range (runtime.SchemeBuilder{
+		porchapi.AddToScheme,
+	}) {
+		if err := api(scheme); err != nil {
+			return nil, err
+		}
+	}
+	return scheme, nil
+}
+
+func TestCmd(t *testing.T) {
+	var scheme, err = createScheme()
+	if err != nil {
+		t.Fatalf("error creating scheme: %v", err)
+	}
+	testCases := map[string]struct {
+		output     string
+		wantErr    bool
+		ns         string
+		fakeclient client.WithWatch
+	}{
+		"Repo not in ns": {
+			ns:         "doesnotexist",
+			wantErr:    true,
+			fakeclient: fake.NewClientBuilder().Build(),
+		},
+		"successful init": {
+			ns:     "ns",
+			output: "pr created\n",
+			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
+				Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+					if obj.GetObjectKind().GroupVersionKind().Kind == "PackageRevision" {
+						obj.SetName("pr")
+					}
+					return nil
+				},
+			}).WithScheme(scheme).Build(),
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
+
+			cmd := &cobra.Command{}
+			o := os.Stdout
+			e := os.Stderr
+			read, write, _ := os.Pipe()
+			os.Stdout = write
+			os.Stderr = write
+
+			r := &runner{
+				ctx: context.Background(),
+				cfg: &genericclioptions.ConfigFlags{
+					Namespace: &tc.ns,
+				},
+				client:  tc.fakeclient,
+				Command: cmd,
+			}
+			go func() {
+				defer write.Close()
+				err := r.runE(cmd, []string{})
+				if err != nil && !tc.wantErr {
+					t.Errorf("unexpected error: %v", err)
+				}
+			}()
+			out, _ := io.ReadAll(read)
+			os.Stdout = o
+			os.Stderr = e
+
+			if diff := cmp.Diff(string(tc.output), string(out)); diff != "" {
+				t.Errorf("Unexpected result (-want, +got): %s", diff)
+			}
+		})
+	}
+}
diff --git a/pkg/cli/commands/rpkg/reject/command_test.go b/pkg/cli/commands/rpkg/reject/command_test.go
new file mode 100644
index 00000000..ab04cd58
--- /dev/null
+++ b/pkg/cli/commands/rpkg/reject/command_test.go
@@ -0,0 +1,131 @@
+// Copyright 2022 The kpt and Nephio 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 reject
+
+import (
+	"context"
+	"io"
+	"os"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
+	"github.com/spf13/cobra"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"k8s.io/client-go/rest"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func createScheme() (*runtime.Scheme, error) {
+	scheme := runtime.NewScheme()
+	for _, api := range (runtime.SchemeBuilder{
+		porchapi.AddToScheme,
+	}) {
+		if err := api(scheme); err != nil {
+			return nil, err
+		}
+	}
+	scheme.AddKnownTypes(porchapi.SchemeGroupVersion, &porchapi.PackageRevision{})
+	return scheme, nil
+}
+
+func TestCmd(t *testing.T) {
+	pkgRevName := "test-fjdos9u2nfe2f32"
+	var scheme, err = createScheme()
+	if err != nil {
+		t.Fatalf("error creating scheme: %v", err)
+	}
+	testCases := map[string]struct {
+		output  string
+		wantErr bool
+		ns      string
+		lc      porchapi.PackageRevisionLifecycle
+	}{
+		"Package not found in ns": {
+			ns:      "doesnotexist",
+			wantErr: true,
+		},
+		"Reject deletion proposed": {
+			ns: "ns",
+			lc: porchapi.PackageRevisionLifecycleDeletionProposed,
+
+			output: pkgRevName + " no longer proposed for deletion\n",
+		},
+		"Reject proposed": {
+			ns:      "ns",
+			wantErr: false,
+			lc:      porchapi.PackageRevisionLifecycleProposed,
+			output:  pkgRevName + " rejected\n",
+		},
+		"Reject draft": {
+			ns:      "ns",
+			lc:      porchapi.PackageRevisionLifecycleDraft,
+			output:  "cannot reject " + pkgRevName + " with lifecycle 'Draft'\n",
+			wantErr: true,
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
+			cmd := &cobra.Command{}
+			o := os.Stdout
+			e := os.Stderr
+			read, write, _ := os.Pipe()
+			os.Stdout = write
+			os.Stderr = write
+
+			c := fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle: tc.lc,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "ns",
+						Name:      pkgRevName,
+					}}).Build()
+
+			r := &runner{
+				ctx: context.Background(),
+				cfg: &genericclioptions.ConfigFlags{
+					Namespace: &tc.ns,
+				},
+				client:      &rest.RESTClient{},
+				porchClient: c,
+				Command:     cmd,
+			}
+			go func() {
+				defer write.Close()
+				err := r.runE(cmd, []string{pkgRevName})
+				if err != nil && !tc.wantErr {
+					t.Errorf("unexpected error: %v", err)
+				}
+			}()
+			out, _ := io.ReadAll(read)
+			os.Stdout = o
+			os.Stderr = e
+
+			if diff := cmp.Diff(string(tc.output), string(out)); diff != "" {
+				t.Errorf("Unexpected result (-want, +got): %s", diff)
+			}
+		})
+	}
+}

From 18860b1f6032e2e2ebd7d451eeb1d83cc748fe89 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Wed, 26 Jun 2024 11:07:15 +0000
Subject: [PATCH 02/13] update rpkg reject unit tests(in-progress)

---
 internal/kpt/util/porch/approval.go          |  30 +-----
 pkg/cli/commands/rpkg/approve/command.go     |  10 +-
 pkg/cli/commands/rpkg/reject/command.go      |  27 ++---
 pkg/cli/commands/rpkg/reject/command_test.go | 103 +++++++++++++------
 4 files changed, 85 insertions(+), 85 deletions(-)

diff --git a/internal/kpt/util/porch/approval.go b/internal/kpt/util/porch/approval.go
index cec8bfd7..74d7a3b9 100644
--- a/internal/kpt/util/porch/approval.go
+++ b/internal/kpt/util/porch/approval.go
@@ -19,27 +19,13 @@ import (
 	"fmt"
 
 	"github.com/nephio-project/porch/api/porch/v1alpha1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func UpdatePackageRevisionApproval(ctx context.Context, client rest.Interface, key client.ObjectKey, new v1alpha1.PackageRevisionLifecycle) error {
-	scheme := runtime.NewScheme()
-	if err := v1alpha1.SchemeBuilder.AddToScheme(scheme); err != nil {
-		return err
-	}
+func UpdatePackageRevisionApproval(ctx context.Context, client client.Client, key client.ObjectKey, new v1alpha1.PackageRevisionLifecycle) error {
 
-	codec := runtime.NewParameterCodec(scheme)
 	var pr v1alpha1.PackageRevision
-	if err := client.Get().
-		Namespace(key.Namespace).
-		Resource("packagerevisions").
-		Name(key.Name).
-		VersionedParams(&metav1.GetOptions{}, codec).
-		Do(ctx).
-		Into(&pr); err != nil {
+	if err := client.Get(ctx, key, &pr); err != nil {
 		return err
 	}
 
@@ -56,15 +42,5 @@ func UpdatePackageRevisionApproval(ctx context.Context, client rest.Interface, k
 	// Approve - change the package revision kind to "final".
 	pr.Spec.Lifecycle = new
 
-	opts := metav1.UpdateOptions{}
-	result := &v1alpha1.PackageRevision{}
-	return client.Put().
-		Namespace(pr.Namespace).
-		Resource("packagerevisions").
-		Name(pr.Name).
-		SubResource("approval").
-		VersionedParams(&opts, codec).
-		Body(&pr).
-		Do(ctx).
-		Into(result)
+	return client.Update(ctx, &pr)
 }
diff --git a/pkg/cli/commands/rpkg/approve/command.go b/pkg/cli/commands/rpkg/approve/command.go
index 7170f0cc..2e6c6cbe 100644
--- a/pkg/cli/commands/rpkg/approve/command.go
+++ b/pkg/cli/commands/rpkg/approve/command.go
@@ -25,7 +25,6 @@ import (
 	"github.com/nephio-project/porch/pkg/cli/commands/rpkg/docs"
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -39,9 +38,8 @@ func NewCommand(ctx context.Context, rcg *genericclioptions.ConfigFlags) *cobra.
 
 func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner {
 	r := &runner{
-		ctx:    ctx,
-		cfg:    rcg,
-		client: nil,
+		ctx: ctx,
+		cfg: rcg,
 	}
 
 	c := &cobra.Command{
@@ -61,7 +59,7 @@ func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner
 type runner struct {
 	ctx     context.Context
 	cfg     *genericclioptions.ConfigFlags
-	client  rest.Interface
+	client  client.Client
 	Command *cobra.Command
 
 	// Flags
@@ -70,7 +68,7 @@ type runner struct {
 func (r *runner) preRunE(_ *cobra.Command, _ []string) error {
 	const op errors.Op = command + ".preRunE"
 
-	client, err := porch.CreateRESTClient(r.cfg)
+	client, err := porch.CreateClientWithFlags(r.cfg)
 	if err != nil {
 		return errors.E(op, err)
 	}
diff --git a/pkg/cli/commands/rpkg/reject/command.go b/pkg/cli/commands/rpkg/reject/command.go
index 10ed2ea4..52f23eb0 100644
--- a/pkg/cli/commands/rpkg/reject/command.go
+++ b/pkg/cli/commands/rpkg/reject/command.go
@@ -25,7 +25,6 @@ import (
 	"github.com/nephio-project/porch/pkg/cli/commands/rpkg/docs"
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -39,9 +38,8 @@ func NewCommand(ctx context.Context, rcg *genericclioptions.ConfigFlags) *cobra.
 
 func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner {
 	r := &runner{
-		ctx:    ctx,
-		cfg:    rcg,
-		client: nil,
+		ctx: ctx,
+		cfg: rcg,
 	}
 
 	c := &cobra.Command{
@@ -59,11 +57,10 @@ func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner
 }
 
 type runner struct {
-	ctx         context.Context
-	cfg         *genericclioptions.ConfigFlags
-	client      rest.Interface
-	porchClient client.Client
-	Command     *cobra.Command
+	ctx     context.Context
+	cfg     *genericclioptions.ConfigFlags
+	client  client.Client
+	Command *cobra.Command
 
 	// Flags
 }
@@ -75,17 +72,11 @@ func (r *runner) preRunE(_ *cobra.Command, args []string) error {
 		return errors.E(op, "PACKAGE_REVISION is a required positional argument")
 	}
 
-	client, err := porch.CreateRESTClient(r.cfg)
+	client, err := porch.CreateClientWithFlags(r.cfg)
 	if err != nil {
 		return errors.E(op, err)
 	}
 	r.client = client
-
-	porchClient, err := porch.CreateClientWithFlags(r.cfg)
-	if err != nil {
-		return errors.E(op, err)
-	}
-	r.porchClient = porchClient
 	return nil
 }
 
@@ -97,7 +88,7 @@ func (r *runner) runE(_ *cobra.Command, args []string) error {
 
 	for _, name := range args {
 		pr := &v1alpha1.PackageRevision{}
-		if err := r.porchClient.Get(r.ctx, client.ObjectKey{
+		if err := r.client.Get(r.ctx, client.ObjectKey{
 			Namespace: namespace,
 			Name:      name,
 		}, pr); err != nil {
@@ -116,7 +107,7 @@ func (r *runner) runE(_ *cobra.Command, args []string) error {
 			}
 		case v1alpha1.PackageRevisionLifecycleDeletionProposed:
 			pr.Spec.Lifecycle = v1alpha1.PackageRevisionLifecyclePublished
-			if err := r.porchClient.Update(r.ctx, pr); err != nil {
+			if err := r.client.Update(r.ctx, pr); err != nil {
 				messages = append(messages, err.Error())
 				fmt.Fprintf(r.Command.ErrOrStderr(), "%s failed (%s)\n", name, err)
 			} else {
diff --git a/pkg/cli/commands/rpkg/reject/command_test.go b/pkg/cli/commands/rpkg/reject/command_test.go
index ab04cd58..e3b82759 100644
--- a/pkg/cli/commands/rpkg/reject/command_test.go
+++ b/pkg/cli/commands/rpkg/reject/command_test.go
@@ -26,7 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-	"k8s.io/client-go/rest"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
@@ -44,7 +44,9 @@ func createScheme() (*runtime.Scheme, error) {
 }
 
 func TestCmd(t *testing.T) {
-	pkgRevName := "test-fjdos9u2nfe2f32"
+	pkgRevName := "test-pr"
+	repoName := "test-repo"
+	ns := "ns"
 	var scheme, err = createScheme()
 	if err != nil {
 		t.Fatalf("error creating scheme: %v", err)
@@ -52,36 +54,84 @@ func TestCmd(t *testing.T) {
 	testCases := map[string]struct {
 		output  string
 		wantErr bool
-		ns      string
-		lc      porchapi.PackageRevisionLifecycle
+		fc      client.WithWatch
 	}{
 		"Package not found in ns": {
-			ns:      "doesnotexist",
 			wantErr: true,
+			fc: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle: porchapi.PackageRevisionLifecycleDraft,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "dummy-ns",
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Reject deletion proposed": {
-			ns: "ns",
-			lc: porchapi.PackageRevisionLifecycleDeletionProposed,
-
 			output: pkgRevName + " no longer proposed for deletion\n",
+			fc: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleDeletionProposed,
+						RepositoryName: repoName,
+						PackageName:    pkgRevName,
+						WorkspaceName:  "v1",
+						Revision:       "latest",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
+		},
+		"Reject draft": {
+			wantErr: true,
+			output:  "cannot reject " + pkgRevName + " with lifecycle 'Draft'\n",
+			fc: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle: porchapi.PackageRevisionLifecycleDraft,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Reject proposed": {
-			ns:      "ns",
 			wantErr: false,
-			lc:      porchapi.PackageRevisionLifecycleProposed,
 			output:  pkgRevName + " rejected\n",
-		},
-		"Reject draft": {
-			ns:      "ns",
-			lc:      porchapi.PackageRevisionLifecycleDraft,
-			output:  "cannot reject " + pkgRevName + " with lifecycle 'Draft'\n",
-			wantErr: true,
+			fc: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects([]client.Object{&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle: porchapi.PackageRevisionLifecycleProposed,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}}...).Build(),
 		},
 	}
 
 	for tn := range testCases {
 		tc := testCases[tn]
 		t.Run(tn, func(t *testing.T) {
+
 			cmd := &cobra.Command{}
 			o := os.Stdout
 			e := os.Stderr
@@ -89,28 +139,13 @@ func TestCmd(t *testing.T) {
 			os.Stdout = write
 			os.Stderr = write
 
-			c := fake.NewClientBuilder().WithScheme(scheme).
-				WithObjects(&porchapi.PackageRevision{
-					TypeMeta: metav1.TypeMeta{
-						Kind:       "PackageRevision",
-						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
-					},
-					Spec: porchapi.PackageRevisionSpec{
-						Lifecycle: tc.lc,
-					},
-					ObjectMeta: metav1.ObjectMeta{
-						Namespace: "ns",
-						Name:      pkgRevName,
-					}}).Build()
-
 			r := &runner{
 				ctx: context.Background(),
 				cfg: &genericclioptions.ConfigFlags{
-					Namespace: &tc.ns,
+					Namespace: &ns,
 				},
-				client:      &rest.RESTClient{},
-				porchClient: c,
-				Command:     cmd,
+				client:  tc.fc,
+				Command: cmd,
 			}
 			go func() {
 				defer write.Close()

From bd0ac63574fe163bf1e8407a1a7f0a6c041b1026 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Wed, 26 Jun 2024 11:44:22 +0000
Subject: [PATCH 03/13] add units tests for approve command

---
 pkg/cli/commands/rpkg/approve/command_test.go | 137 ++++++++++++++++++
 pkg/cli/commands/rpkg/reject/command_test.go  |  91 ++++--------
 2 files changed, 168 insertions(+), 60 deletions(-)
 create mode 100644 pkg/cli/commands/rpkg/approve/command_test.go

diff --git a/pkg/cli/commands/rpkg/approve/command_test.go b/pkg/cli/commands/rpkg/approve/command_test.go
new file mode 100644
index 00000000..c32703cf
--- /dev/null
+++ b/pkg/cli/commands/rpkg/approve/command_test.go
@@ -0,0 +1,137 @@
+// Copyright 2022 The kpt and Nephio 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 approve
+
+import (
+	"context"
+	"io"
+	"os"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
+	"github.com/spf13/cobra"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func createScheme() (*runtime.Scheme, error) {
+	scheme := runtime.NewScheme()
+	for _, api := range (runtime.SchemeBuilder{
+		porchapi.AddToScheme,
+	}) {
+		if err := api(scheme); err != nil {
+			return nil, err
+		}
+	}
+	scheme.AddKnownTypes(porchapi.SchemeGroupVersion, &porchapi.PackageRevision{})
+	return scheme, nil
+}
+
+func TestCmd(t *testing.T) {
+	pkgRevName := "test-pr"
+	repoName := "test-repo"
+	ns := "ns"
+	var scheme, err = createScheme()
+	if err != nil {
+		t.Fatalf("error creating scheme: %v", err)
+	}
+	testCases := map[string]struct {
+		output  string
+		wantErr bool
+		lc      porchapi.PackageRevisionLifecycle
+		ns      string
+	}{
+		"Package not found in ns": {
+			wantErr: true,
+			output:  pkgRevName + " failed (packagerevisions.porch.kpt.dev \"" + pkgRevName + "\" not found)\n",
+			ns:      "dummy",
+		},
+		"Approve draft package": {
+			wantErr: true,
+			output:  pkgRevName + " failed (cannot change approval from Draft to Published)\n",
+			ns:      ns,
+			lc:      porchapi.PackageRevisionLifecycleDraft,
+		},
+		"Approve published package": {
+			output: pkgRevName + " approved\n",
+			ns:     ns,
+			lc:     porchapi.PackageRevisionLifecyclePublished,
+		},
+		"Approve deletion-proposed package": {
+			output: pkgRevName + " approved\n",
+			ns:     ns,
+			lc:     porchapi.PackageRevisionLifecycleDeletionProposed,
+		},
+		"Approve proposed package": {
+			output: pkgRevName + " approved\n",
+			ns:     ns,
+			lc:     porchapi.PackageRevisionLifecycleProposed,
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
+
+			c := fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      tc.lc,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: tc.ns,
+						Name:      pkgRevName,
+					}}).Build()
+
+			cmd := &cobra.Command{}
+			o := os.Stdout
+			e := os.Stderr
+			read, write, _ := os.Pipe()
+			os.Stdout = write
+			os.Stderr = write
+
+			r := &runner{
+				ctx: context.Background(),
+				cfg: &genericclioptions.ConfigFlags{
+					Namespace: &ns,
+				},
+				client:  c,
+				Command: cmd,
+			}
+			go func() {
+				defer write.Close()
+				err := r.runE(cmd, []string{pkgRevName})
+				if err != nil && !tc.wantErr {
+					t.Errorf("unexpected error: %v", err)
+				}
+			}()
+			out, _ := io.ReadAll(read)
+			os.Stdout = o
+			os.Stderr = e
+
+			if diff := cmp.Diff(string(tc.output), string(out)); diff != "" {
+				t.Errorf("Unexpected result (-want, +got): %s", diff)
+			}
+		})
+	}
+}
diff --git a/pkg/cli/commands/rpkg/reject/command_test.go b/pkg/cli/commands/rpkg/reject/command_test.go
index e3b82759..eb2bcd7a 100644
--- a/pkg/cli/commands/rpkg/reject/command_test.go
+++ b/pkg/cli/commands/rpkg/reject/command_test.go
@@ -26,7 +26,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
@@ -54,83 +53,55 @@ func TestCmd(t *testing.T) {
 	testCases := map[string]struct {
 		output  string
 		wantErr bool
-		fc      client.WithWatch
+		lc      porchapi.PackageRevisionLifecycle
+		ns      string
 	}{
 		"Package not found in ns": {
 			wantErr: true,
-			fc: fake.NewClientBuilder().WithScheme(scheme).
-				WithObjects(&porchapi.PackageRevision{
-					TypeMeta: metav1.TypeMeta{
-						Kind:       "PackageRevision",
-						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
-					},
-					Spec: porchapi.PackageRevisionSpec{
-						Lifecycle: porchapi.PackageRevisionLifecycleDraft,
-					},
-					ObjectMeta: metav1.ObjectMeta{
-						Namespace: "dummy-ns",
-						Name:      pkgRevName,
-					}}).Build(),
 		},
-		"Reject deletion proposed": {
+		"Reject deletion-proposed package": {
 			output: pkgRevName + " no longer proposed for deletion\n",
-			fc: fake.NewClientBuilder().WithScheme(scheme).
-				WithObjects(&porchapi.PackageRevision{
-					TypeMeta: metav1.TypeMeta{
-						Kind:       "PackageRevision",
-						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
-					},
-					Spec: porchapi.PackageRevisionSpec{
-						Lifecycle:      porchapi.PackageRevisionLifecycleDeletionProposed,
-						RepositoryName: repoName,
-						PackageName:    pkgRevName,
-						WorkspaceName:  "v1",
-						Revision:       "latest",
-					},
-					ObjectMeta: metav1.ObjectMeta{
-						Namespace: ns,
-						Name:      pkgRevName,
-					}}).Build(),
+			ns:     ns,
+			lc:     porchapi.PackageRevisionLifecycleDeletionProposed,
 		},
-		"Reject draft": {
+		"Reject draft package": {
 			wantErr: true,
 			output:  "cannot reject " + pkgRevName + " with lifecycle 'Draft'\n",
-			fc: fake.NewClientBuilder().WithScheme(scheme).
-				WithObjects(&porchapi.PackageRevision{
-					TypeMeta: metav1.TypeMeta{
-						Kind:       "PackageRevision",
-						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
-					},
-					Spec: porchapi.PackageRevisionSpec{
-						Lifecycle: porchapi.PackageRevisionLifecycleDraft,
-					},
-					ObjectMeta: metav1.ObjectMeta{
-						Namespace: ns,
-						Name:      pkgRevName,
-					}}).Build(),
+			ns:      ns,
+			lc:      porchapi.PackageRevisionLifecycleDraft,
 		},
-		"Reject proposed": {
+		"Reject published package": {
+			wantErr: true,
+			output:  "cannot reject " + pkgRevName + " with lifecycle 'Published'\n",
+			ns:      ns,
+			lc:      porchapi.PackageRevisionLifecyclePublished,
+		},
+		"Reject proposed package": {
 			wantErr: false,
 			output:  pkgRevName + " rejected\n",
-			fc: fake.NewClientBuilder().WithScheme(scheme).
-				WithObjects([]client.Object{&porchapi.PackageRevision{
+			ns:      ns,
+			lc:      porchapi.PackageRevisionLifecycleProposed,
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
+
+			c := fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
 					TypeMeta: metav1.TypeMeta{
 						Kind:       "PackageRevision",
 						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
 					},
 					Spec: porchapi.PackageRevisionSpec{
-						Lifecycle: porchapi.PackageRevisionLifecycleProposed,
+						Lifecycle:      tc.lc,
+						RepositoryName: repoName,
 					},
 					ObjectMeta: metav1.ObjectMeta{
-						Namespace: ns,
+						Namespace: tc.ns,
 						Name:      pkgRevName,
-					}}}...).Build(),
-		},
-	}
-
-	for tn := range testCases {
-		tc := testCases[tn]
-		t.Run(tn, func(t *testing.T) {
+					}}).Build()
 
 			cmd := &cobra.Command{}
 			o := os.Stdout
@@ -144,7 +115,7 @@ func TestCmd(t *testing.T) {
 				cfg: &genericclioptions.ConfigFlags{
 					Namespace: &ns,
 				},
-				client:  tc.fc,
+				client:  c,
 				Command: cmd,
 			}
 			go func() {

From a14e99119f1abadb06a1b4262634c935adb42c90 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Wed, 26 Jun 2024 12:07:14 +0000
Subject: [PATCH 04/13] remove approval.go

---
 internal/kpt/util/porch/approval.go           | 46 -------------------
 pkg/cli/commands/rpkg/approve/command.go      | 26 ++++++++---
 pkg/cli/commands/rpkg/approve/command_test.go | 10 ++--
 pkg/cli/commands/rpkg/reject/command.go       |  6 +--
 4 files changed, 26 insertions(+), 62 deletions(-)
 delete mode 100644 internal/kpt/util/porch/approval.go

diff --git a/internal/kpt/util/porch/approval.go b/internal/kpt/util/porch/approval.go
deleted file mode 100644
index 74d7a3b9..00000000
--- a/internal/kpt/util/porch/approval.go
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright 2022 The kpt and Nephio 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 porch
-
-import (
-	"context"
-	"fmt"
-
-	"github.com/nephio-project/porch/api/porch/v1alpha1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-)
-
-func UpdatePackageRevisionApproval(ctx context.Context, client client.Client, key client.ObjectKey, new v1alpha1.PackageRevisionLifecycle) error {
-
-	var pr v1alpha1.PackageRevision
-	if err := client.Get(ctx, key, &pr); err != nil {
-		return err
-	}
-
-	switch lifecycle := pr.Spec.Lifecycle; lifecycle {
-	case v1alpha1.PackageRevisionLifecycleProposed, v1alpha1.PackageRevisionLifecycleDeletionProposed:
-		// ok
-	case new:
-		// already correct value
-		return nil
-	default:
-		return fmt.Errorf("cannot change approval from %s to %s", lifecycle, new)
-	}
-
-	// Approve - change the package revision kind to "final".
-	pr.Spec.Lifecycle = new
-
-	return client.Update(ctx, &pr)
-}
diff --git a/pkg/cli/commands/rpkg/approve/command.go b/pkg/cli/commands/rpkg/approve/command.go
index 2e6c6cbe..4a0e8182 100644
--- a/pkg/cli/commands/rpkg/approve/command.go
+++ b/pkg/cli/commands/rpkg/approve/command.go
@@ -79,18 +79,30 @@ func (r *runner) preRunE(_ *cobra.Command, _ []string) error {
 func (r *runner) runE(_ *cobra.Command, args []string) error {
 	const op errors.Op = command + ".runE"
 	var messages []string
-
 	namespace := *r.cfg.Namespace
 
 	for _, name := range args {
-		if err := porch.UpdatePackageRevisionApproval(r.ctx, r.client, client.ObjectKey{
+		pr := &v1alpha1.PackageRevision{}
+		if err := r.client.Get(r.ctx, client.ObjectKey{
 			Namespace: namespace,
 			Name:      name,
-		}, v1alpha1.PackageRevisionLifecyclePublished); err != nil {
-			messages = append(messages, err.Error())
-			fmt.Fprintf(r.Command.ErrOrStderr(), "%s failed (%s)\n", name, err)
-		} else {
-			fmt.Fprintf(r.Command.OutOrStdout(), "%s approved\n", name)
+		}, pr); err != nil {
+			return errors.E(op, err)
+		}
+
+		switch pr.Spec.Lifecycle {
+		case v1alpha1.PackageRevisionLifecycleProposed, v1alpha1.PackageRevisionLifecycleDeletionProposed:
+			pr.Spec.Lifecycle = v1alpha1.PackageRevisionLifecyclePublished
+			if err := r.client.Update(r.ctx, pr); err != nil {
+				messages = append(messages, err.Error())
+				fmt.Fprintf(r.Command.ErrOrStderr(), "%s failed (%s)\n", name, err)
+			} else {
+				fmt.Fprintf(r.Command.OutOrStdout(), "%s approved\n", name)
+			}
+		default:
+			msg := fmt.Sprintf("cannot approve %s with lifecycle '%s'", name, pr.Spec.Lifecycle)
+			messages = append(messages, msg)
+			fmt.Fprintln(r.Command.ErrOrStderr(), msg)
 		}
 	}
 
diff --git a/pkg/cli/commands/rpkg/approve/command_test.go b/pkg/cli/commands/rpkg/approve/command_test.go
index c32703cf..d7c22173 100644
--- a/pkg/cli/commands/rpkg/approve/command_test.go
+++ b/pkg/cli/commands/rpkg/approve/command_test.go
@@ -58,19 +58,19 @@ func TestCmd(t *testing.T) {
 	}{
 		"Package not found in ns": {
 			wantErr: true,
-			output:  pkgRevName + " failed (packagerevisions.porch.kpt.dev \"" + pkgRevName + "\" not found)\n",
 			ns:      "dummy",
 		},
 		"Approve draft package": {
 			wantErr: true,
-			output:  pkgRevName + " failed (cannot change approval from Draft to Published)\n",
+			output:  "cannot approve " + pkgRevName + " with lifecycle 'Draft'\n",
 			ns:      ns,
 			lc:      porchapi.PackageRevisionLifecycleDraft,
 		},
 		"Approve published package": {
-			output: pkgRevName + " approved\n",
-			ns:     ns,
-			lc:     porchapi.PackageRevisionLifecyclePublished,
+			wantErr: true,
+			output:  "cannot approve " + pkgRevName + " with lifecycle 'Published'\n",
+			ns:      ns,
+			lc:      porchapi.PackageRevisionLifecyclePublished,
 		},
 		"Approve deletion-proposed package": {
 			output: pkgRevName + " approved\n",
diff --git a/pkg/cli/commands/rpkg/reject/command.go b/pkg/cli/commands/rpkg/reject/command.go
index 52f23eb0..6d63b5f2 100644
--- a/pkg/cli/commands/rpkg/reject/command.go
+++ b/pkg/cli/commands/rpkg/reject/command.go
@@ -96,10 +96,8 @@ func (r *runner) runE(_ *cobra.Command, args []string) error {
 		}
 		switch pr.Spec.Lifecycle {
 		case v1alpha1.PackageRevisionLifecycleProposed:
-			if err := porch.UpdatePackageRevisionApproval(r.ctx, r.client, client.ObjectKey{
-				Namespace: namespace,
-				Name:      name,
-			}, v1alpha1.PackageRevisionLifecycleDraft); err != nil {
+			pr.Spec.Lifecycle = v1alpha1.PackageRevisionLifecycleDraft
+			if err := r.client.Update(r.ctx, pr); err != nil {
 				messages = append(messages, err.Error())
 				fmt.Fprintf(r.Command.ErrOrStderr(), "%s failed (%s)\n", name, err)
 			} else {

From 7e95895fc60171797b9776cb8f4ec4cbc90f5eca Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Wed, 26 Jun 2024 13:44:38 +0000
Subject: [PATCH 05/13] update reject and approve unit tests

---
 pkg/cli/commands/rpkg/approve/command_test.go | 2 +-
 pkg/cli/commands/rpkg/reject/command_test.go  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/cli/commands/rpkg/approve/command_test.go b/pkg/cli/commands/rpkg/approve/command_test.go
index d7c22173..08a8d418 100644
--- a/pkg/cli/commands/rpkg/approve/command_test.go
+++ b/pkg/cli/commands/rpkg/approve/command_test.go
@@ -122,7 +122,7 @@ func TestCmd(t *testing.T) {
 				defer write.Close()
 				err := r.runE(cmd, []string{pkgRevName})
 				if err != nil && !tc.wantErr {
-					t.Errorf("unexpected error: %v", err)
+					t.Errorf("unexpected error: %v", err.Error())
 				}
 			}()
 			out, _ := io.ReadAll(read)
diff --git a/pkg/cli/commands/rpkg/reject/command_test.go b/pkg/cli/commands/rpkg/reject/command_test.go
index eb2bcd7a..cb6f63ad 100644
--- a/pkg/cli/commands/rpkg/reject/command_test.go
+++ b/pkg/cli/commands/rpkg/reject/command_test.go
@@ -122,7 +122,7 @@ func TestCmd(t *testing.T) {
 				defer write.Close()
 				err := r.runE(cmd, []string{pkgRevName})
 				if err != nil && !tc.wantErr {
-					t.Errorf("unexpected error: %v", err)
+					t.Errorf("unexpected error: %v", err.Error())
 				}
 			}()
 			out, _ := io.ReadAll(read)

From edf14f357e2269c043483057458ea8b23dc15445 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Wed, 26 Jun 2024 14:05:00 +0000
Subject: [PATCH 06/13] Revert "remove approval.go"

This reverts commit a14e99119f1abadb06a1b4262634c935adb42c90.
---
 internal/kpt/util/porch/approval.go           | 46 +++++++++++++++++++
 pkg/cli/commands/rpkg/approve/command.go      | 26 +++--------
 pkg/cli/commands/rpkg/approve/command_test.go | 10 ++--
 pkg/cli/commands/rpkg/reject/command.go       |  6 ++-
 4 files changed, 62 insertions(+), 26 deletions(-)
 create mode 100644 internal/kpt/util/porch/approval.go

diff --git a/internal/kpt/util/porch/approval.go b/internal/kpt/util/porch/approval.go
new file mode 100644
index 00000000..74d7a3b9
--- /dev/null
+++ b/internal/kpt/util/porch/approval.go
@@ -0,0 +1,46 @@
+// Copyright 2022 The kpt and Nephio 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 porch
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/nephio-project/porch/api/porch/v1alpha1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func UpdatePackageRevisionApproval(ctx context.Context, client client.Client, key client.ObjectKey, new v1alpha1.PackageRevisionLifecycle) error {
+
+	var pr v1alpha1.PackageRevision
+	if err := client.Get(ctx, key, &pr); err != nil {
+		return err
+	}
+
+	switch lifecycle := pr.Spec.Lifecycle; lifecycle {
+	case v1alpha1.PackageRevisionLifecycleProposed, v1alpha1.PackageRevisionLifecycleDeletionProposed:
+		// ok
+	case new:
+		// already correct value
+		return nil
+	default:
+		return fmt.Errorf("cannot change approval from %s to %s", lifecycle, new)
+	}
+
+	// Approve - change the package revision kind to "final".
+	pr.Spec.Lifecycle = new
+
+	return client.Update(ctx, &pr)
+}
diff --git a/pkg/cli/commands/rpkg/approve/command.go b/pkg/cli/commands/rpkg/approve/command.go
index 4a0e8182..2e6c6cbe 100644
--- a/pkg/cli/commands/rpkg/approve/command.go
+++ b/pkg/cli/commands/rpkg/approve/command.go
@@ -79,30 +79,18 @@ func (r *runner) preRunE(_ *cobra.Command, _ []string) error {
 func (r *runner) runE(_ *cobra.Command, args []string) error {
 	const op errors.Op = command + ".runE"
 	var messages []string
+
 	namespace := *r.cfg.Namespace
 
 	for _, name := range args {
-		pr := &v1alpha1.PackageRevision{}
-		if err := r.client.Get(r.ctx, client.ObjectKey{
+		if err := porch.UpdatePackageRevisionApproval(r.ctx, r.client, client.ObjectKey{
 			Namespace: namespace,
 			Name:      name,
-		}, pr); err != nil {
-			return errors.E(op, err)
-		}
-
-		switch pr.Spec.Lifecycle {
-		case v1alpha1.PackageRevisionLifecycleProposed, v1alpha1.PackageRevisionLifecycleDeletionProposed:
-			pr.Spec.Lifecycle = v1alpha1.PackageRevisionLifecyclePublished
-			if err := r.client.Update(r.ctx, pr); err != nil {
-				messages = append(messages, err.Error())
-				fmt.Fprintf(r.Command.ErrOrStderr(), "%s failed (%s)\n", name, err)
-			} else {
-				fmt.Fprintf(r.Command.OutOrStdout(), "%s approved\n", name)
-			}
-		default:
-			msg := fmt.Sprintf("cannot approve %s with lifecycle '%s'", name, pr.Spec.Lifecycle)
-			messages = append(messages, msg)
-			fmt.Fprintln(r.Command.ErrOrStderr(), msg)
+		}, v1alpha1.PackageRevisionLifecyclePublished); err != nil {
+			messages = append(messages, err.Error())
+			fmt.Fprintf(r.Command.ErrOrStderr(), "%s failed (%s)\n", name, err)
+		} else {
+			fmt.Fprintf(r.Command.OutOrStdout(), "%s approved\n", name)
 		}
 	}
 
diff --git a/pkg/cli/commands/rpkg/approve/command_test.go b/pkg/cli/commands/rpkg/approve/command_test.go
index 08a8d418..2f235598 100644
--- a/pkg/cli/commands/rpkg/approve/command_test.go
+++ b/pkg/cli/commands/rpkg/approve/command_test.go
@@ -58,19 +58,19 @@ func TestCmd(t *testing.T) {
 	}{
 		"Package not found in ns": {
 			wantErr: true,
+			output:  pkgRevName + " failed (packagerevisions.porch.kpt.dev \"" + pkgRevName + "\" not found)\n",
 			ns:      "dummy",
 		},
 		"Approve draft package": {
 			wantErr: true,
-			output:  "cannot approve " + pkgRevName + " with lifecycle 'Draft'\n",
+			output:  pkgRevName + " failed (cannot change approval from Draft to Published)\n",
 			ns:      ns,
 			lc:      porchapi.PackageRevisionLifecycleDraft,
 		},
 		"Approve published package": {
-			wantErr: true,
-			output:  "cannot approve " + pkgRevName + " with lifecycle 'Published'\n",
-			ns:      ns,
-			lc:      porchapi.PackageRevisionLifecyclePublished,
+			output: pkgRevName + " approved\n",
+			ns:     ns,
+			lc:     porchapi.PackageRevisionLifecyclePublished,
 		},
 		"Approve deletion-proposed package": {
 			output: pkgRevName + " approved\n",
diff --git a/pkg/cli/commands/rpkg/reject/command.go b/pkg/cli/commands/rpkg/reject/command.go
index 6d63b5f2..52f23eb0 100644
--- a/pkg/cli/commands/rpkg/reject/command.go
+++ b/pkg/cli/commands/rpkg/reject/command.go
@@ -96,8 +96,10 @@ func (r *runner) runE(_ *cobra.Command, args []string) error {
 		}
 		switch pr.Spec.Lifecycle {
 		case v1alpha1.PackageRevisionLifecycleProposed:
-			pr.Spec.Lifecycle = v1alpha1.PackageRevisionLifecycleDraft
-			if err := r.client.Update(r.ctx, pr); err != nil {
+			if err := porch.UpdatePackageRevisionApproval(r.ctx, r.client, client.ObjectKey{
+				Namespace: namespace,
+				Name:      name,
+			}, v1alpha1.PackageRevisionLifecycleDraft); err != nil {
 				messages = append(messages, err.Error())
 				fmt.Fprintf(r.Command.ErrOrStderr(), "%s failed (%s)\n", name, err)
 			} else {

From efff4cbc125f467b32431473c45a8fa94e3cedf9 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Wed, 26 Jun 2024 14:06:18 +0000
Subject: [PATCH 07/13] Revert "update reject and approve unit tests"

This reverts commit 7e95895fc60171797b9776cb8f4ec4cbc90f5eca.
---
 pkg/cli/commands/rpkg/approve/command_test.go | 2 +-
 pkg/cli/commands/rpkg/reject/command_test.go  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/cli/commands/rpkg/approve/command_test.go b/pkg/cli/commands/rpkg/approve/command_test.go
index 2f235598..c32703cf 100644
--- a/pkg/cli/commands/rpkg/approve/command_test.go
+++ b/pkg/cli/commands/rpkg/approve/command_test.go
@@ -122,7 +122,7 @@ func TestCmd(t *testing.T) {
 				defer write.Close()
 				err := r.runE(cmd, []string{pkgRevName})
 				if err != nil && !tc.wantErr {
-					t.Errorf("unexpected error: %v", err.Error())
+					t.Errorf("unexpected error: %v", err)
 				}
 			}()
 			out, _ := io.ReadAll(read)
diff --git a/pkg/cli/commands/rpkg/reject/command_test.go b/pkg/cli/commands/rpkg/reject/command_test.go
index cb6f63ad..eb2bcd7a 100644
--- a/pkg/cli/commands/rpkg/reject/command_test.go
+++ b/pkg/cli/commands/rpkg/reject/command_test.go
@@ -122,7 +122,7 @@ func TestCmd(t *testing.T) {
 				defer write.Close()
 				err := r.runE(cmd, []string{pkgRevName})
 				if err != nil && !tc.wantErr {
-					t.Errorf("unexpected error: %v", err.Error())
+					t.Errorf("unexpected error: %v", err)
 				}
 			}()
 			out, _ := io.ReadAll(read)

From af47e8ca32a1f7da1ec63c78c22f4c954724d488 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Wed, 26 Jun 2024 15:19:17 +0000
Subject: [PATCH 08/13] refactor reject and approve unit tests for
 subresourceupdate

---
 internal/kpt/util/porch/approval.go           |   6 +-
 pkg/cli/commands/rpkg/approve/command_test.go | 103 ++++++++++++-----
 pkg/cli/commands/rpkg/reject/command_test.go  | 106 +++++++++++++-----
 3 files changed, 159 insertions(+), 56 deletions(-)

diff --git a/internal/kpt/util/porch/approval.go b/internal/kpt/util/porch/approval.go
index 74d7a3b9..c0bdd680 100644
--- a/internal/kpt/util/porch/approval.go
+++ b/internal/kpt/util/porch/approval.go
@@ -4,14 +4,13 @@
 // 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
+//	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 porch
 
 import (
@@ -38,9 +37,8 @@ func UpdatePackageRevisionApproval(ctx context.Context, client client.Client, ke
 	default:
 		return fmt.Errorf("cannot change approval from %s to %s", lifecycle, new)
 	}
-
 	// Approve - change the package revision kind to "final".
 	pr.Spec.Lifecycle = new
+	return client.SubResource("approval").Update(ctx, &pr)
 
-	return client.Update(ctx, &pr)
 }
diff --git a/pkg/cli/commands/rpkg/approve/command_test.go b/pkg/cli/commands/rpkg/approve/command_test.go
index c32703cf..58706547 100644
--- a/pkg/cli/commands/rpkg/approve/command_test.go
+++ b/pkg/cli/commands/rpkg/approve/command_test.go
@@ -26,7 +26,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 )
 
 func createScheme() (*runtime.Scheme, error) {
@@ -51,57 +53,108 @@ func TestCmd(t *testing.T) {
 		t.Fatalf("error creating scheme: %v", err)
 	}
 	testCases := map[string]struct {
-		output  string
-		wantErr bool
-		lc      porchapi.PackageRevisionLifecycle
-		ns      string
+		output     string
+		wantErr    bool
+		fakeclient client.WithWatch
 	}{
 		"Package not found in ns": {
 			wantErr: true,
 			output:  pkgRevName + " failed (packagerevisions.porch.kpt.dev \"" + pkgRevName + "\" not found)\n",
-			ns:      "dummy",
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "dummy",
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Approve draft package": {
 			wantErr: true,
 			output:  pkgRevName + " failed (cannot change approval from Draft to Published)\n",
-			ns:      ns,
-			lc:      porchapi.PackageRevisionLifecycleDraft,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleDraft,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Approve published package": {
 			output: pkgRevName + " approved\n",
-			ns:     ns,
-			lc:     porchapi.PackageRevisionLifecyclePublished,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecyclePublished,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Approve deletion-proposed package": {
 			output: pkgRevName + " approved\n",
-			ns:     ns,
-			lc:     porchapi.PackageRevisionLifecycleDeletionProposed,
+			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
+				//fake subresourceupdate
+				SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error {
+					return nil
+				},
+			}).WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleDeletionProposed,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Approve proposed package": {
 			output: pkgRevName + " approved\n",
-			ns:     ns,
-			lc:     porchapi.PackageRevisionLifecycleProposed,
-		},
-	}
-
-	for tn := range testCases {
-		tc := testCases[tn]
-		t.Run(tn, func(t *testing.T) {
-
-			c := fake.NewClientBuilder().WithScheme(scheme).
+			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
+				//fake subresourceupdate
+				SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error {
+					return nil
+				},
+			}).WithScheme(scheme).
 				WithObjects(&porchapi.PackageRevision{
 					TypeMeta: metav1.TypeMeta{
 						Kind:       "PackageRevision",
 						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
 					},
 					Spec: porchapi.PackageRevisionSpec{
-						Lifecycle:      tc.lc,
+						Lifecycle:      porchapi.PackageRevisionLifecycleProposed,
 						RepositoryName: repoName,
 					},
 					ObjectMeta: metav1.ObjectMeta{
-						Namespace: tc.ns,
+						Namespace: ns,
 						Name:      pkgRevName,
-					}}).Build()
+					}}).Build(),
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
 
 			cmd := &cobra.Command{}
 			o := os.Stdout
@@ -115,7 +168,7 @@ func TestCmd(t *testing.T) {
 				cfg: &genericclioptions.ConfigFlags{
 					Namespace: &ns,
 				},
-				client:  c,
+				client:  tc.fakeclient,
 				Command: cmd,
 			}
 			go func() {
diff --git a/pkg/cli/commands/rpkg/reject/command_test.go b/pkg/cli/commands/rpkg/reject/command_test.go
index eb2bcd7a..2fe61950 100644
--- a/pkg/cli/commands/rpkg/reject/command_test.go
+++ b/pkg/cli/commands/rpkg/reject/command_test.go
@@ -26,7 +26,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 )
 
 func createScheme() (*runtime.Scheme, error) {
@@ -51,57 +53,107 @@ func TestCmd(t *testing.T) {
 		t.Fatalf("error creating scheme: %v", err)
 	}
 	testCases := map[string]struct {
-		output  string
-		wantErr bool
-		lc      porchapi.PackageRevisionLifecycle
-		ns      string
+		output     string
+		wantErr    bool
+		fakeclient client.WithWatch
 	}{
 		"Package not found in ns": {
 			wantErr: true,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleProposed,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "dummy",
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Reject deletion-proposed package": {
 			output: pkgRevName + " no longer proposed for deletion\n",
-			ns:     ns,
-			lc:     porchapi.PackageRevisionLifecycleDeletionProposed,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleDeletionProposed,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Reject draft package": {
 			wantErr: true,
 			output:  "cannot reject " + pkgRevName + " with lifecycle 'Draft'\n",
-			ns:      ns,
-			lc:      porchapi.PackageRevisionLifecycleDraft,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleDraft,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Reject published package": {
 			wantErr: true,
 			output:  "cannot reject " + pkgRevName + " with lifecycle 'Published'\n",
-			ns:      ns,
-			lc:      porchapi.PackageRevisionLifecyclePublished,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecyclePublished,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
 		},
 		"Reject proposed package": {
-			wantErr: false,
-			output:  pkgRevName + " rejected\n",
-			ns:      ns,
-			lc:      porchapi.PackageRevisionLifecycleProposed,
-		},
-	}
-
-	for tn := range testCases {
-		tc := testCases[tn]
-		t.Run(tn, func(t *testing.T) {
-
-			c := fake.NewClientBuilder().WithScheme(scheme).
-				WithObjects(&porchapi.PackageRevision{
+			output: pkgRevName + " rejected\n",
+			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
+				//fake subresourceupdate
+				SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error {
+					return nil
+				},
+			}).WithScheme(scheme).
+				WithRuntimeObjects(&porchapi.PackageRevision{
 					TypeMeta: metav1.TypeMeta{
 						Kind:       "PackageRevision",
 						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
 					},
 					Spec: porchapi.PackageRevisionSpec{
-						Lifecycle:      tc.lc,
+						Lifecycle:      porchapi.PackageRevisionLifecycleProposed,
 						RepositoryName: repoName,
 					},
 					ObjectMeta: metav1.ObjectMeta{
-						Namespace: tc.ns,
+						Namespace: ns,
 						Name:      pkgRevName,
-					}}).Build()
+					}}).Build(),
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
 
 			cmd := &cobra.Command{}
 			o := os.Stdout
@@ -115,7 +167,7 @@ func TestCmd(t *testing.T) {
 				cfg: &genericclioptions.ConfigFlags{
 					Namespace: &ns,
 				},
-				client:  c,
+				client:  tc.fakeclient,
 				Command: cmd,
 			}
 			go func() {

From 991011270220cb55d528a38ce35ff86e8586729e Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Thu, 27 Jun 2024 14:04:31 +0000
Subject: [PATCH 09/13] add units tests for clone and get rpkg commands

---
 pkg/cli/commands/rpkg/clone/command_test.go | 114 ++++++++++++++++++++
 pkg/cli/commands/rpkg/get/command_test.go   |  78 ++++++++++++++
 pkg/cli/commands/rpkg/init/command_test.go  |   4 +-
 3 files changed, 194 insertions(+), 2 deletions(-)
 create mode 100644 pkg/cli/commands/rpkg/clone/command_test.go
 create mode 100644 pkg/cli/commands/rpkg/get/command_test.go

diff --git a/pkg/cli/commands/rpkg/clone/command_test.go b/pkg/cli/commands/rpkg/clone/command_test.go
new file mode 100644
index 00000000..09dbd7d4
--- /dev/null
+++ b/pkg/cli/commands/rpkg/clone/command_test.go
@@ -0,0 +1,114 @@
+// Copyright 2022 The kpt and Nephio 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 clone
+
+import (
+	"context"
+	"io"
+	"os"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
+	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+)
+
+func createScheme() (*runtime.Scheme, error) {
+	scheme := runtime.NewScheme()
+	for _, api := range (runtime.SchemeBuilder{
+		porchapi.AddToScheme,
+	}) {
+		if err := api(scheme); err != nil {
+			return nil, err
+		}
+	}
+	scheme.AddKnownTypes(porchapi.SchemeGroupVersion, &porchapi.PackageRevision{})
+	return scheme, nil
+}
+
+func TestCmd(t *testing.T) {
+	repoName := "test-repo"
+	ns := "ns"
+	var scheme, err = createScheme()
+	if err != nil {
+		t.Fatalf("error creating scheme: %v", err)
+	}
+	testCases := map[string]struct {
+		output     string
+		wantErr    bool
+		ns         string
+		fakeclient client.WithWatch
+	}{
+		"metadata.name required": {
+			wantErr:    true,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).Build(),
+		},
+		"clone package": {
+			wantErr: false,
+			ns:      ns,
+			output:  "pr-clone created\n",
+			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
+				Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+					if obj.GetObjectKind().GroupVersionKind().Kind == "PackageRevision" {
+						obj.SetName("pr-clone")
+					}
+					return nil
+				},
+			}).WithScheme(scheme).Build(),
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
+
+			cmd := &cobra.Command{}
+			o := os.Stdout
+			e := os.Stderr
+			read, write, _ := os.Pipe()
+			os.Stdout = write
+			os.Stderr = write
+
+			r := &runner{
+				ctx: context.Background(),
+				cfg: &genericclioptions.ConfigFlags{
+					Namespace: &tc.ns,
+				},
+				client:     tc.fakeclient,
+				Command:    cmd,
+				repository: repoName,
+			}
+			go func() {
+				defer write.Close()
+				err := r.runE(cmd, []string{})
+				if err != nil && !tc.wantErr {
+					t.Errorf("unexpected error: %v", err.Error())
+				}
+			}()
+			out, _ := io.ReadAll(read)
+			os.Stdout = o
+			os.Stderr = e
+
+			if diff := cmp.Diff(string(tc.output), string(out)); diff != "" {
+				t.Errorf("Unexpected result (-want, +got): %s", diff)
+			}
+		})
+	}
+}
diff --git a/pkg/cli/commands/rpkg/get/command_test.go b/pkg/cli/commands/rpkg/get/command_test.go
new file mode 100644
index 00000000..06b77dce
--- /dev/null
+++ b/pkg/cli/commands/rpkg/get/command_test.go
@@ -0,0 +1,78 @@
+// Copyright 2022 The kpt and Nephio 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 get
+
+import (
+	"context"
+	"io"
+	"os"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/nephio-project/porch/internal/kpt/options"
+	"github.com/spf13/cobra"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+)
+
+func TestCmd(t *testing.T) {
+	pkgRevName := "test-pr"
+	ns := "ns"
+
+	testCases := map[string]struct {
+		output  string
+		wantErr bool
+	}{
+		"ns not found": {
+			wantErr: true,
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
+
+			cmd := &cobra.Command{}
+			o := os.Stdout
+			e := os.Stderr
+			read, write, _ := os.Pipe()
+			os.Stdout = write
+			os.Stderr = write
+
+			rcg := &genericclioptions.ConfigFlags{
+				Namespace: &ns,
+			}
+
+			r := &runner{
+				ctx:      context.Background(),
+				Command:  cmd,
+				getFlags: options.Get{ConfigFlags: rcg},
+			}
+			go func() {
+				defer write.Close()
+				err := r.runE(cmd, []string{pkgRevName})
+				if err != nil && !tc.wantErr {
+					t.Errorf("unexpected error: %v", err.Error())
+				}
+			}()
+			out, _ := io.ReadAll(read)
+			os.Stdout = o
+			os.Stderr = e
+
+			if diff := cmp.Diff(string(tc.output), string(out)); diff != "" {
+				t.Errorf("Unexpected result (-want, +got): %s", diff)
+			}
+		})
+	}
+}
diff --git a/pkg/cli/commands/rpkg/init/command_test.go b/pkg/cli/commands/rpkg/init/command_test.go
index f1a4e548..35c07e00 100644
--- a/pkg/cli/commands/rpkg/init/command_test.go
+++ b/pkg/cli/commands/rpkg/init/command_test.go
@@ -54,10 +54,10 @@ func TestCmd(t *testing.T) {
 		ns         string
 		fakeclient client.WithWatch
 	}{
-		"Repo not in ns": {
+		"metadata.name required": {
 			ns:         "doesnotexist",
 			wantErr:    true,
-			fakeclient: fake.NewClientBuilder().Build(),
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).Build(),
 		},
 		"successful init": {
 			ns:     "ns",

From fdeaa1d9876c3f7269a7b675ada288fb155d1cc5 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Mon, 1 Jul 2024 11:27:06 +0000
Subject: [PATCH 10/13] add unit test for copy command

Signed-off-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
---
 pkg/cli/commands/rpkg/copy/command_test.go | 134 +++++++++++++++++++++
 1 file changed, 134 insertions(+)
 create mode 100644 pkg/cli/commands/rpkg/copy/command_test.go

diff --git a/pkg/cli/commands/rpkg/copy/command_test.go b/pkg/cli/commands/rpkg/copy/command_test.go
new file mode 100644
index 00000000..d781b05b
--- /dev/null
+++ b/pkg/cli/commands/rpkg/copy/command_test.go
@@ -0,0 +1,134 @@
+// Copyright 2022 The kpt and Nephio 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 copy
+
+import (
+	"context"
+	"io"
+	"os"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
+	"github.com/spf13/cobra"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+)
+
+func createScheme() (*runtime.Scheme, error) {
+	scheme := runtime.NewScheme()
+	for _, api := range (runtime.SchemeBuilder{
+		porchapi.AddToScheme,
+	}) {
+		if err := api(scheme); err != nil {
+			return nil, err
+		}
+	}
+	scheme.AddKnownTypes(porchapi.SchemeGroupVersion, &porchapi.PackageRevision{})
+	return scheme, nil
+}
+
+func TestCmd(t *testing.T) {
+	repoName := "test-repo"
+	ns := "ns"
+	pkgRevName := "test-package"
+	var scheme, err = createScheme()
+	if err != nil {
+		t.Fatalf("error creating scheme: %v", err)
+	}
+	testCases := map[string]struct {
+		output     string
+		wantErr    bool
+		ns         string
+		fakeclient client.WithWatch
+	}{
+		"metadata.name required": {
+			wantErr:    true,
+			fakeclient: fake.NewClientBuilder().WithScheme(scheme).Build(),
+		},
+		"copy package": {
+			wantErr: false,
+			ns:      ns,
+			output:  pkgRevName + " created\n",
+			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
+				Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+					if obj.GetObjectKind().GroupVersionKind().Kind == "PackageRevision" {
+						obj.SetName(pkgRevName)
+					}
+					return nil
+				},
+			}).WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleProposed,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
+		},
+	}
+
+	for tn := range testCases {
+		tc := testCases[tn]
+		t.Run(tn, func(t *testing.T) {
+
+			cmd := &cobra.Command{}
+			o := os.Stdout
+			e := os.Stderr
+			read, write, _ := os.Pipe()
+			os.Stdout = write
+			os.Stderr = write
+
+			r := &runner{
+				ctx: context.Background(),
+				cfg: &genericclioptions.ConfigFlags{
+					Namespace: &tc.ns,
+				},
+				copy: porchapi.PackageEditTaskSpec{
+					Source: &porchapi.PackageRevisionRef{
+						Name: pkgRevName,
+					},
+				},
+				client:    tc.fakeclient,
+				workspace: "v2",
+				Command:   cmd,
+			}
+			go func() {
+				defer write.Close()
+				err := r.runE(cmd, []string{})
+				if err != nil && !tc.wantErr {
+					t.Errorf("unexpected error: %v", err.Error())
+				}
+			}()
+			out, _ := io.ReadAll(read)
+			os.Stdout = o
+			os.Stderr = e
+
+			if diff := cmp.Diff(string(tc.output), string(out)); diff != "" {
+				t.Errorf("Unexpected result (-want, +got): %s", diff)
+			}
+		})
+	}
+}

From d9b21fbb68d3a843ce54494809830a9f5dd5c7b1 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Mon, 1 Jul 2024 11:52:26 +0000
Subject: [PATCH 11/13] improve copy command unit test

---
 pkg/cli/commands/rpkg/copy/command_test.go | 37 +++++++++++++++++++---
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/pkg/cli/commands/rpkg/copy/command_test.go b/pkg/cli/commands/rpkg/copy/command_test.go
index d781b05b..4495fb9c 100644
--- a/pkg/cli/commands/rpkg/copy/command_test.go
+++ b/pkg/cli/commands/rpkg/copy/command_test.go
@@ -56,16 +56,45 @@ func TestCmd(t *testing.T) {
 		output     string
 		wantErr    bool
 		ns         string
+		workspace  string
 		fakeclient client.WithWatch
 	}{
 		"metadata.name required": {
 			wantErr:    true,
 			fakeclient: fake.NewClientBuilder().WithScheme(scheme).Build(),
 		},
+		"Workspace argument required": {
+			wantErr:   true,
+			ns:        ns,
+			workspace: "",
+			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
+				Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+					if obj.GetObjectKind().GroupVersionKind().Kind == "PackageRevision" {
+						obj.SetName(pkgRevName)
+					}
+					return nil
+				},
+			}).WithScheme(scheme).
+				WithObjects(&porchapi.PackageRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "PackageRevision",
+						APIVersion: porchapi.SchemeGroupVersion.Identifier(),
+					},
+					Spec: porchapi.PackageRevisionSpec{
+						Lifecycle:      porchapi.PackageRevisionLifecycleProposed,
+						RepositoryName: repoName,
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+						Name:      pkgRevName,
+					}}).Build(),
+		},
+
 		"copy package": {
-			wantErr: false,
-			ns:      ns,
-			output:  pkgRevName + " created\n",
+			wantErr:   false,
+			ns:        ns,
+			output:    pkgRevName + " created\n",
+			workspace: "v2",
 			fakeclient: fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
 				Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
 					if obj.GetObjectKind().GroupVersionKind().Kind == "PackageRevision" {
@@ -112,7 +141,7 @@ func TestCmd(t *testing.T) {
 					},
 				},
 				client:    tc.fakeclient,
-				workspace: "v2",
+				workspace: tc.workspace,
 				Command:   cmd,
 			}
 			go func() {

From 085f4e04416c4da8b1a088e60383b96a626574a4 Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Mon, 1 Jul 2024 12:32:03 +0000
Subject: [PATCH 12/13] remove get command unit test

---
 pkg/cli/commands/rpkg/get/command_test.go | 78 -----------------------
 1 file changed, 78 deletions(-)
 delete mode 100644 pkg/cli/commands/rpkg/get/command_test.go

diff --git a/pkg/cli/commands/rpkg/get/command_test.go b/pkg/cli/commands/rpkg/get/command_test.go
deleted file mode 100644
index 06b77dce..00000000
--- a/pkg/cli/commands/rpkg/get/command_test.go
+++ /dev/null
@@ -1,78 +0,0 @@
-// Copyright 2022 The kpt and Nephio 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 get
-
-import (
-	"context"
-	"io"
-	"os"
-	"testing"
-
-	"github.com/google/go-cmp/cmp"
-	"github.com/nephio-project/porch/internal/kpt/options"
-	"github.com/spf13/cobra"
-	"k8s.io/cli-runtime/pkg/genericclioptions"
-)
-
-func TestCmd(t *testing.T) {
-	pkgRevName := "test-pr"
-	ns := "ns"
-
-	testCases := map[string]struct {
-		output  string
-		wantErr bool
-	}{
-		"ns not found": {
-			wantErr: true,
-		},
-	}
-
-	for tn := range testCases {
-		tc := testCases[tn]
-		t.Run(tn, func(t *testing.T) {
-
-			cmd := &cobra.Command{}
-			o := os.Stdout
-			e := os.Stderr
-			read, write, _ := os.Pipe()
-			os.Stdout = write
-			os.Stderr = write
-
-			rcg := &genericclioptions.ConfigFlags{
-				Namespace: &ns,
-			}
-
-			r := &runner{
-				ctx:      context.Background(),
-				Command:  cmd,
-				getFlags: options.Get{ConfigFlags: rcg},
-			}
-			go func() {
-				defer write.Close()
-				err := r.runE(cmd, []string{pkgRevName})
-				if err != nil && !tc.wantErr {
-					t.Errorf("unexpected error: %v", err.Error())
-				}
-			}()
-			out, _ := io.ReadAll(read)
-			os.Stdout = o
-			os.Stderr = e
-
-			if diff := cmp.Diff(string(tc.output), string(out)); diff != "" {
-				t.Errorf("Unexpected result (-want, +got): %s", diff)
-			}
-		})
-	}
-}

From 57eb7068ee1bd11712ddf80c765a1bf7921e6a9f Mon Sep 17 00:00:00 2001
From: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Date: Mon, 22 Jul 2024 11:41:29 +0000
Subject: [PATCH 13/13] Update copyright year

---
 pkg/cli/commands/rpkg/approve/command_test.go       | 2 +-
 pkg/cli/commands/rpkg/clone/command_test.go         | 2 +-
 pkg/cli/commands/rpkg/copy/command_test.go          | 2 +-
 pkg/cli/commands/rpkg/del/command_test.go           | 2 +-
 pkg/cli/commands/rpkg/init/command_test.go          | 2 +-
 pkg/cli/commands/rpkg/propose/command_test.go       | 2 +-
 pkg/cli/commands/rpkg/proposedelete/command_test.go | 2 +-
 pkg/cli/commands/rpkg/push/command_test.go          | 2 +-
 pkg/cli/commands/rpkg/reject/command_test.go        | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/pkg/cli/commands/rpkg/approve/command_test.go b/pkg/cli/commands/rpkg/approve/command_test.go
index 58706547..30dfb957 100644
--- a/pkg/cli/commands/rpkg/approve/command_test.go
+++ b/pkg/cli/commands/rpkg/approve/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/clone/command_test.go b/pkg/cli/commands/rpkg/clone/command_test.go
index 09dbd7d4..6ee0f54e 100644
--- a/pkg/cli/commands/rpkg/clone/command_test.go
+++ b/pkg/cli/commands/rpkg/clone/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/copy/command_test.go b/pkg/cli/commands/rpkg/copy/command_test.go
index 4495fb9c..ce216128 100644
--- a/pkg/cli/commands/rpkg/copy/command_test.go
+++ b/pkg/cli/commands/rpkg/copy/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/del/command_test.go b/pkg/cli/commands/rpkg/del/command_test.go
index e2680846..63998f91 100644
--- a/pkg/cli/commands/rpkg/del/command_test.go
+++ b/pkg/cli/commands/rpkg/del/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/init/command_test.go b/pkg/cli/commands/rpkg/init/command_test.go
index 35c07e00..e9ef5c82 100644
--- a/pkg/cli/commands/rpkg/init/command_test.go
+++ b/pkg/cli/commands/rpkg/init/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/propose/command_test.go b/pkg/cli/commands/rpkg/propose/command_test.go
index cab886c0..fce021f2 100644
--- a/pkg/cli/commands/rpkg/propose/command_test.go
+++ b/pkg/cli/commands/rpkg/propose/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/proposedelete/command_test.go b/pkg/cli/commands/rpkg/proposedelete/command_test.go
index 5cfce9e6..7ad83ba2 100644
--- a/pkg/cli/commands/rpkg/proposedelete/command_test.go
+++ b/pkg/cli/commands/rpkg/proposedelete/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/push/command_test.go b/pkg/cli/commands/rpkg/push/command_test.go
index bc6e3283..eb1a5e7c 100644
--- a/pkg/cli/commands/rpkg/push/command_test.go
+++ b/pkg/cli/commands/rpkg/push/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/pkg/cli/commands/rpkg/reject/command_test.go b/pkg/cli/commands/rpkg/reject/command_test.go
index 2fe61950..94ca4730 100644
--- a/pkg/cli/commands/rpkg/reject/command_test.go
+++ b/pkg/cli/commands/rpkg/reject/command_test.go
@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.