From c5fe173afd31636bf014dac31f6e601d91e1ae53 Mon Sep 17 00:00:00 2001
From: DCCooper <1866858@gmail.com>
Date: Sat, 30 Oct 2021 10:12:40 +0800
Subject: [PATCH 12/16] bugfix: fix when load separated image error return

reason: if base and lib dir are both not provided, daemon
side will read the info from "manifest" file in the dest dir
automatically, so no error return here

Signed-off-by: DCCooper <1866858@gmail.com>
---
 cmd/cli/load.go      |  4 +--
 cmd/cli/load_test.go | 59 +++++++++++++++++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/cmd/cli/load.go b/cmd/cli/load.go
index cf142592..44fefdd2 100644
--- a/cmd/cli/load.go
+++ b/cmd/cli/load.go
@@ -178,7 +178,7 @@ func (sep *separatorLoadOption) check(pwd string) error {
 		return errors.New("image tarball directory should not be empty")
 	}
 
-	if sep.base == sep.lib {
+	if len(sep.base) != 0 && sep.base == sep.lib {
 		return errors.New("base and lib tarballs are the same")
 	}
 
@@ -203,7 +203,7 @@ func (sep *separatorLoadOption) check(pwd string) error {
 
 	sep.dir = util.MakeAbsolute(sep.dir, pwd)
 	if !util.IsExist(sep.dir) {
-		return errors.Errorf("image tarball directory %s is not exist", sep.dir)
+		return errors.Errorf("image tarball directory %q is not exist", sep.dir)
 	}
 
 	return nil
diff --git a/cmd/cli/load_test.go b/cmd/cli/load_test.go
index 0bad4cbd..cb8217ce 100644
--- a/cmd/cli/load_test.go
+++ b/cmd/cli/load_test.go
@@ -15,6 +15,7 @@ package main
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -22,7 +23,9 @@ import (
 
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/fs"
+
 	constant "isula.org/isula-build"
+	"isula.org/isula-build/util"
 )
 
 func TestLoadCmd(t *testing.T) {
@@ -182,6 +185,8 @@ func TestResolveLoadPath(t *testing.T) {
 }
 
 func TestCheckLoadOpts(t *testing.T) {
+	pwd, err := os.Getwd()
+	assert.NilError(t, err)
 	root := fs.NewDir(t, t.Name())
 	defer root.Remove()
 	emptyFile, err := os.Create(filepath.Join(root.Path(), "empty.tar"))
@@ -201,9 +206,10 @@ func TestCheckLoadOpts(t *testing.T) {
 		sep    separatorLoadOption
 	}
 	tests := []struct {
-		name    string
-		fields  fields
-		wantErr bool
+		name       string
+		fields     fields
+		wantErr    bool
+		errMessage string
 	}{
 		{
 			name: "TC-normal load options",
@@ -212,15 +218,17 @@ func TestCheckLoadOpts(t *testing.T) {
 			},
 		},
 		{
-			name:    "TC-empty load path",
-			wantErr: true,
+			name:       "TC-empty load path",
+			wantErr:    true,
+			errMessage: "tarball path should not be empty",
 		},
 		{
 			name: "TC-empty load file",
 			fields: fields{
 				path: emptyFile.Name(),
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: "loading file is empty",
 		},
 		{
 			name: "TC-separated load",
@@ -243,7 +251,8 @@ func TestCheckLoadOpts(t *testing.T) {
 					lib:  libFile.Name(),
 				},
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: "app image name(-i) should not be empty",
 		},
 		{
 			name: "TC-separated load with empty dir",
@@ -254,7 +263,8 @@ func TestCheckLoadOpts(t *testing.T) {
 					lib:  libFile.Name(),
 				},
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: "image tarball directory should not be empty",
 		},
 		{
 			name: "TC-separated load with invalid app name",
@@ -266,7 +276,8 @@ func TestCheckLoadOpts(t *testing.T) {
 					lib:  libFile.Name(),
 				},
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: fmt.Sprintf("invalid image name: %s", "invalid:app:name"),
 		},
 		{
 			name: "TC-separated load with empty base tarball",
@@ -278,7 +289,8 @@ func TestCheckLoadOpts(t *testing.T) {
 					lib:  libFile.Name(),
 				},
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: "resolve base tarball path failed: loading file is empty",
 		},
 		{
 			name: "TC-separated load with empty lib tarball",
@@ -290,7 +302,8 @@ func TestCheckLoadOpts(t *testing.T) {
 					lib:  emptyFile.Name(),
 				},
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: "resolve lib tarball path failed: loading file is empty",
 		},
 		{
 			name: "TC-separated load with same base and lib tarball",
@@ -302,7 +315,8 @@ func TestCheckLoadOpts(t *testing.T) {
 					lib:  fileWithContent.Name(),
 				},
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: "base and lib tarballs are the same",
 		},
 		{
 			name: "TC-separated load with dir not exist",
@@ -314,7 +328,20 @@ func TestCheckLoadOpts(t *testing.T) {
 					lib:  libFile.Name(),
 				},
 			},
-			wantErr: true,
+			wantErr:    true,
+			errMessage: fmt.Sprintf("image tarball directory %q is not exist", util.MakeAbsolute("path not exist", pwd)),
+		},
+		{
+			// if base and lib dir are both not provided, daemon side will read
+			// the info from "manifest" file in the dest dir automatically
+			// so no error return here
+			name: "TC-base and lib dir both not provided",
+			fields: fields{
+				path: "app:latest",
+				sep: separatorLoadOption{
+					dir: root.Path(),
+				},
+			},
 		},
 	}
 	for _, tt := range tests {
@@ -324,9 +351,13 @@ func TestCheckLoadOpts(t *testing.T) {
 				loadID: tt.fields.loadID,
 				sep:    tt.fields.sep,
 			}
-			if err := opt.checkLoadOpts(); (err != nil) != tt.wantErr {
+			err := opt.checkLoadOpts()
+			if (err != nil) != tt.wantErr {
 				t.Errorf("loadOptions.checkLoadOpts() error = %v, wantErr %v", err, tt.wantErr)
 			}
+			if err != nil && err.Error() != tt.errMessage {
+				t.Errorf("loadOptions.checkLoadOpts() error = %v, wantErr %v", err, tt.errMessage)
+			}
 		})
 	}
 }
-- 
2.27.0