packer/plugin: More robust command exit detection + tests

packer/plugin/command.go

@@ -36,21 +36,30 @@ func Command(cmd *exec.Cmd) (result packer.Command, err error) {
 		select {
 		case <-cmdExited:
 			err = errors.New("plugin exited before we could connect")
-			return
-		case <-time.After(10 * time.Millisecond):
-			if line, err := out.ReadBytes('\n'); err == nil {
-				address = strings.TrimSpace(string(line))
-				done = true
-			}
+			done = true
+		default:
+		}
 
-			// Make sure to reset err to nil
+		if line, lerr := out.ReadBytes('\n'); lerr == nil {
+			// Trim the address and reset the err since we were able
+			// to read some sort of address.
+			address = strings.TrimSpace(string(line))
 			err = nil
+			break
 		}
+
+		// If error is nil from previously, return now
+		if err != nil {
+			return
+		}
+
+		// Wait a bit
+		time.Sleep(10 * time.Millisecond)
 	}
 
 	client, err := rpc.Dial("tcp", address)
 	if err != nil {
-		panic(err)
+		return
 	}
 
 	result = packrpc.Command(client)

packer/plugin/command_test.go

@@ -28,4 +28,13 @@ func TestCommand_CommandExited(t *testing.T) {
 
 	_, err := Command(helperProcess("im-a-command-that-doesnt-work"))
 	assert.NotNil(err, "should have an error")
+	assert.Equal(err.Error(), "plugin exited before we could connect", "be correct error")
+}
+
+func TestCommand_BadRPC(t *testing.T) {
+	assert := asserts.NewTestingAsserts(t, true)
+
+	_, err := Command(helperProcess("invalid-rpc-address"))
+	assert.NotNil(err, "should have an error")
+	assert.Equal(err.Error(), "missing port in address lolinvalid", "be correct error")
 }

packer/plugin/plugin_test.go

@@ -40,6 +40,8 @@ func TestHelperProcess(*testing.T) {
 		return
 	}
 
+	defer os.Exit(0)
+
 	args := os.Args
 	for len(args) > 0 {
 		if args[0] == "--" {
@@ -59,6 +61,8 @@ func TestHelperProcess(*testing.T) {
 	switch cmd {
 	case "command":
 		ServeCommand(new(helperCommand))
+	case "invalid-rpc-address":
+		fmt.Println("lolinvalid")
 	case "start-timeout":
 		time.Sleep(1 * time.Minute)
 		os.Exit(1)