Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@sfc-gh-knozderko
sfc-gh-knozderko committed Oct 11, 2023
1 parent 04082c1 commit d087974
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 64 deletions.
20 changes: 10 additions & 10 deletions client_configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const (
Trace string = "TRACE" // trace log level
)

// ClientConfig properties root
// ClientConfig config root
type ClientConfig struct {
Common *ClientConfigCommonProps `json:"common"`
}
Expand All @@ -35,18 +35,18 @@ func parseClientConfiguration(filePath string) (*ClientConfig, error) {
if filePath == "" {
return nil, nil
}

Check warning on line 37 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L36-L37 

Added lines #L36 - L37 were not covered by tests
fileContents, readError := os.ReadFile(filePath)
if readError != nil {
return nil, parsingClientConfigError(readError)
fileContents, readErr := os.ReadFile(filePath)
if readErr != nil {
return nil, parsingClientConfigError(readErr)
}

Check warning on line 41 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L40-L41 

Added lines #L40 - L41 were not covered by tests
var clientConfig ClientConfig
parseError := json.Unmarshal(fileContents, &clientConfig)
if parseError != nil {
return nil, parsingClientConfigError(parseError)
parseErr := json.Unmarshal(fileContents, &clientConfig)
if parseErr != nil {
return nil, parsingClientConfigError(parseErr)
}
validateError := validateClientConfiguration(&clientConfig)
if validateError != nil {
return nil, parsingClientConfigError(validateError)
validateErr := validateClientConfiguration(&clientConfig)
if validateErr != nil {
return nil, parsingClientConfigError(validateErr)
}
return &clientConfig, nil
}
Expand Down
108 changes: 54 additions & 54 deletions client_configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,18 @@ import (
"testing"
)

type PositiveTestCase struct {
Name string
FileName string
FileContents string
ExpectedLogLevel *string
ExpectedLogPath *string
}

type NegativeTestCase struct {
Name string
FileName string
FileContents string
ExpectedErrorMessageToContain string
}

func TestThatParsesConfiguration(t *testing.T) {
// given
dir := CreateTempDirectory(t, "conf_parse_tests_")
testCases := []PositiveTestCase{
dir := t.TempDir()
testCases := []struct {
Name string
FileName string
FileContents string
ExpectedLogLevel *string
ExpectedLogPath *string
}{
{
Name: "TestWithLogLevelUpperCase",
FileName: "config.json",
FileName: "config_1.json",
FileContents: `{
"common": {
"log_level" : "INFO",
Expand All @@ -44,7 +34,7 @@ func TestThatParsesConfiguration(t *testing.T) {
},
{
Name: "TestWithLogLevelLowerCase",
FileName: "config.json",
FileName: "config_2.json",
FileContents: `{
"common": {
"log_level" : "info",
Expand All @@ -56,37 +46,60 @@ func TestThatParsesConfiguration(t *testing.T) {
},
{
Name: "TestWithMissingValues",
FileName: "config.json",
FileName: "config_3.json",
FileContents: `{
"common": {}
}`,
ExpectedLogLevel: nil,
ExpectedLogPath: nil,
},
}
for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
fileName := CreateFile(t, testCase.FileName, testCase.FileContents, dir)
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
fileName := CreateFile(t, tc.FileName, tc.FileContents, dir)

// when
config, err := parseClientConfiguration(fileName)

// then
assert := assert.New(t)
assert.Equal(nil, err, "Error should be nil")
assert.Equal(testCase.ExpectedLogLevel, config.Common.LogLevel, "Log level should be as expected")
assert.Equal(testCase.ExpectedLogPath, config.Common.LogPath, "Log path should be as expected")
assert.Equal(tc.ExpectedLogLevel, config.Common.LogLevel, "Log level should be as expected")
assert.Equal(tc.ExpectedLogPath, config.Common.LogPath, "Log path should be as expected")
})
}
}

func TestThatFailsToParse(t *testing.T) {
// given
dir := CreateTempDirectory(t, "conf_negative_parse_tests_")
testCases := []NegativeTestCase{
func TestParseAllLogLevels(t *testing.T) {
dir := t.TempDir()
for _, logLevel := range []string{"OFF", "ERROR", "WARN", "INFO", "DEBUG", "TRACE"} {
t.Run(logLevel, func(t *testing.T) {
fileContents := fmt.Sprintf(`{
"common": {
"log_level" : "%s",
"log_path" : "/some-path/some-directory"
}
}`, logLevel)
fileName := CreateFile(t, fmt.Sprintf("config_%s.json", logLevel), fileContents, dir)

config, err := parseClientConfiguration(fileName)

assert := assert.New(t)
assert.Equal(nil, err, "Error should be nil")
assert.Equal(logLevel, *config.Common.LogLevel, "Log level should be as expected")
})
}
}

func TestParseConfigurationFails(t *testing.T) {
dir := t.TempDir()
testCases := []struct {
Name string
FileName string
FileContents string
ExpectedErrorMessageToContain string
}{
{
Name: "TestWithWrongLogLevel",
FileName: "config.json",
FileName: "config_1.json",
FileContents: `{
"common": {
"log_level" : "something weird",
Expand All @@ -97,7 +110,7 @@ func TestThatFailsToParse(t *testing.T) {
},
{
Name: "TestWithWrongTypeOfLogLevel",
FileName: "config.json",
FileName: "config_2.json",
FileContents: `{
"common": {
"log_level" : 15,
Expand All @@ -108,7 +121,7 @@ func TestThatFailsToParse(t *testing.T) {
},
{
Name: "TestWithWrongTypeOfLogPath",
FileName: "config.json",
FileName: "config_3.json",
FileContents: `{
"common": {
"log_level" : "INFO",
Expand All @@ -118,22 +131,20 @@ func TestThatFailsToParse(t *testing.T) {
ExpectedErrorMessageToContain: "ClientConfigCommonProps.common.log_path",
},
}
for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
fileName := CreateFile(t, testCase.FileName, testCase.FileContents, dir)
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
fileName := CreateFile(t, tc.FileName, tc.FileContents, dir)

// when
_, err := parseClientConfiguration(fileName)

// then
assert := assert.New(t)
assert.Equal(err != nil, true, "Error should not be nil")
errMessage := fmt.Sprint(err)
expectedPrefix := "parsing client config failed"
assert.Equal(strings.HasPrefix(errMessage, expectedPrefix), true,
fmt.Sprintf("Error message: \"%s\" should start with prefix: \"%s\"", errMessage, expectedPrefix))
assert.Equal(strings.Contains(errMessage, testCase.ExpectedErrorMessageToContain), true,
fmt.Sprintf("Error message: \"%s\" should contain given phrase: \"%s\"", errMessage, testCase.ExpectedErrorMessageToContain))
assert.Equal(strings.Contains(errMessage, tc.ExpectedErrorMessageToContain), true,
fmt.Sprintf("Error message: \"%s\" should contain given phrase: \"%s\"", errMessage, tc.ExpectedErrorMessageToContain))
})
}
}
Expand All @@ -147,18 +158,7 @@ func CreateFile(t *testing.T, fileName string, fileContents string, directory st
fullFileName := path.Join(directory, fileName)
writeErr := os.WriteFile(fullFileName, []byte(fileContents), 0644)
if writeErr != nil {
t.Error("Could not create file")
t.Fatal("Could not create file")
}
return fullFileName
}

func CreateTempDirectory(t *testing.T, dirPattern string) string {
dir, dirErr := os.MkdirTemp(os.TempDir(), dirPattern)
if dirErr != nil {
t.Error("Failed to create test directory")
}
t.Cleanup(func() {
os.RemoveAll(dir)
})
return dir
}

0 comments on commit d087974

Please sign in to comment.