add mock, unit test and coverage.

Makefile

@@ -70,7 +70,7 @@ $(GO_DEPS): go.mod $(PATCHES) swsscommon_wrap $(GNOI_YANG)
 	$(GO) mod download github.com/google/[email protected]
 	cp -r $(GOPATH)/pkg/mod/github.com/google/[email protected]/* vendor/github.com/google/gnxi/
 
-# Apply patch from sonic-mgmt-common, ignore glog.patch because glog version changed 
+# Apply patch from sonic-mgmt-common, ignore glog.patch because glog version changed
 	sed -i 's/patch -d $${DEST_DIR}\/github.com\/golang\/glog/\#patch -d $${DEST_DIR}\/github.com\/golang\/glog/g' $(MGMT_COMMON_DIR)/patches/apply.sh
 	$(MGMT_COMMON_DIR)/patches/apply.sh vendor
 	sed -i 's/#patch -d $${DEST_DIR}\/github.com\/golang\/glog/patch -d $${DEST_DIR}\/github.com\/golang\/glog/g' $(MGMT_COMMON_DIR)/patches/apply.sh
@@ -106,7 +106,7 @@ endif
 	$(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/gnmi_dump
 	$(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/build/gnoi_yang/client/gnoi_openconfig_client
 	$(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/build/gnoi_yang/client/gnoi_sonic_client
-	
+
 endif
 
 # download and apply patch for gnmi client, which will break advancetls
@@ -216,11 +216,13 @@ endif
 	sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-data.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_data_client
 	sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-dbus.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_service_client
 	sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -coverprofile=coverage-translutils.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/transl_utils
+	sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -coverprofile=coverage-gnoi-client-system.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/gnoi_client/system
 	$(GO) install github.com/axw/gocov/[email protected]
 	$(GO) install github.com/AlekSi/gocov-xml@latest
 	$(GO) mod vendor
 	gocov convert coverage-*.txt | gocov-xml -source $(shell pwd) > coverage.xml
-	rm -rf coverage-*.txt 
+	rm -rf coverage-*.txt
+
 
 check_memleak: $(DBCONFG) $(ENVFILE)
 	sudo CGO_LDFLAGS="$(MEMCHECK_CGO_LDFLAGS)" CGO_CXXFLAGS="$(MEMCHECK_CGO_CXXFLAGS)" $(GO) test -coverprofile=coverage-telemetry.txt -covermode=atomic -mod=vendor $(MEMCHECK_FLAGS) -v github.com/sonic-net/sonic-gnmi/telemetry

gnoi_client/system/set_package.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
+
 	"github.com/openconfig/gnoi/common"
 	"github.com/openconfig/gnoi/system"
 	"github.com/sonic-net/sonic-gnmi/gnoi_client/utils"
@@ -18,71 +19,102 @@ var (
 	activate = flag.Bool("package_activate", true, "Whether to activate the package after setting it")
 )
 
+// newSystemClient is a package-level variable that returns a new system.SystemClient.
+// We define it here so that unit tests can replace it with a mock constructor if needed.
+var newSystemClient = func(conn *grpc.ClientConn) system.SystemClient {
+	return system.NewSystemClient(conn)
+}
+
+// SetPackage is the main entry point. It validates flags, creates the SystemClient,
+// and then calls setPackageClient to perform the actual SetPackage gRPC flow.
 func SetPackage(conn *grpc.ClientConn, ctx context.Context) {
 	fmt.Println("System SetPackage")
+
+	// Attach user credentials if needed.
 	ctx = utils.SetUserCreds(ctx)
 
+	// Validate the flags before proceeding.
 	err := validateFlags()
 	if err != nil {
-		fmt.Println("Error validating flags: ", err)
+		fmt.Println("Error validating flags:", err)
 		return
 	}
 
+	// Create a new gNOI SystemClient using the function defined above.
+	sc := newSystemClient(conn)
+
+	// Call the helper that implements the SetPackage logic (sending requests, closing, etc.).
+	if err := setPackageClient(sc, ctx); err != nil {
+		fmt.Println("Error during SetPackage:", err)
+	}
+}
+
+// setPackageClient contains the core gRPC calls to send the package request and
+// receive the response. We separate it out for easier testing and mocking.
+func setPackageClient(sc system.SystemClient, ctx context.Context) error {
+	// Prepare the remote download info.
 	download := &common.RemoteDownload{
 		Path: *url,
 	}
+
+	// Build the package with flags.
 	pkg := &system.Package{
 		Filename:       *filename,
 		Version:        *version,
 		Activate:       *activate,
 		RemoteDownload: download,
 	}
+
+	// The gNOI SetPackageRequest can contain different request types, but we only
+	// use the "Package" request type here.
 	req := &system.SetPackageRequest{
 		Request: &system.SetPackageRequest_Package{
 			Package: pkg,
 		},
 	}
 
-	sc := system.NewSystemClient(conn)
+	// Open a streaming RPC.
 	stream, err := sc.SetPackage(ctx)
 	if err != nil {
-		fmt.Println("Error creating stream: ", err)
-		return
+		return fmt.Errorf("error creating stream: %v", err)
 	}
 
 	// Send the package information.
-	err = stream.Send(req)
-	// Device should download the package, so skip the direct transfer and checksum.
+	if err := stream.Send(req); err != nil {
+		return fmt.Errorf("error sending package request: %v", err)
+	}
 
-	err = stream.CloseSend()
-	if err != nil {
-		fmt.Println("Error closing stream: ", err)
-		return
+	// Close the send direction of the stream. The device should download the
+	// package itself, so we are not sending direct data or checksums here.
+	if err := stream.CloseSend(); err != nil {
+		return fmt.Errorf("error closing send direction: %v", err)
 	}
 
-	// Receive the response.
+	// Receive the final response from the device.
 	resp, err := stream.CloseAndRecv()
 	if err != nil {
-		fmt.Println("Error receiving response: ", err)
-		return
+		return fmt.Errorf("error receiving response: %v", err)
 	}
 
+	// For demonstration purposes, we simply print the response.
 	fmt.Println(resp)
+	return nil
 }
 
+// validateFlags ensures all required flags are set correctly before we proceed.
 func validateFlags() error {
 	if *filename == "" {
-		return fmt.Errorf("missing -package_filename: Destination path and filename of the package is required for the SetPackage operation.")
+		return fmt.Errorf("missing -package_filename: Destination path and filename of the package is required for the SetPackage operation")
 	}
 	if *version == "" {
-		return fmt.Errorf("missing -package_version: Version of the package is required for the SetPackage operation.")
+		return fmt.Errorf("missing -package_version: Version of the package is required for the SetPackage operation")
 	}
 	if *url == "" {
-		return fmt.Errorf("missing -package_url: URL to download the package from is required for the SetPackage operation. Direct transfer is not supported yet.")
+		return fmt.Errorf("missing -package_url: URL to download the package from is required for the SetPackage operation. Direct transfer is not supported yet")
 	}
 	if !*activate {
-		// TODO: Currently, installing image will always set it to default.
-		return fmt.Errorf("-package_activate=false is not yet supported: The package will always be activated after setting it.")
+		// TODO: Currently, installing the image will always set it to default.
+		return fmt.Errorf("-package_activate=false is not yet supported: The package will always be activated after setting it")
 	}
 	return nil
 }