Use the passed VBoxManager

This makes the code easier to test.

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

drivers/virtualbox/disk.go

@@ -2,7 +2,6 @@ package virtualbox
 
 import (
 	"bufio"
-	"io"
 	"strings"
 )
 
@@ -11,28 +10,31 @@ type VirtualDisk struct {
 	Path string
 }
 
-func (d *Driver) getVMDiskInfo(name string) (*VirtualDisk, error) {
+func getVMDiskInfo(name string, vbox VBoxManager) (*VirtualDisk, error) {
-	out, err := d.vbmOut("showvminfo", name, "--machinereadable")
+	out, err := vbox.vbmOut("showvminfo", name, "--machinereadable")
 	if err != nil {
 		return nil, err
 	}
 
-	r := strings.NewReader(out)
+	return parseDiskInfo(out)
-	return parseDiskInfo(r)
 }
 
-func parseDiskInfo(r io.Reader) (*VirtualDisk, error) {
+func parseDiskInfo(out string) (*VirtualDisk, error) {
-	s := bufio.NewScanner(r)
 	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]
 		switch key {
 		case "SATA-1-0":
@@ -44,5 +46,6 @@ func parseDiskInfo(r io.Reader) (*VirtualDisk, error) {
 	if err := s.Err(); err != nil {
 		return nil, err
 	}
+
 	return disk, nil
 }

drivers/virtualbox/disk_test.go

@@ -1,8 +1,10 @@
 package virtualbox
 
 import (
-	"strings"
 	"testing"
+
+	"github.com/docker/machine/drivers/vmwarevsphere/errors"
+	"github.com/stretchr/testify/assert"
 )
 
 var (
@@ -18,19 +20,39 @@ nic1="nat"
 )
 
 func TestVMDiskInfo(t *testing.T) {
-	r := strings.NewReader(testDiskInfoText)
+	vbox := &VBoxManagerMock{
-	disk, err := parseDiskInfo(r)
+		args:   "showvminfo default --machinereadable",
-	if err != nil {
+		stdOut: testDiskInfoText,
-		t.Fatal(err)
 	}
 
-	diskPath := "/home/ehazlett/vm/test/disk.vmdk"
+	disk, err := getVMDiskInfo("default", vbox)
-	diskUUID := "12345-abcdefg"
-	if disk.Path != diskPath {
-		t.Fatalf("expected disk path %s", diskPath)
-	}
 
-	if disk.UUID != diskUUID {
+	assert.Equal(t, "/home/ehazlett/vm/test/disk.vmdk", disk.Path)
-		t.Fatalf("expected disk uuid %s", diskUUID)
+	assert.Equal(t, "12345-abcdefg", disk.UUID)
-	}
+	assert.NoError(t, err)
+}
+
+func TestVMDiskInfoError(t *testing.T) {
+	vbox := &VBoxManagerMock{
+		args: "showvminfo default --machinereadable",
+		err:  errors.New("BUG"),
+	}
+
+	disk, err := getVMDiskInfo("default", vbox)
+
+	assert.Nil(t, disk)
+	assert.EqualError(t, err, "BUG")
+}
+
+func TestVMDiskInfoInvalidOutput(t *testing.T) {
+	vbox := &VBoxManagerMock{
+		args:   "showvminfo default --machinereadable",
+		stdOut: "INVALID",
+	}
+
+	disk, err := getVMDiskInfo("default", vbox)
+
+	assert.Empty(t, disk.Path)
+	assert.Empty(t, disk.UUID)
+	assert.NoError(t, err)
 }

drivers/virtualbox/network.go

@@ -31,61 +31,50 @@ type hostOnlyNetwork struct {
 	NetworkName string // referenced in DHCP.NetworkName
 }
 
-// TODO: use VBoxManager.vbm() instead
-func vbm(args ...string) error {
-	vBoxManager := &VBoxCmdManager{}
-	_, _, err := vBoxManager.vbmOutErr(args...)
-	return err
-}
-
-// TODO: use VBoxManager.vbmOut() instead
-func vbmOut(args ...string) (string, error) {
-	vBoxManager := &VBoxCmdManager{}
-	stdout, _, err := vBoxManager.vbmOutErr(args...)
-	return stdout, err
-}
-
 // Save changes the configuration of the host-only network.
-func (n *hostOnlyNetwork) Save() error {
+func (n *hostOnlyNetwork) Save(vbox VBoxManager) error {
 	if n.IPv4.IP != nil && n.IPv4.Mask != nil {
-		if err := vbm("hostonlyif", "ipconfig", n.Name, "--ip", n.IPv4.IP.String(), "--netmask", net.IP(n.IPv4.Mask).String()); err != nil {
+		if err := vbox.vbm("hostonlyif", "ipconfig", n.Name, "--ip", n.IPv4.IP.String(), "--netmask", net.IP(n.IPv4.Mask).String()); err != nil {
 			return err
 		}
 	}
 
 	if n.IPv6.IP != nil && n.IPv6.Mask != nil {
 		prefixLen, _ := n.IPv6.Mask.Size()
-		if err := vbm("hostonlyif", "ipconfig", n.Name, "--ipv6", n.IPv6.IP.String(), "--netmasklengthv6", fmt.Sprintf("%d", prefixLen)); err != nil {
+		if err := vbox.vbm("hostonlyif", "ipconfig", n.Name, "--ipv6", n.IPv6.IP.String(), "--netmasklengthv6", fmt.Sprintf("%d", prefixLen)); err != nil {
 			return err
 		}
 	}
 
 	if n.DHCP {
-		vbm("hostonlyif", "ipconfig", n.Name, "--dhcp") // not implemented as of VirtualBox 4.3
+		vbox.vbm("hostonlyif", "ipconfig", n.Name, "--dhcp") // not implemented as of VirtualBox 4.3
 	}
 
 	return nil
 }
 
 // createHostonlyNet creates a new host-only network.
-func createHostonlyNet() (*hostOnlyNetwork, error) {
+func createHostonlyNet(vbox VBoxManager) (*hostOnlyNetwork, error) {
-	out, err := vbmOut("hostonlyif", "create")
+	out, err := vbox.vbmOut("hostonlyif", "create")
 	if err != nil {
 		return nil, err
 	}
+
 	res := reHostonlyInterfaceCreated.FindStringSubmatch(string(out))
 	if res == nil {
 		return nil, errors.New("failed to create hostonly interface")
 	}
+
 	return &hostOnlyNetwork{Name: res[1]}, nil
 }
 
 // listHostOnlyNetworks gets all host-only networks in a  map keyed by HostonlyNet.NetworkName.
-func listHostOnlyNetworks() (map[string]*hostOnlyNetwork, error) {
+func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error) {
-	out, err := vbmOut("list", "hostonlyifs")
+	out, err := vbox.vbmOut("list", "hostonlyifs")
 	if err != nil {
 		return nil, err
 	}
+
 	s := bufio.NewScanner(strings.NewReader(out))
 	m := map[string]*hostOnlyNetwork{}
 	n := &hostOnlyNetwork{}
@@ -153,8 +142,8 @@ func getHostOnlyNetwork(nets map[string]*hostOnlyNetwork, hostIP net.IP, netmask
 	return nil
 }
 
-func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP, dhcpUpperIP net.IP, dhcpLowerIP net.IP) (*hostOnlyNetwork, error) {
+func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP, dhcpUpperIP net.IP, dhcpLowerIP net.IP, vbox VBoxManager) (*hostOnlyNetwork, error) {
-	nets, err := listHostOnlyNetworks()
+	nets, err := listHostOnlyNetworks(vbox)
 	if err != nil {
 		return nil, err
 	}
@@ -163,13 +152,13 @@ func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP
 
 	if hostOnlyNet == nil {
 		// No existing host-only interface found. Create a new one.
-		hostOnlyNet, err = createHostonlyNet()
+		hostOnlyNet, err = createHostonlyNet(vbox)
 		if err != nil {
 			return nil, err
 		}
 		hostOnlyNet.IPv4.IP = hostIP
 		hostOnlyNet.IPv4.Mask = netmask
-		if err := hostOnlyNet.Save(); err != nil {
+		if err := hostOnlyNet.Save(vbox); err != nil {
 			return nil, err
 		}
 
@@ -179,7 +168,7 @@ func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP
 		dhcp.LowerIP = dhcpUpperIP
 		dhcp.UpperIP = dhcpLowerIP
 		dhcp.Enabled = true
-		if err := addHostonlyDHCP(hostOnlyNet.Name, dhcp); err != nil {
+		if err := addHostonlyDHCP(hostOnlyNet.Name, dhcp, vbox); err != nil {
 			return nil, err
 		}
 	}
@@ -196,12 +185,12 @@ type dhcpServer struct {
 	Enabled     bool
 }
 
-func addDHCPServer(kind, name string, d dhcpServer) error {
+func addDHCPServer(kind, name string, d dhcpServer, vbox VBoxManager) error {
 	command := "modify"
 
 	// On some platforms (OSX), creating a hostonlyinterface adds a default dhcpserver
 	// While on others (Windows?) it does not.
-	dhcps, err := getDHCPServers()
+	dhcps, err := getDHCPServers(vbox)
 	if err != nil {
 		return err
 	}
@@ -222,17 +211,18 @@ func addDHCPServer(kind, name string, d dhcpServer) error {
 	} else {
 		args = append(args, "--disable")
 	}
-	return vbm(args...)
+
+	return vbox.vbm(args...)
 }
 
 // addHostonlyDHCP adds a DHCP server to a host-only network.
-func addHostonlyDHCP(ifname string, d dhcpServer) error {
+func addHostonlyDHCP(ifname string, d dhcpServer, vbox VBoxManager) error {
-	return addDHCPServer("--netname", "HostInterfaceNetworking-"+ifname, d)
+	return addDHCPServer("--netname", "HostInterfaceNetworking-"+ifname, d, vbox)
 }
 
 // getDHCPServers gets all DHCP server settings in a map keyed by DHCP.NetworkName.
-func getDHCPServers() (map[string]*dhcpServer, error) {
+func getDHCPServers(vbox VBoxManager) (map[string]*dhcpServer, error) {
-	out, err := vbmOut("list", "dhcpservers")
+	out, err := vbox.vbmOut("list", "dhcpservers")
 	if err != nil {
 		return nil, err
 	}

drivers/virtualbox/virtualbox.go

@@ -239,7 +239,7 @@ func (d *Driver) Create() error {
 		// make sure vm is stopped
 		_ = d.vbm("controlvm", name, "poweroff")
 
-		diskInfo, err := d.getVMDiskInfo(name)
+		diskInfo, err := getVMDiskInfo(name, d.VBoxManager)
 		if err != nil {
 			return err
 		}
@@ -253,7 +253,7 @@ func (d *Driver) Create() error {
 		}
 
 		log.Debugf("Importing VM settings...")
-		vmInfo, err := d.getVMInfo(name)
+		vmInfo, err := getVMInfo(name, d.VBoxManager)
 		if err != nil {
 			return err
 		}
@@ -681,6 +681,7 @@ func (d *Driver) setupHostOnlyNetwork(machineName string) error {
 		dhcpAddr,
 		lowerDHCPIP,
 		upperDHCPIP,
+		d.VBoxManager,
 	)
 	if err != nil {
 		return err

drivers/virtualbox/virtualbox_test.go

@@ -31,13 +31,22 @@ func TestDefaultSSHUsername(t *testing.T) {
 }
 
 type VBoxManagerMock struct {
-	VBoxCmdManager
 	args   string
 	stdOut string
 	stdErr string
 	err    error
 }
 
+func (v *VBoxManagerMock) vbm(args ...string) error {
+	_, _, err := v.vbmOutErr(args...)
+	return err
+}
+
+func (v *VBoxManagerMock) vbmOut(args ...string) (string, error) {
+	stdout, _, err := v.vbmOutErr(args...)
+	return stdout, err
+}
+
 func (v *VBoxManagerMock) vbmOutErr(args ...string) (string, string, error) {
 	if strings.Join(args, " ") == v.args {
 		return v.stdOut, v.stdErr, v.err

drivers/virtualbox/vm.go

@@ -12,8 +12,8 @@ type VM struct {
 	Memory int
 }
 
-func (d *Driver) getVMInfo(name string) (*VM, error) {
+func getVMInfo(name string, vbox VBoxManager) (*VM, error) {
-	out, err := d.vbmOut("showvminfo", name, "--machinereadable")
+	out, err := vbox.vbmOut("showvminfo", name, "--machinereadable")
 	if err != nil {
 		return nil, err
 	}