Skip to content

Commit

Permalink
Unit tests for the StaticDhcpHost model (#8)
Browse files Browse the repository at this point in the history 
While writing the unit tests was verified the need for better error
handling during host conversion to the config format. The code was
improved to not produce inconsistent configurations.

Signed-off-by: Filipe Utzig <filipe@gringolito.com>
  • Loading branch information
@gringolito
gringolito authored Oct 10, 2024
1 parent 0601fc2 commit fbe3c66
Show file tree
Hide file tree
Showing 5 changed files with 316 additions and 47 deletions.
6 changes: 5 additions & 1 deletion pkg/host/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,11 @@ func (r *repository) parse(file *os.File) (*[]model.StaticDhcpHost, error) {
func (r *repository) save(hosts *[]model.StaticDhcpHost) error {
config := make([]string, 0, len(*hosts))
for _, host := range *hosts {
config = append(config, host.ToConfig())
hostConfig, err := host.ToConfig()
if err != nil {
return err
}
config = append(config, hostConfig)
}

err := os.WriteFile(r.staticHostsFilePath, []byte(strings.Join(config, "\n")), os.FileMode(0644))
Expand Down
92 changes: 52 additions & 40 deletions pkg/host/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,18 @@ func TestHostRepositoryFindAll(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, hosts *[]model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "FindAll() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "FindAll() returned an unexpected error type")
assert.Error(t, err, "FindAll() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "FindAll() returned an unexpected error")
},
},
{
name: "InvalidHostsFileError",
setupFileContent: InvalidHostsFileContent,
setup: voidSetup,
assert: func(t *testing.T, hosts *[]model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "FindAll() did NOT returned an expected error")
assert.Error(t, err, "FindAll() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "FindAll() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "FindAll() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down Expand Up @@ -180,8 +180,8 @@ func TestHostRepositoryFind(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "Find() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "Find() returned an unexpected error type")
assert.Error(t, err, "Find() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "Find() returned an unexpected error")
},
},
{
Expand All @@ -190,9 +190,9 @@ func TestHostRepositoryFind(t *testing.T) {
argument: &ValidHost,
setup: voidSetup,
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "Find() did NOT returned an expected error")
assert.Error(t, err, "Find() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "Find() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "Find() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down Expand Up @@ -259,8 +259,8 @@ func TestHostRepositoryFindByIP(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "FindByIP() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "FindByIP() returned an unexpected error type")
assert.Error(t, err, "FindByIP() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "FindByIP() returned an unexpected error")
},
},
{
Expand All @@ -269,9 +269,9 @@ func TestHostRepositoryFindByIP(t *testing.T) {
argument: ValidHost.IPAddress,
setup: voidSetup,
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "FindByIP() did NOT returned an expected error")
assert.Error(t, err, "FindByIP() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "FindByIP() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "FindByIP() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down Expand Up @@ -338,8 +338,8 @@ func TestHostRepositoryFindByMac(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "FindByMac() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "FindByMac() returned an unexpected error type")
assert.Error(t, err, "FindByMac() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "FindByMac() returned an unexpected error")
},
},
{
Expand All @@ -348,9 +348,9 @@ func TestHostRepositoryFindByMac(t *testing.T) {
argument: tests.ParseMAC(ValidMACAddress),
setup: voidSetup,
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "FindByMac() did NOT returned an expected error")
assert.Error(t, err, "FindByMac() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "FindByMac() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "FindByMac() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down Expand Up @@ -430,8 +430,8 @@ func TestHostRepositoryDelete(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "Delete() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "Delete() returned an unexpected error type")
assert.Error(t, err, "Delete() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "Delete() returned an unexpected error")
},
},
{
Expand All @@ -444,8 +444,8 @@ func TestHostRepositoryDelete(t *testing.T) {
f.Chmod(os.FileMode(0444))
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "Delete() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrPermission, "Delete() returned an unexpected error type")
assert.Error(t, err, "Delete() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrPermission, "Delete() returned an unexpected error")
},
},
{
Expand All @@ -454,9 +454,9 @@ func TestHostRepositoryDelete(t *testing.T) {
argument: &ValidHost,
setup: voidSetup,
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "Delete() did NOT returned an expected error")
assert.Error(t, err, "Delete() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "Delete() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "Delete() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down Expand Up @@ -536,8 +536,8 @@ func TestHostRepositoryDeleteByIP(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "DeleteByIP() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "DeleteByIP() returned an unexpected error type")
assert.Error(t, err, "DeleteByIP() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "DeleteByIP() returned an unexpected error")
},
},
{
Expand All @@ -550,8 +550,8 @@ func TestHostRepositoryDeleteByIP(t *testing.T) {
f.Chmod(os.FileMode(0444))
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "DeleteByIP() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrPermission, "DeleteByIP() returned an unexpected error type")
assert.Error(t, err, "DeleteByIP() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrPermission, "DeleteByIP() returned an unexpected error")
},
},
{
Expand All @@ -560,9 +560,9 @@ func TestHostRepositoryDeleteByIP(t *testing.T) {
argument: net.ParseIP(ValidIPAddress),
setup: voidSetup,
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "DeleteByIP() did NOT returned an expected error")
assert.Error(t, err, "DeleteByIP() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "DeleteByIP() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "DeleteByIP() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down Expand Up @@ -642,8 +642,8 @@ func TestHostRepositoryDeleteByMac(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "DeleteByMac() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "DeleteByMac() returned an unexpected error type")
assert.Error(t, err, "DeleteByMac() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "DeleteByMac() returned an unexpected error")
},
},
{
Expand All @@ -656,8 +656,8 @@ func TestHostRepositoryDeleteByMac(t *testing.T) {
f.Chmod(os.FileMode(0444))
},
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "DeleteByMac() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrPermission, "DeleteByMac() returned an unexpected error type")
assert.Error(t, err, "DeleteByMac() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrPermission, "DeleteByMac() returned an unexpected error")
},
},
{
Expand All @@ -666,9 +666,9 @@ func TestHostRepositoryDeleteByMac(t *testing.T) {
argument: tests.ParseMAC(ValidMACAddress),
setup: voidSetup,
assert: func(t *testing.T, host *model.StaticDhcpHost, err error, tc *testcase) {
assert.Error(t, err, "DeleteByMac() did NOT returned an expected error")
assert.Error(t, err, "DeleteByMac() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "DeleteByMac() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "DeleteByMac() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down Expand Up @@ -730,8 +730,8 @@ func TestHostRepositorySave(t *testing.T) {
os.Remove(tc.fileName)
},
assert: func(t *testing.T, err error, tc *testcase) {
assert.Error(t, err, "Save() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrNotExist, "Save() returned an unexpected error type")
assert.Error(t, err, "Save() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrNotExist, "Save() returned an unexpected error")
},
},
{
Expand All @@ -744,8 +744,8 @@ func TestHostRepositorySave(t *testing.T) {
f.Chmod(os.FileMode(0444))
},
assert: func(t *testing.T, err error, tc *testcase) {
assert.Error(t, err, "Save() did NOT returned an expected error")
assert.ErrorIs(t, err, os.ErrPermission, "Save() returned an unexpected error type")
assert.Error(t, err, "Save() did NOT returned an error")
assert.ErrorIs(t, err, os.ErrPermission, "Save() returned an unexpected error")
},
},
{
Expand All @@ -754,9 +754,21 @@ func TestHostRepositorySave(t *testing.T) {
host: &ValidHost,
setup: voidSetup,
assert: func(t *testing.T, err error, tc *testcase) {
assert.Error(t, err, "Save() did NOT returned an expected error")
assert.Error(t, err, "Save() did NOT returned an error")
// Just to ensure that we are not getting false negatives
assert.NotErrorIs(t, err, os.ErrNotExist, "Save() returned an unexpected error type")
assert.NotErrorIs(t, err, os.ErrNotExist, "Save() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
},
{
name: "InvalidHost",
setupFileContent: ValidHostFileContent,
host: &model.StaticDhcpHost{},
setup: voidSetup,
assert: func(t *testing.T, err error, tc *testcase) {
assert.Error(t, err, "Save() did NOT returned an error")
assert.ErrorIs(t, err, model.ErrDHCPHostMissingMACAddress, "Save() returned an unexpected error")
// Verify that the file content hasn't changed
assertFileContent(t, tc.setupFileContent, tc.fileName)
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/host/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ func TestHostServiceErrors(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
err := &DuplicatedEntryError{Field: test.field, Value: test.value}
expectedMessage := fmt.Sprintf(duplicatedEntryErrorMessage, test.field, test.value)
assert.ErrorContains(t, err, expectedMessage)
assert.EqualError(t, err, expectedMessage)
})
}
}
38 changes: 33 additions & 5 deletions pkg/model/static_dhcp_host.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,57 @@ type StaticDhcpHost struct {
HostName string
}

var ErrInvalidDHCPHost = errors.New("invalid DHCP host entry")
const errInvalidDHCPHostConfig = "invalid DHCP host config: %s"

var ErrDHCPHostMissingMACAddress = errors.New("invalid DHCP host: missing MAC address")
var ErrDHCPHostMissingIPAddress = errors.New("invalid DHCP host: missing IP address")
var ErrDHCPHostMissingHostName = errors.New("invalid DHCP host: missing hostname")

func (h *StaticDhcpHost) FromConfig(config string) error {
tokens := strings.Split(config, ",")
if len(tokens) != 3 {
return ErrInvalidDHCPHost
return fmt.Errorf(errInvalidDHCPHostConfig, config)
}

var mac string
_, err := fmt.Sscanf(tokens[0], "dhcp-host=%s", &mac)
if err != nil {
return err
return errors.Join(fmt.Errorf(errInvalidDHCPHostConfig, config), err)
}

h.MacAddress, err = net.ParseMAC(mac)
h.IPAddress = net.ParseIP(tokens[1])
if h.IPAddress == nil {
err = errors.Join(err, &net.AddrError{Err: "invalid IP address", Addr: tokens[1]})
}

h.HostName = tokens[2]

return err
}

func (h *StaticDhcpHost) ToConfig() string {
return fmt.Sprintf("dhcp-host=%s,%s,%s", h.MacAddress.String(), h.IPAddress.String(), h.HostName)
func (h *StaticDhcpHost) check() error {
var err error = nil
if h.MacAddress.String() == "" {
err = errors.Join(err, ErrDHCPHostMissingMACAddress)
}
if h.IPAddress.String() == "<nil>" {
err = errors.Join(err, ErrDHCPHostMissingIPAddress)
}
if h.HostName == "" {
err = errors.Join(err, ErrDHCPHostMissingHostName)
}
return err
}

func (h *StaticDhcpHost) ToConfig() (string, error) {
err := h.check()
if err != nil {
return "", err
}

config := fmt.Sprintf("dhcp-host=%s,%s,%s", h.MacAddress.String(), h.IPAddress.String(), h.HostName)
return config, nil
}

func (h *StaticDhcpHost) Equal(other StaticDhcpHost) bool {
Expand Down
Loading

0 comments on commit fbe3c66

Please sign in to comment.