Improve test coverage

Signed-off-by: David Gageot <david@gageot.net>

drivers/virtualbox/disk.go

@@ -1,10 +1,5 @@
 package virtualbox
 
-import (
-	"bufio"
-	"strings"
-)
-
 type VirtualDisk struct {
 	UUID string
 	Path string
@@ -16,34 +11,19 @@ func getVMDiskInfo(name string, vbox VBoxManager) (*VirtualDisk, error) {
 		return nil, err
 	}
 
-	return parseDiskInfo(out)
-}
-
-func parseDiskInfo(out string) (*VirtualDisk, error) {
 	disk := &VirtualDisk{}
 
-	r := strings.NewReader(out)
+	err = parseKeyValues(out, reEqualQuoteLine, func(key, val string) error {
-	s := bufio.NewScanner(r)
-	for s.Scan() {
-		line := s.Text()
-		if line == "" {
-			continue
-		}
-
-		res := reEqualQuoteLine.FindStringSubmatch(line)
-		if res == nil {
-			continue
-		}
-
-		key, val := res[1], res[2]
 		switch key {
 		case "SATA-1-0":
 			disk.Path = val
 		case "SATA-ImageUUID-1-0":
 			disk.UUID = val
 		}
-	}
+
-	if err := s.Err(); err != nil {
+		return nil
+	})
+	if err != nil {
 		return nil, err
 	}
 

drivers/virtualbox/disk_test.go

@@ -7,22 +7,19 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-const (
+const stdOutDiskInfo = `
-	validDiskInfoText = `
 storagecontrollerbootable0="on"
 "SATA-0-0"="/home/ehazlett/.boot2docker/boot2docker.iso"
 "SATA-IsEjected"="off"
 "SATA-1-0"="/home/ehazlett/vm/test/disk.vmdk"
 "SATA-ImageUUID-1-0"="12345-abcdefg"
 "SATA-2-0"="none"
-nic1="nat"
+nic1="nat"`
-    `
-)
 
 func TestVMDiskInfo(t *testing.T) {
 	vbox := &VBoxManagerMock{
 		args:   "showvminfo default --machinereadable",
-		stdOut: validDiskInfoText,
+		stdOut: stdOutDiskInfo,
 	}
 
 	disk, err := getVMDiskInfo("default", vbox)

drivers/virtualbox/network.go

@@ -1,13 +1,11 @@
 package virtualbox
 
 import (
-	"bufio"
 	"errors"
 	"fmt"
 	"net"
 	"regexp"
 	"strconv"
-	"strings"
 )
 
 const (
@@ -79,19 +77,8 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error)
 	m := map[string]*hostOnlyNetwork{}
 	n := &hostOnlyNetwork{}
 
-	s := bufio.NewScanner(strings.NewReader(out))
+	err = parseKeyValues(out, reColonLine, func(key, val string) error {
-	for s.Scan() {
+		switch key {
-		line := s.Text()
-		if line == "" {
-			continue
-		}
-
-		res := reColonLine.FindStringSubmatch(line)
-		if res == nil {
-			continue
-		}
-
-		switch key, val := res[1], res[2]; key {
 		case "Name":
 			n.Name = val
 		case "GUID":
@@ -107,13 +94,13 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error)
 		case "IPV6NetworkMaskPrefixLength":
 			l, err := strconv.ParseUint(val, 10, 8)
 			if err != nil {
-				return nil, err
+				return err
 			}
 			n.IPv6.Mask = net.CIDRMask(int(l), net.IPv6len*8)
 		case "HardwareAddress":
 			mac, err := net.ParseMAC(val)
 			if err != nil {
-				return nil, err
+				return err
 			}
 			n.HwAddr = mac
 		case "MediumType":
@@ -125,11 +112,13 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error)
 			m[val] = n
 			n = &hostOnlyNetwork{}
 		}
-	}
 
-	if err := s.Err(); err != nil {
+		return nil
+	})
+	if err != nil {
 		return nil, err
 	}
+
 	return m, nil
 }
 
@@ -247,22 +236,15 @@ func getDHCPServers(vbox VBoxManager) (map[string]*dhcpServer, error) {
 	if err != nil {
 		return nil, err
 	}
-	s := bufio.NewScanner(strings.NewReader(out))
+
 	m := map[string]*dhcpServer{}
 	dhcp := &dhcpServer{}
-	for s.Scan() {
+
-		line := s.Text()
+	err = parseKeyValues(out, reColonLine, func(key, val string) error {
-		if line == "" {
+		switch key {
-			m[dhcp.NetworkName] = dhcp
-			dhcp = &dhcpServer{}
-			continue
-		}
-		res := reColonLine.FindStringSubmatch(line)
-		if res == nil {
-			continue
-		}
-		switch key, val := res[1], res[2]; key {
 		case "NetworkName":
+			dhcp = &dhcpServer{}
+			m[val] = dhcp
 			dhcp.NetworkName = val
 		case "IP":
 			dhcp.IPv4.IP = net.ParseIP(val)
@@ -275,10 +257,13 @@ func getDHCPServers(vbox VBoxManager) (map[string]*dhcpServer, error) {
 		case "Enabled":
 			dhcp.Enabled = (val == "Yes")
 		}
-	}
+
-	if err := s.Err(); err != nil {
+		return nil
+	})
+	if err != nil {
 		return nil, err
 	}
+
 	return m, nil
 }
 

drivers/virtualbox/network_test.go

@@ -8,7 +8,9 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-const stdOutOneHostOnlyNetwork = `Name:            vboxnet0
+const (
+	stdOutOneHostOnlyNetwork = `
+Name:            vboxnet0
 GUID:            786f6276-656e-4074-8000-0a0027000000
 DHCP:            Disabled
 IPAddress:       192.168.99.1
@@ -21,7 +23,8 @@ Status:          Up
 VBoxNetworkName: HostInterfaceNetworking-vboxnet0
 
 `
-const stdOutTwoHostOnlyNetwork = `Name:            vboxnet0
+	stdOutTwoHostOnlyNetwork = `
+Name:            vboxnet0
 GUID:            786f6276-656e-4074-8000-0a0027000000
 DHCP:            Disabled
 IPAddress:       192.168.99.1
@@ -44,8 +47,23 @@ HardwareAddress: 0a:00:27:00:00:01
 MediumType:      Ethernet
 Status:          Up
 VBoxNetworkName: HostInterfaceNetworking-vboxnet1
-
 `
+	stdOutListTwoDHCPServers = `
+NetworkName:    HostInterfaceNetworking-vboxnet0
+IP:             192.168.99.6
+NetworkMask:    255.255.255.0
+lowerIPAddress: 192.168.99.100
+upperIPAddress: 192.168.99.254
+Enabled:        Yes
+
+NetworkName:    HostInterfaceNetworking-vboxnet1
+IP:             192.168.99.7
+NetworkMask:    255.255.255.0
+lowerIPAddress: 192.168.99.100
+upperIPAddress: 192.168.99.254
+Enabled:        No
+`
+)
 
 // Tests that when we have a host only network which matches our expectations,
 // it gets returned correctly.
@@ -223,3 +241,33 @@ func TestFailWithDuplicateHostOnlyNetworks(t *testing.T) {
 	assert.Nil(t, net)
 	assert.Equal(t, errDuplicateHostOnlyInterfaceNetworks, err)
 }
+
+func TestGetDHCPServers(t *testing.T) {
+	vbox := &VBoxManagerMock{
+		args:   "list dhcpservers",
+		stdOut: stdOutListTwoDHCPServers,
+	}
+
+	servers, err := getDHCPServers(vbox)
+
+	assert.Equal(t, 2, len(servers))
+	assert.NoError(t, err)
+
+	server, present := servers["HostInterfaceNetworking-vboxnet0"]
+	assert.True(t, present)
+	assert.Equal(t, "HostInterfaceNetworking-vboxnet0", server.NetworkName)
+	assert.Equal(t, "192.168.99.6", server.IPv4.IP.String())
+	assert.Equal(t, "192.168.99.100", server.LowerIP.String())
+	assert.Equal(t, "192.168.99.254", server.UpperIP.String())
+	assert.Equal(t, "ffffff00", server.IPv4.Mask.String())
+	assert.True(t, server.Enabled)
+
+	server, present = servers["HostInterfaceNetworking-vboxnet1"]
+	assert.True(t, present)
+	assert.Equal(t, "HostInterfaceNetworking-vboxnet1", server.NetworkName)
+	assert.Equal(t, "192.168.99.7", server.IPv4.IP.String())
+	assert.Equal(t, "192.168.99.100", server.LowerIP.String())
+	assert.Equal(t, "192.168.99.254", server.UpperIP.String())
+	assert.Equal(t, "ffffff00", server.IPv4.Mask.String())
+	assert.False(t, server.Enabled)
+}

drivers/virtualbox/vbm.go

@@ -1,6 +1,7 @@
 package virtualbox
 
 import (
+	"bufio"
 	"bytes"
 	"errors"
 	"fmt"
@@ -83,3 +84,27 @@ func checkVBoxManageVersion(version string) error {
 
 	return nil
 }
+
+func parseKeyValues(stdOut string, regexp *regexp.Regexp, callback func(key, val string) error) error {
+	r := strings.NewReader(stdOut)
+	s := bufio.NewScanner(r)
+
+	for s.Scan() {
+		line := s.Text()
+		if line == "" {
+			continue
+		}
+
+		res := regexp.FindStringSubmatch(line)
+		if res == nil {
+			continue
+		}
+
+		key, val := res[1], res[2]
+		if err := callback(key, val); err != nil {
+			return err
+		}
+	}
+
+	return s.Err()
+}

drivers/virtualbox/vm.go

@@ -1,11 +1,6 @@
 package virtualbox
 
-import (
+import "strconv"
-	"bufio"
-	"io"
-	"strconv"
-	"strings"
-)
 
 type VM struct {
 	CPUs   int
@@ -18,39 +13,29 @@ func getVMInfo(name string, vbox VBoxManager) (*VM, error) {
 		return nil, err
 	}
 
-	r := strings.NewReader(out)
-	return parseVMInfo(r)
-}
-
-func parseVMInfo(r io.Reader) (*VM, error) {
-	s := bufio.NewScanner(r)
 	vm := &VM{}
-	for s.Scan() {
+
-		line := s.Text()
+	err = parseKeyValues(out, reEqualLine, func(key, val string) error {
-		if line == "" {
+		switch key {
-			continue
-		}
-		res := reEqualLine.FindStringSubmatch(line)
-		if res == nil {
-			continue
-		}
-		switch key, val := res[1], res[2]; key {
 		case "cpus":
 			v, err := strconv.Atoi(val)
 			if err != nil {
-				return nil, err
+				return err
 			}
 			vm.CPUs = v
 		case "memory":
 			v, err := strconv.Atoi(val)
 			if err != nil {
-				return nil, err
+				return err
 			}
 			vm.Memory = v
 		}
-	}
+
-	if err := s.Err(); err != nil {
+		return nil
+	})
+	if err != nil {
 		return nil, err
 	}
+
 	return vm, nil
 }

drivers/virtualbox/vm_test.go

@@ -1,12 +1,13 @@
 package virtualbox
 
 import (
-	"strings"
 	"testing"
+
+	"github.com/docker/machine/drivers/vmwarevsphere/errors"
+	"github.com/stretchr/testify/assert"
 )
 
-var (
+var stdOutVMInfo = `
-	testVMInfoText = `
 storagecontrollerbootable0="on"
 memory=1024
 cpus=2
@@ -15,24 +16,29 @@ cpus=2
 "SATA-1-0"="/home/ehazlett/vm/test/disk.vmdk"
 "SATA-ImageUUID-1-0"="12345-abcdefg"
 "SATA-2-0"="none"
-nic1="nat"
+nic1="nat"`
-`
-)
 
 func TestVMInfo(t *testing.T) {
-	r := strings.NewReader(testVMInfoText)
+	vbox := &VBoxManagerMock{
-	vm, err := parseVMInfo(r)
+		args:   "showvminfo host --machinereadable",
-	if err != nil {
+		stdOut: stdOutVMInfo,
-		t.Fatal(err)
 	}
 
-	vmCPUs := 2
+	vm, err := getVMInfo("host", vbox)
-	vmMemory := 1024
-	if vm.CPUs != vmCPUs {
-		t.Fatalf("expected %d cpus; received %d", vmCPUs, vm.CPUs)
-	}
 
-	if vm.Memory != vmMemory {
+	assert.Equal(t, 2, vm.CPUs)
-		t.Fatalf("expected memory %d; received %d", vmMemory, vm.Memory)
+	assert.Equal(t, 1024, vm.Memory)
-	}
+	assert.NoError(t, err)
+}
+
+func TestVMInfoError(t *testing.T) {
+	vbox := &VBoxManagerMock{
+		args: "showvminfo host --machinereadable",
+		err:  errors.New("BUG"),
+	}
+
+	vm, err := getVMInfo("host", vbox)
+
+	assert.Nil(t, vm)
+	assert.EqualError(t, err, "BUG")
 }