more tests to enhance coverage

cmd/commands/add.go

@@ -4,10 +4,7 @@
 package commands
 
 import (
-	"bufio"
-	"os"
 	"path/filepath"
-	"strings"
 
 	errs "github.com/open-cmsis-pack/cpackget/cmd/errors"
 	"github.com/open-cmsis-pack/cpackget/cmd/installer"
@@ -81,28 +78,11 @@ Add a pack using the following "<pack>" specification or using packs provided by
 			return err
 		}
 
-		if addCmdFlags.packsListFileName != "" {
-			log.Infof("Parsing packs urls via file %v", addCmdFlags.packsListFileName)
-
-			file, err := os.Open(addCmdFlags.packsListFileName)
-			if err != nil {
-				return err
-			}
-			defer file.Close()
-
-			scanner := bufio.NewScanner(file)
-			for scanner.Scan() {
-				tmpEntry := strings.TrimSpace(scanner.Text())
-				if len(tmpEntry) == 0 {
-					continue
-				}
-				args = append(args, tmpEntry)
-			}
-
-			if err := scanner.Err(); err != nil {
-				return err
-			}
+		files, err := utils.GetListFiles(addCmdFlags.packsListFileName)
+		if err != nil {
+			return err
 		}
+		args = append(args, files...)
 
 		if len(args) == 0 {
 			log.Warn("Missing a pack-path or list with pack urls specified via -f/--packs-list-filename")

cmd/commands/add_test.go

@@ -109,6 +109,13 @@ var addCmdTests = []TestCase{
 			os.Remove(fileWithNoPacksListed)
 		},
 	},
+	{
+		name:           "test adding packs listed in missing file",
+		args:           []string{"add", "-f", fileWithPacksListed},
+		createPackRoot: true,
+		expectedErr:    errs.ErrFileNotFound,
+		expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed},
+	},
 }
 
 func TestAddCmd(t *testing.T) {

cmd/commands/root_test.go

@@ -138,6 +138,12 @@ func runTests(t *testing.T, tests []TestCase) {
 		test.assert = assert
 		t.Run(test.name, func(t *testing.T) {
 			localTestingDir := strings.ReplaceAll(test.name, " ", "_")
+
+			// Save the original environment variables to restore them after the test
+			originalEnv := make(map[string]string)
+			originalEnv["CPACKGET_DEFAULT_MODE_PATH"] = os.Getenv("CPACKGET_DEFAULT_MODE_PATH")
+			originalEnv["CMSIS_PACK_ROOT"] = os.Getenv("CMSIS_PACK_ROOT")
+
 			if test.defaultMode {
 				os.Setenv("CPACKGET_DEFAULT_MODE_PATH", localTestingDir)
 			}
@@ -194,6 +200,12 @@ func runTests(t *testing.T, tests []TestCase) {
 				})
 			}
 
+			// Reset the environment variables to their original values
+			// after the command execution
+			for key, value := range originalEnv {
+				os.Setenv(key, value)
+			}
+
 			outBytes, err1 := io.ReadAll(stdout)
 			errBytes, err2 := io.ReadAll(stderr)
 			assert.Nil(err1)

cmd/commands/update.go

@@ -4,10 +4,6 @@
 package commands
 
 import (
-	"bufio"
-	"os"
-	"strings"
-
 	errs "github.com/open-cmsis-pack/cpackget/cmd/errors"
 	"github.com/open-cmsis-pack/cpackget/cmd/installer"
 	"github.com/open-cmsis-pack/cpackget/cmd/utils"
@@ -61,28 +57,11 @@ Update a pack using the following "<pack>" specification or using packs provided
 			return err
 		}
 
-		if updateCmdFlags.packsListFileName != "" {
-			log.Infof("Parsing packs urls via file %v", updateCmdFlags.packsListFileName)
-
-			file, err := os.Open(updateCmdFlags.packsListFileName)
-			if err != nil {
-				return err
-			}
-			defer file.Close()
-
-			scanner := bufio.NewScanner(file)
-			for scanner.Scan() {
-				tmpEntry := strings.TrimSpace(scanner.Text())
-				if len(tmpEntry) == 0 {
-					continue
-				}
-				args = append(args, tmpEntry)
-			}
-
-			if err := scanner.Err(); err != nil {
-				return err
-			}
+		files, err := utils.GetListFiles(updateCmdFlags.packsListFileName)
+		if err != nil {
+			return err
 		}
+		args = append(args, files...)
 
 		var lastErr error
 
@@ -91,7 +70,7 @@ Update a pack using the following "<pack>" specification or using packs provided
 				return nil // nothing to do
 			}
 			installer.UnlockPackRoot()
-			err := installer.UpdatePack("", !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, viper.GetInt("timeout"))
+			err := installer.UpdatePack("", !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, false, viper.GetInt("timeout"))
 			if err != nil {
 				lastErr = err
 				if !errs.AlreadyLogged(err) {
@@ -105,7 +84,7 @@ Update a pack using the following "<pack>" specification or using packs provided
 		log.Debugf("Specified packs %v", args)
 		installer.UnlockPackRoot()
 		for _, packPath := range args {
-			err := installer.UpdatePack(packPath, !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, viper.GetInt("timeout"))
+			err := installer.UpdatePack(packPath, !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, false, viper.GetInt("timeout"))
 			if err != nil {
 				lastErr = err
 				if !errs.AlreadyLogged(err) {

cmd/commands/update_test.go

@@ -4,13 +4,11 @@
 package commands_test
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
-)
 
-var (
-// packFilePath1          = filepath.Join(testingDir, "TheVendor.PublicLocalPack.pack")
-// fileWithPacksListed1   = "file_with_listed_packs.txt"
-// fileWithNoPacksListed1 = "file_with_no_listed_packs.txt"
+	errs "github.com/open-cmsis-pack/cpackget/cmd/errors"
 )
 
 var updateCmdTests = []TestCase{
@@ -19,85 +17,70 @@ var updateCmdTests = []TestCase{
 		args:        []string{"help", "update"},
 		expectedErr: nil,
 	},
-	/*{
+	{
 		name:           "test updating pack file no args",
 		args:           []string{"update"},
 		createPackRoot: true,
-		expectedStdout: []string{"Missing a pack-path or list with pack urls specified via -f/--packs-list-filename"},
-		expectedErr:    errs.ErrIncorrectCmdArgs,
-	},*/
-	/*{
-		name:           "test updating pack file default mode",
-		args:           []string{"update", packFilePath1},
-		createPackRoot: true,
-		defaultMode:    true,
-		expectedStdout: []string{"updating pack", filepath.Base(packFilePath1)},
-	},*/
-	/*{
-		name:           "test updating pack file default mode no preexisting index",
-		args:           []string{"update", packFilePath1},
-		createPackRoot: false,
-		defaultMode:    true,
-		expectedStdout: []string{"updating pack", filepath.Base(packFilePath1)},
-	},*/
+		expectedErr:    nil,
+	},
 	{
 		name:           "test updating pack missing file",
 		args:           []string{"update", "DoesNotExist.Pack"},
 		createPackRoot: true,
-		//		expectedStdout: []string{"cannot be determined"},
-		//		expectedErr:    errs.ErrPackURLCannotBeFound,
+		expectedStdout: []string{"is not installed"},
 	},
-	/*	{
-			name:           "test updating pack file",
-			args:           []string{"update", packFilePath},
-			createPackRoot: true,
-			expectedStdout: []string{"updating pack", filepath.Base(packFilePath)},
+	{
+		name:           "test updating packs listed in file",
+		args:           []string{"update", "-f", fileWithPacksListed},
+		createPackRoot: true,
+		expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed,
+			"is not installed", filepath.Base("DoesNotExist.Pack")},
+		setUpFunc: func(t *TestCase) {
+			f, _ := os.Create(fileWithPacksListed)
+			_, _ = f.WriteString("DoesNotExist.Pack")
+			f.Close()
+		},
+		tearDownFunc: func() {
+			os.Remove(fileWithPacksListed)
 		},
-		{
-			name:           "test updating packs listed in file",
-			args:           []string{"update", "-f", fileWithPacksListed},
-			createPackRoot: true,
-			expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed,
-				"Updating pack", filepath.Base(packFilePath)},
-			setUpFunc: func(t *TestCase) {
-				f, _ := os.Create(fileWithPacksListed)
-				_, _ = f.WriteString(packFilePath)
-				f.Close()
-			},
-			tearDownFunc: func() {
-				os.Remove(fileWithPacksListed)
-			},
+	},
+	{
+		name:           "test updating empty packs list file",
+		args:           []string{"update", "-f", fileWithNoPacksListed},
+		createPackRoot: true,
+		expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
+		expectedErr:    nil,
+		setUpFunc: func(t *TestCase) {
+			f, _ := os.Create(fileWithNoPacksListed)
+			_, _ = f.WriteString("")
+			f.Close()
 		},
-		{
-			name:           "test updating empty packs list file",
-			args:           []string{"update", "-f", fileWithNoPacksListed},
-			createPackRoot: true,
-			expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
-			expectedErr:    nil,
-			setUpFunc: func(t *TestCase) {
-				f, _ := os.Create(fileWithNoPacksListed)
-				_, _ = f.WriteString("")
-				f.Close()
-			},
-			tearDownFunc: func() {
-				os.Remove(fileWithNoPacksListed)
-			},
+		tearDownFunc: func() {
+			os.Remove(fileWithNoPacksListed)
 		},
-		{
-			name:           "test updating empty packs list file (but whitespace characters)",
-			args:           []string{"update", "-f", fileWithNoPacksListed},
-			createPackRoot: true,
-			expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
-			expectedErr:    nil,
-			setUpFunc: func(t *TestCase) {
-				f, _ := os.Create(fileWithNoPacksListed)
-				_, _ = f.WriteString("  \n  \t  \n")
-				f.Close()
-			},
-			tearDownFunc: func() {
-				os.Remove(fileWithNoPacksListed)
-			},
-		},*/
+	},
+	{
+		name:           "test updating empty packs list file (but whitespace characters)",
+		args:           []string{"update", "-f", fileWithNoPacksListed},
+		createPackRoot: true,
+		expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
+		expectedErr:    nil,
+		setUpFunc: func(t *TestCase) {
+			f, _ := os.Create(fileWithNoPacksListed)
+			_, _ = f.WriteString("  \n  \t  \n")
+			f.Close()
+		},
+		tearDownFunc: func() {
+			os.Remove(fileWithNoPacksListed)
+		},
+	},
+	{
+		name:           "test updating packs listed in missing file",
+		args:           []string{"update", "-f", fileWithPacksListed},
+		createPackRoot: true,
+		expectedErr:    errs.ErrFileNotFound,
+		expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed},
+	},
 }
 
 func TestUpdateCmd(t *testing.T) {

cmd/installer/pack.go

@@ -84,6 +84,8 @@ func isGlobal(packPath string) (bool, error) {
 	if info.IsPackID {
 		return true, nil
 	}
+	// If the pack path is a URL, it is not global in the sense of this function
+	// TODO: shouldn't this check only compare "file://" and not http(s)?
 	if !strings.HasPrefix(info.Location, "http://") && !strings.HasPrefix(info.Location, "https://") && strings.HasPrefix(info.Location, "file://") {
 		return true, nil
 	}
@@ -141,6 +143,7 @@ func preparePack(packPath string, toBeRemoved, forceLatest, noLocal, nometa bool
 	pack.versionModifier = info.VersionModifier
 	pack.isPackID = info.IsPackID
 
+	// TODO: shouldn't this check only compare "file://" and not http(s)?
 	if !strings.HasPrefix(pack.URL, "http://") && !strings.HasPrefix(pack.URL, "https://") && strings.HasPrefix(pack.URL, "file://") {
 		pack.IsLocallySourced = true
 	}

cmd/installer/root.go

@@ -410,19 +410,21 @@ func massDownloadPdscFiles(pdscTag xml.PdscTag, skipInstalledPdscFiles bool, tim
 //
 // Returns:
 //   - error: An error if the update process fails, otherwise nil.
-func UpdatePack(packPath string, checkEula, noRequirements, subCall bool, timeout int) error {
+func UpdatePack(packPath string, checkEula, noRequirements, subCall, testing bool, timeout int) error {
 
 	if !subCall {
 		if packPath == "" {
-			if err := UpdatePublicIndexIfOnline(); err != nil {
-				return err
+			if !testing {
+				if err := UpdatePublicIndexIfOnline(); err != nil {
+					return err
+				}
 			}
 		} else {
 			global, err := isGlobal(packPath)
 			if err != nil {
 				return err
 			}
-			if global {
+			if global && !testing {
 				if err := UpdatePublicIndexIfOnline(); err != nil {
 					return err
 				}
@@ -435,7 +437,7 @@ func UpdatePack(packPath string, checkEula, noRequirements, subCall bool, timeou
 			return err
 		}
 		for _, installedPack := range installedPacks {
-			err = UpdatePack(installedPack.VName(), checkEula, noRequirements, true, timeout)
+			err = UpdatePack(installedPack.VName(), checkEula, noRequirements, true, testing, timeout)
 			if err != nil {
 				log.Error(err)
 			}

cmd/installer/root_pack_add_test.go

@@ -797,8 +797,8 @@ func TestAddPack(t *testing.T) {
 			//			assert.False(utils.FileExists(installer.Installation.PackIdx))
 		})
 
-		t.Run("test installing pack with pack id using release url"+packPath, func(t *testing.T) {
-			localTestingDir := "test-add-pack-with-pack-id-using-release-url" + safePackPath
+		t.Run("test installing pack with pack id using release url "+packPath, func(t *testing.T) {
+			localTestingDir := "test-add-pack-with-pack-id-using-release-url-" + safePackPath
 			assert.Nil(installer.SetPackRoot(localTestingDir, CreatePackRoot))
 			installer.UnlockPackRoot()
 			assert.Nil(installer.ReadIndexFiles())