From 84199a12e33432e81b41990c1affe7c5485e44e0 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 24 Nov 2015 10:22:34 +0100 Subject: [PATCH] Improve test coverage Signed-off-by: David Gageot --- drivers/virtualbox/disk.go | 30 +++-------------- drivers/virtualbox/disk_test.go | 9 ++--- drivers/virtualbox/network.go | 53 +++++++++++------------------ drivers/virtualbox/network_test.go | 54 ++++++++++++++++++++++++++++-- drivers/virtualbox/vbm.go | 25 ++++++++++++++ drivers/virtualbox/vm.go | 37 ++++++-------------- drivers/virtualbox/vm_test.go | 42 +++++++++++++---------- 7 files changed, 138 insertions(+), 112 deletions(-) diff --git a/drivers/virtualbox/disk.go b/drivers/virtualbox/disk.go index c1a207e402..83af1c9d46 100644 --- a/drivers/virtualbox/disk.go +++ b/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) - 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] + err = parseKeyValues(out, reEqualQuoteLine, func(key, val string) error { 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 } diff --git a/drivers/virtualbox/disk_test.go b/drivers/virtualbox/disk_test.go index bd43e3d936..7f52005639 100644 --- a/drivers/virtualbox/disk_test.go +++ b/drivers/virtualbox/disk_test.go @@ -7,22 +7,19 @@ import ( "github.com/stretchr/testify/assert" ) -const ( - validDiskInfoText = ` +const stdOutDiskInfo = ` 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) diff --git a/drivers/virtualbox/network.go b/drivers/virtualbox/network.go index 41524a6095..7c16a7e506 100644 --- a/drivers/virtualbox/network.go +++ b/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)) - for s.Scan() { - line := s.Text() - if line == "" { - continue - } - - res := reColonLine.FindStringSubmatch(line) - if res == nil { - continue - } - - switch key, val := res[1], res[2]; key { + err = parseKeyValues(out, reColonLine, func(key, val string) error { + switch 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() - if line == "" { - m[dhcp.NetworkName] = dhcp - dhcp = &dhcpServer{} - continue - } - res := reColonLine.FindStringSubmatch(line) - if res == nil { - continue - } - switch key, val := res[1], res[2]; key { + + err = parseKeyValues(out, reColonLine, func(key, val string) error { + switch 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 } diff --git a/drivers/virtualbox/network_test.go b/drivers/virtualbox/network_test.go index d40f2b6e97..736974fe48 100644 --- a/drivers/virtualbox/network_test.go +++ b/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) +} diff --git a/drivers/virtualbox/vbm.go b/drivers/virtualbox/vbm.go index 4b100c1d1b..3f05b3f72b 100644 --- a/drivers/virtualbox/vbm.go +++ b/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() +} diff --git a/drivers/virtualbox/vm.go b/drivers/virtualbox/vm.go index e72857c5bf..cd95ada767 100644 --- a/drivers/virtualbox/vm.go +++ b/drivers/virtualbox/vm.go @@ -1,11 +1,6 @@ package virtualbox -import ( - "bufio" - "io" - "strconv" - "strings" -) +import "strconv" 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() - if line == "" { - continue - } - res := reEqualLine.FindStringSubmatch(line) - if res == nil { - continue - } - switch key, val := res[1], res[2]; key { + + err = parseKeyValues(out, reEqualLine, func(key, val string) error { + switch 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 } diff --git a/drivers/virtualbox/vm_test.go b/drivers/virtualbox/vm_test.go index 5c9d86e362..ba732eef82 100644 --- a/drivers/virtualbox/vm_test.go +++ b/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 ( - testVMInfoText = ` +var stdOutVMInfo = ` 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) - vm, err := parseVMInfo(r) - if err != nil { - t.Fatal(err) + vbox := &VBoxManagerMock{ + args: "showvminfo host --machinereadable", + stdOut: stdOutVMInfo, } - vmCPUs := 2 - vmMemory := 1024 - if vm.CPUs != vmCPUs { - t.Fatalf("expected %d cpus; received %d", vmCPUs, vm.CPUs) - } + vm, err := getVMInfo("host", vbox) - if vm.Memory != vmMemory { - t.Fatalf("expected memory %d; received %d", vmMemory, vm.Memory) - } + assert.Equal(t, 2, vm.CPUs) + 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") }