Bootdevice testing (#3)

Start step_first_boot_device_test and rejigger ParseBootDeviceIdentifier to avoid regex where reasonable.

builder/hyperv/common/step_set_first_boot_device.go

@@ -18,66 +18,86 @@ type StepSetFirstBootDevice struct {
 
 func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string, uint, uint, error) {
 
-	captureExpression := "^(FLOPPY|IDE|NET)|(CD|DVD)$"
-	if generation > 1 {
-		captureExpression = "^((IDE|SCSI):(\\d+):(\\d+))|(DVD|CD)|(NET)$"
-	}
+	// all input strings are forced to upperCase for comparison, I believe this is
+	// safe as all of our values are 7bit ASCII clean.
 
-	r, err := regexp.Compile(captureExpression)
-	if err != nil {
-		return "", 0, 0, err
+	lookupDeviceIdentifier := strings.ToUpper(deviceIdentifier)
+
+	if (generation == 1) {
+
+		// Gen1 values are a simple set of if/then/else values, which we coalesce into a map
+		// here for simplicity
+
+		lookupTable := map[string]string {
+			"FLOPPY": "FLOPPY",
+			"IDE": "IDE",
+			"NET": "NET",
+			"CD": "CD",
+			"DVD": "CD",
+		}
+
+		controllerType, isDefined := lookupTable[lookupDeviceIdentifier]
+		if (!isDefined) {
+
+			return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device group identifier.", deviceIdentifier)
+
+		}
+
+		// success
+		return controllerType, 0, 0, nil
 	}
 
-	// match against the appropriate set of values.. we force to uppercase to ensure that
-	// all devices are always in the same case
+	// everything else is treated as generation 2... the first set of lookups covers
+	// the simple options..
 
-	identifierMatches := r.FindStringSubmatch(strings.ToUpper(deviceIdentifier))
-	if identifierMatches == nil {
-		return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device or device group identifier.", deviceIdentifier)
+	lookupTable := map[string]string {
+		"CD": "CD",
+		"DVD": "CD",
+		"NET": "NET",
 	}
 
-	switch {
+	controllerType, isDefined := lookupTable[lookupDeviceIdentifier]
+	if (isDefined) {
 
-	// CD or DVD are always returned as "CD"
-	case ((generation == 1) && (identifierMatches[2] != "")) || ((generation > 1) && (identifierMatches[5] != "")):
-		return "CD", 0, 0, nil
+		// these types do not require controllerNumber or controllerLocation
+		return controllerType, 0, 0, nil
 
-	// generation 1 only has FLOPPY, IDE or NET remaining..
-	case (generation == 1):
-		return identifierMatches[0], 0, 0, nil
+	}
 
-	// generation 2, check for IDE or SCSI and parse location and number
-	case (identifierMatches[2] != ""):
-		{
+	// not a simple option, check for a controllerType:controllerNumber:controllerLocation formatted
+	// device..
 
-			var controllerLocation int64
-			var controllerNumber int64
+	r, err := regexp.Compile("^(IDE|SCSI):(\\d+):(\\d+)$")
+	if err != nil {
+		return "", 0, 0, err
+	}
 
-			// NOTE: controllerNumber and controllerLocation cannot be negative, the regex expression
-			// would not have matched if either number was signed
+	controllerMatch := r.FindStringSubmatch(lookupDeviceIdentifier)
+	if controllerMatch != nil {
 
-			controllerNumber, err = strconv.ParseInt(identifierMatches[3], 10, 8)
-			if err == nil {
+		var controllerLocation int64
+		var controllerNumber int64
 
-				controllerLocation, err = strconv.ParseInt(identifierMatches[4], 10, 8)
-				if err == nil {
+		// NOTE: controllerNumber and controllerLocation cannot be negative, the regex expression
+		// would not have matched if either number was signed
 
-					return identifierMatches[2], uint(controllerNumber), uint(controllerLocation), nil
+		controllerNumber, err = strconv.ParseInt(controllerMatch[2], 10, 8)
+		if err == nil {
 
-				}
+			controllerLocation, err = strconv.ParseInt(controllerMatch[3], 10, 8)
+			if err == nil {
 
-			}
+				return controllerMatch[1], uint(controllerNumber), uint(controllerLocation), nil
 
-			return "", 0, 0, err
+			}
 
 		}
 
-	// only "NET" left on generation 2
-	default:
-		return "NET", 0, 0, nil
+		return "", 0, 0, err
 
 	}
 
+	return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device identifier.", deviceIdentifier)
 }
 
 func (s *StepSetFirstBootDevice) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {

builder/hyperv/common/step_set_first_boot_device_test.go

@@ -0,0 +1,74 @@
+package common
+
+import (
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestStepSetFirstBootDevice_impl(t *testing.T) {
+	var _ multistep.Step = new(StepSetFirstBootDevice)
+}
+
+func TestStepSetFirstBootDevice(t *testing.T) {
+//	t.Fatal("Fail IT!")
+}
+
+type parseBootDeviceIdentifierTest struct {
+	generation         uint
+	deviceIdentifier   string
+	controllerType     string
+	controllerNumber   uint
+	controllerLocation uint
+	shouldError        bool
+}
+
+func TestStepSetFirstBootDevice_ParseIdentifier(t *testing.T) {
+
+	identifierTests := [...]parseBootDeviceIdentifierTest{
+		{1, "IDE", "IDE", 0, 0, false},
+		{1, "idE", "IDE", 0, 0, false},
+		{1, "CD", "CD", 0, 0, false},
+		{1, "cD", "CD", 0, 0, false},
+		{1, "DVD", "CD", 0, 0, false},
+		{1, "Dvd", "CD", 0, 0, false},
+		{1, "FLOPPY", "FLOPPY", 0, 0, false},
+		{1, "FloppY", "FLOPPY", 0, 0, false},
+		{1, "NET", "NET", 0, 0, false},
+		{1, "net", "NET", 0, 0, false},
+		{1, "", "", 0, 0, true},
+		{1, "bad", "", 0, 0, true},
+		{1, "IDE:0:0", "", 0, 0, true},
+		{1, "SCSI:0:0", "", 0, 0, true},
+	}
+
+	for _, identifierTest := range identifierTests {
+
+		controllerType, controllerNumber, controllerLocation, err := ParseBootDeviceIdentifier(
+			identifierTest.deviceIdentifier,
+			identifierTest.generation)
+
+		if (err != nil) != identifierTest.shouldError {
+
+			t.Fatalf("Test %q (gen %v): shouldError: %v but err: %v", identifierTest.deviceIdentifier, 
+				identifierTest.generation, identifierTest.shouldError, err)
+			
+		}
+
+		switch {
+
+		case controllerType != identifierTest.controllerType:
+			t.Fatalf("Test %q (gen %v): controllerType: %q != %q", identifierTest.deviceIdentifier, identifierTest.generation,
+				identifierTest.controllerType, controllerType)
+
+		case controllerNumber != identifierTest.controllerNumber:
+			t.Fatalf("Test %q (gen %v): controllerNumber: %v != %v", identifierTest.deviceIdentifier, identifierTest.generation,
+				identifierTest.controllerNumber, controllerNumber)
+
+		case controllerLocation != identifierTest.controllerLocation:
+			t.Fatalf("Test %q (gen %v): controllerLocation: %v != %v", identifierTest.deviceIdentifier, identifierTest.generation,
+				identifierTest.controllerLocation, controllerLocation)
+
+		}
+	}
+}