Skip to content

Commit

Permalink
fix: Applying review comments: using errors.Is to assert on errors in…
Browse files Browse the repository at this point in the history
…stead of their strings
  • Loading branch information
RawanMostafa08 committed Sep 24, 2024
1 parent 1362a24 commit ad396fa
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 16 deletions.
3 changes: 2 additions & 1 deletion pkg/iniparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var ErrNoKey = errors.New("key not found")
var ErrNoSection = errors.New("section not found")
var ErrNotINI = errors.New("this is not an ini file")
var ErrGlobalKey = errors.New("global keys aren't supported")
var ErrFileNotExist = errors.New("file doesn't exist")

// The Parser acts as the data structure storing all of the parsed sections
type Parser struct {
Expand Down Expand Up @@ -88,7 +89,7 @@ func (i Parser) LoadFromFile(path string) error {

data, err := os.ReadFile(path)
if err != nil {
return fmt.Errorf("error in reading the file: %w", err)
return ErrFileNotExist
}
i.LoadFromString(string(data))

Check failure on line 94 in pkg/iniparser.go

View workflow job for this annotation

GitHub Actions / golangci

Error return value of `i.LoadFromString` is not checked (errcheck)
return nil
Expand Down
29 changes: 14 additions & 15 deletions pkg/iniparser_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package iniparser

import (
"errors"
"fmt"
"os"
"reflect"
Expand Down Expand Up @@ -107,7 +108,7 @@ func TestLoadFromString(t *testing.T) {
organization = Acme Widgets Inc.
[database]`

parser := NewParser()
err := parser.LoadFromString(IniFile)
assertEquality(t, err, ErrGlobalKey)
Expand All @@ -122,23 +123,23 @@ func TestLoadFromFile(t *testing.T) {
testcaseName string
filePath string
expected map[string]map[string]string
err string
err error
}{
{
testcaseName: "Normal case: ini file is present",
filePath: "testdata/file.ini",
expected: expectedFile,
err: "",
err: nil,
},
{
testcaseName: "corner case: not an ini file",
filePath: "testdata/file.txt",
err: "this is not an ini file",
err: ErrNotINI,
},
{
testcaseName: "corner case: file not found",
filePath: "testdata/filex.ini",
err: "error in reading the file: open testdata/filex.ini: no such file or directory",
err: ErrFileNotExist,
},
}
for _, testcase := range testcases {
Expand All @@ -147,11 +148,10 @@ func TestLoadFromFile(t *testing.T) {
parser := NewParser()

err := parser.LoadFromFile(testcase.filePath)
if testcase.err == "" {
if testcase.err == nil {
assertEquality(t, expectedFile, parser.sections)
} else {
assertEquality(t, testcase.err, err.Error())
}
assertEquality(t, errors.Is(err, testcase.err), true)

})
}
Expand Down Expand Up @@ -246,26 +246,26 @@ func TestSet(t *testing.T) {
sectionName string
key string
value string
err string
err error
}{
{
testcaseName: "Normal case: section and key are present",
sectionName: "database",
key: "server",
value: "127.0.0.1",
err: "",
err: nil,
},
{
testcaseName: "corner case: section not found",
sectionName: "user",
key: "server",
err: "section not found",
err: ErrNoSection,
},
{
testcaseName: "corner case: key not found",
sectionName: "database",
key: "size",
err: "key not found",
err: ErrNoKey,
},
}
for _, testcase := range testcases {
Expand All @@ -277,12 +277,11 @@ func TestSet(t *testing.T) {
t.Errorf("Error! %v", err)
}
err = parser.Set(testcase.sectionName, testcase.key, testcase.value)
if testcase.err == "" {
if testcase.err == nil {
value := parser.sections[testcase.sectionName][testcase.key]
assertEquality(t, testcase.value, value)
} else {
assertEquality(t, testcase.err, err.Error())
}
assertEquality(t, errors.Is(err, testcase.err), true)
})
}

Expand Down

0 comments on commit ad396fa

Please sign in to comment.