Skip to content

Commit 5d2e42c

Browse files
bgn42bgn42
authored
more tests to enhance coverage (#513)
## Changes more tests - ## Checklist <!-- Put an `x` in the boxes. All tasks must be completed and boxes checked before merging. --> - [x] 🤖 This change is covered by unit tests (if applicable). - [x] 🤹 Manual testing has been performed (if necessary). - [x] 🛡️ Security impacts have been considered (if relevant). - [x] 📖 Documentation updates are complete (if required). - [x] 🧠 Third-party dependencies and TPIP updated (if required).
1 parent 959ed79 commit 5d2e42c

File tree

14 files changed

+445
-131
lines changed

14 files changed

+445
-131
lines changed

‎cmd/commands/add.go‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,7 @@
44
package commands
55

66
import (
7-
"bufio"
8-
"os"
97
"path/filepath"
10-
"strings"
118

129
errs "github.com/open-cmsis-pack/cpackget/cmd/errors"
1310
"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
8178
return err
8279
}
8380

84-
if addCmdFlags.packsListFileName != "" {
85-
log.Infof("Parsing packs urls via file %v", addCmdFlags.packsListFileName)
86-
87-
file, err := os.Open(addCmdFlags.packsListFileName)
88-
if err != nil {
89-
return err
90-
}
91-
defer file.Close()
92-
93-
scanner := bufio.NewScanner(file)
94-
for scanner.Scan() {
95-
tmpEntry := strings.TrimSpace(scanner.Text())
96-
if len(tmpEntry) == 0 {
97-
continue
98-
}
99-
args = append(args, tmpEntry)
100-
}
101-
102-
if err := scanner.Err(); err != nil {
103-
return err
104-
}
81+
files, err := utils.GetListFiles(addCmdFlags.packsListFileName)
82+
if err != nil {
83+
return err
10584
}
85+
args = append(args, files...)
10686

10787
if len(args) == 0 {
10888
log.Warn("Missing a pack-path or list with pack urls specified via -f/--packs-list-filename")

‎cmd/commands/add_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ var addCmdTests = []TestCase{
109109
os.Remove(fileWithNoPacksListed)
110110
},
111111
},
112+
{
113+
name: "test adding packs listed in missing file",
114+
args: []string{"add", "-f", fileWithPacksListed},
115+
createPackRoot: true,
116+
expectedErr: errs.ErrFileNotFound,
117+
expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed},
118+
},
112119
}
113120

114121
func TestAddCmd(t *testing.T) {

‎cmd/commands/root_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,12 @@ func runTests(t *testing.T, tests []TestCase) {
138138
test.assert = assert
139139
t.Run(test.name, func(t *testing.T) {
140140
localTestingDir := strings.ReplaceAll(test.name, " ", "_")
141+
142+
// Save the original environment variables to restore them after the test
143+
originalEnv := make(map[string]string)
144+
originalEnv["CPACKGET_DEFAULT_MODE_PATH"] = os.Getenv("CPACKGET_DEFAULT_MODE_PATH")
145+
originalEnv["CMSIS_PACK_ROOT"] = os.Getenv("CMSIS_PACK_ROOT")
146+
141147
if test.defaultMode {
142148
os.Setenv("CPACKGET_DEFAULT_MODE_PATH", localTestingDir)
143149
}
@@ -194,6 +200,12 @@ func runTests(t *testing.T, tests []TestCase) {
194200
})
195201
}
196202

203+
// Reset the environment variables to their original values
204+
// after the command execution
205+
for key, value := range originalEnv {
206+
os.Setenv(key, value)
207+
}
208+
197209
outBytes, err1 := io.ReadAll(stdout)
198210
errBytes, err2 := io.ReadAll(stderr)
199211
assert.Nil(err1)

‎cmd/commands/update.go‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@
44
package commands
55

66
import (
7-
"bufio"
8-
"os"
9-
"strings"
10-
117
errs "github.com/open-cmsis-pack/cpackget/cmd/errors"
128
"github.com/open-cmsis-pack/cpackget/cmd/installer"
139
"github.com/open-cmsis-pack/cpackget/cmd/utils"
@@ -61,28 +57,11 @@ Update a pack using the following "<pack>" specification or using packs provided
6157
return err
6258
}
6359

64-
if updateCmdFlags.packsListFileName != "" {
65-
log.Infof("Parsing packs urls via file %v", updateCmdFlags.packsListFileName)
66-
67-
file, err := os.Open(updateCmdFlags.packsListFileName)
68-
if err != nil {
69-
return err
70-
}
71-
defer file.Close()
72-
73-
scanner := bufio.NewScanner(file)
74-
for scanner.Scan() {
75-
tmpEntry := strings.TrimSpace(scanner.Text())
76-
if len(tmpEntry) == 0 {
77-
continue
78-
}
79-
args = append(args, tmpEntry)
80-
}
81-
82-
if err := scanner.Err(); err != nil {
83-
return err
84-
}
60+
files, err := utils.GetListFiles(updateCmdFlags.packsListFileName)
61+
if err != nil {
62+
return err
8563
}
64+
args = append(args, files...)
8665

8766
var lastErr error
8867

@@ -91,7 +70,7 @@ Update a pack using the following "<pack>" specification or using packs provided
9170
return nil // nothing to do
9271
}
9372
installer.UnlockPackRoot()
94-
err := installer.UpdatePack("", !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, viper.GetInt("timeout"))
73+
err := installer.UpdatePack("", !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, false, viper.GetInt("timeout"))
9574
if err != nil {
9675
lastErr = err
9776
if !errs.AlreadyLogged(err) {
@@ -105,7 +84,7 @@ Update a pack using the following "<pack>" specification or using packs provided
10584
log.Debugf("Specified packs %v", args)
10685
installer.UnlockPackRoot()
10786
for _, packPath := range args {
108-
err := installer.UpdatePack(packPath, !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, viper.GetInt("timeout"))
87+
err := installer.UpdatePack(packPath, !updateCmdFlags.skipEula, updateCmdFlags.noRequirements, false, false, viper.GetInt("timeout"))
10988
if err != nil {
11089
lastErr = err
11190
if !errs.AlreadyLogged(err) {

‎cmd/commands/update_test.go‎

Lines changed: 56 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@
44
package commands_test
55

66
import (
7+
"os"
8+
"path/filepath"
79
"testing"
8-
)
910

10-
var (
11-
// packFilePath1 = filepath.Join(testingDir, "TheVendor.PublicLocalPack.pack")
12-
// fileWithPacksListed1 = "file_with_listed_packs.txt"
13-
// fileWithNoPacksListed1 = "file_with_no_listed_packs.txt"
11+
errs "github.com/open-cmsis-pack/cpackget/cmd/errors"
1412
)
1513

1614
var updateCmdTests = []TestCase{
@@ -19,85 +17,70 @@ var updateCmdTests = []TestCase{
1917
args: []string{"help", "update"},
2018
expectedErr: nil,
2119
},
22-
/*{
20+
{
2321
name: "test updating pack file no args",
2422
args: []string{"update"},
2523
createPackRoot: true,
26-
expectedStdout: []string{"Missing a pack-path or list with pack urls specified via -f/--packs-list-filename"},
27-
expectedErr: errs.ErrIncorrectCmdArgs,
28-
},*/
29-
/*{
30-
name: "test updating pack file default mode",
31-
args: []string{"update", packFilePath1},
32-
createPackRoot: true,
33-
defaultMode: true,
34-
expectedStdout: []string{"updating pack", filepath.Base(packFilePath1)},
35-
},*/
36-
/*{
37-
name: "test updating pack file default mode no preexisting index",
38-
args: []string{"update", packFilePath1},
39-
createPackRoot: false,
40-
defaultMode: true,
41-
expectedStdout: []string{"updating pack", filepath.Base(packFilePath1)},
42-
},*/
24+
expectedErr: nil,
25+
},
4326
{
4427
name: "test updating pack missing file",
4528
args: []string{"update", "DoesNotExist.Pack"},
4629
createPackRoot: true,
47-
// expectedStdout: []string{"cannot be determined"},
48-
// expectedErr: errs.ErrPackURLCannotBeFound,
30+
expectedStdout: []string{"is not installed"},
4931
},
50-
/* {
51-
name: "test updating pack file",
52-
args: []string{"update", packFilePath},
53-
createPackRoot: true,
54-
expectedStdout: []string{"updating pack", filepath.Base(packFilePath)},
32+
{
33+
name: "test updating packs listed in file",
34+
args: []string{"update", "-f", fileWithPacksListed},
35+
createPackRoot: true,
36+
expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed,
37+
"is not installed", filepath.Base("DoesNotExist.Pack")},
38+
setUpFunc: func(t *TestCase) {
39+
f, _ := os.Create(fileWithPacksListed)
40+
_, _ = f.WriteString("DoesNotExist.Pack")
41+
f.Close()
42+
},
43+
tearDownFunc: func() {
44+
os.Remove(fileWithPacksListed)
5545
},
56-
{
57-
name: "test updating packs listed in file",
58-
args: []string{"update", "-f", fileWithPacksListed},
59-
createPackRoot: true,
60-
expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed,
61-
"Updating pack", filepath.Base(packFilePath)},
62-
setUpFunc: func(t *TestCase) {
63-
f, _ := os.Create(fileWithPacksListed)
64-
_, _ = f.WriteString(packFilePath)
65-
f.Close()
66-
},
67-
tearDownFunc: func() {
68-
os.Remove(fileWithPacksListed)
69-
},
46+
},
47+
{
48+
name: "test updating empty packs list file",
49+
args: []string{"update", "-f", fileWithNoPacksListed},
50+
createPackRoot: true,
51+
expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
52+
expectedErr: nil,
53+
setUpFunc: func(t *TestCase) {
54+
f, _ := os.Create(fileWithNoPacksListed)
55+
_, _ = f.WriteString("")
56+
f.Close()
7057
},
71-
{
72-
name: "test updating empty packs list file",
73-
args: []string{"update", "-f", fileWithNoPacksListed},
74-
createPackRoot: true,
75-
expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
76-
expectedErr: nil,
77-
setUpFunc: func(t *TestCase) {
78-
f, _ := os.Create(fileWithNoPacksListed)
79-
_, _ = f.WriteString("")
80-
f.Close()
81-
},
82-
tearDownFunc: func() {
83-
os.Remove(fileWithNoPacksListed)
84-
},
58+
tearDownFunc: func() {
59+
os.Remove(fileWithNoPacksListed)
8560
},
86-
{
87-
name: "test updating empty packs list file (but whitespace characters)",
88-
args: []string{"update", "-f", fileWithNoPacksListed},
89-
createPackRoot: true,
90-
expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
91-
expectedErr: nil,
92-
setUpFunc: func(t *TestCase) {
93-
f, _ := os.Create(fileWithNoPacksListed)
94-
_, _ = f.WriteString(" \n \t \n")
95-
f.Close()
96-
},
97-
tearDownFunc: func() {
98-
os.Remove(fileWithNoPacksListed)
99-
},
100-
},*/
61+
},
62+
{
63+
name: "test updating empty packs list file (but whitespace characters)",
64+
args: []string{"update", "-f", fileWithNoPacksListed},
65+
createPackRoot: true,
66+
expectedStdout: []string{"Parsing packs urls via file " + fileWithNoPacksListed},
67+
expectedErr: nil,
68+
setUpFunc: func(t *TestCase) {
69+
f, _ := os.Create(fileWithNoPacksListed)
70+
_, _ = f.WriteString(" \n \t \n")
71+
f.Close()
72+
},
73+
tearDownFunc: func() {
74+
os.Remove(fileWithNoPacksListed)
75+
},
76+
},
77+
{
78+
name: "test updating packs listed in missing file",
79+
args: []string{"update", "-f", fileWithPacksListed},
80+
createPackRoot: true,
81+
expectedErr: errs.ErrFileNotFound,
82+
expectedStdout: []string{"Parsing packs urls via file " + fileWithPacksListed},
83+
},
10184
}
10285

10386
func TestUpdateCmd(t *testing.T) {

‎cmd/installer/pack.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ func isGlobal(packPath string) (bool, error) {
8484
if info.IsPackID {
8585
return true, nil
8686
}
87+
// If the pack path is a URL, it is not global in the sense of this function
88+
// TODO: shouldn't this check only compare "file://" and not http(s)?
8789
if !strings.HasPrefix(info.Location, "http://") && !strings.HasPrefix(info.Location, "https://") && strings.HasPrefix(info.Location, "file://") {
8890
return true, nil
8991
}
@@ -141,6 +143,7 @@ func preparePack(packPath string, toBeRemoved, forceLatest, noLocal, nometa bool
141143
pack.versionModifier = info.VersionModifier
142144
pack.isPackID = info.IsPackID
143145

146+
// TODO: shouldn't this check only compare "file://" and not http(s)?
144147
if !strings.HasPrefix(pack.URL, "http://") && !strings.HasPrefix(pack.URL, "https://") && strings.HasPrefix(pack.URL, "file://") {
145148
pack.IsLocallySourced = true
146149
}

‎cmd/installer/root.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -410,19 +410,21 @@ func massDownloadPdscFiles(pdscTag xml.PdscTag, skipInstalledPdscFiles bool, tim
410410
//
411411
// Returns:
412412
// - error: An error if the update process fails, otherwise nil.
413-
func UpdatePack(packPath string, checkEula, noRequirements, subCall bool, timeout int) error {
413+
func UpdatePack(packPath string, checkEula, noRequirements, subCall, testing bool, timeout int) error {
414414

415415
if !subCall {
416416
if packPath == "" {
417-
if err := UpdatePublicIndexIfOnline(); err != nil {
418-
return err
417+
if !testing {
418+
if err := UpdatePublicIndexIfOnline(); err != nil {
419+
return err
420+
}
419421
}
420422
} else {
421423
global, err := isGlobal(packPath)
422424
if err != nil {
423425
return err
424426
}
425-
if global {
427+
if global && !testing {
426428
if err := UpdatePublicIndexIfOnline(); err != nil {
427429
return err
428430
}
@@ -435,7 +437,7 @@ func UpdatePack(packPath string, checkEula, noRequirements, subCall bool, timeou
435437
return err
436438
}
437439
for _, installedPack := range installedPacks {
438-
err = UpdatePack(installedPack.VName(), checkEula, noRequirements, true, timeout)
440+
err = UpdatePack(installedPack.VName(), checkEula, noRequirements, true, testing, timeout)
439441
if err != nil {
440442
log.Error(err)
441443
}

‎cmd/installer/root_pack_add_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,8 +797,8 @@ func TestAddPack(t *testing.T) {
797797
// assert.False(utils.FileExists(installer.Installation.PackIdx))
798798
})
799799

800-
t.Run("test installing pack with pack id using release url"+packPath, func(t *testing.T) {
801-
localTestingDir := "test-add-pack-with-pack-id-using-release-url" + safePackPath
800+
t.Run("test installing pack with pack id using release url "+packPath, func(t *testing.T) {
801+
localTestingDir := "test-add-pack-with-pack-id-using-release-url-" + safePackPath
802802
assert.Nil(installer.SetPackRoot(localTestingDir, CreatePackRoot))
803803
installer.UnlockPackRoot()
804804
assert.Nil(installer.ReadIndexFiles())

0 commit comments

Comments
 (0)