Clarify error messages in interactive for groups (#294)

internal/driver/interactive.go

@@ -42,6 +42,9 @@ func interactive(p *profile.Profile, o *plugin.Options) error {
 	interactiveMode = true
 	shortcuts := profileShortcuts(p)
 
+	// Get all groups in pprofVariables to allow for clearer error messages.
+	groups := groupOptions(pprofVariables)
+
 	greetings(p, o.UI)
 	for {
 		input, err := o.UI.ReadLine("(pprof) ")
@@ -87,6 +90,9 @@ func interactive(p *profile.Profile, o *plugin.Options) error {
 						o.UI.PrintErr(err)
 					}
 					continue
+				} else if okValues := groups[name]; okValues != nil {
+					o.UI.PrintErr(fmt.Errorf("Unrecognized value for %s: %q. Use one of %s", name, value, strings.Join(okValues, ", ")))
+					continue
 				}
 			}
 
@@ -118,6 +124,23 @@ func interactive(p *profile.Profile, o *plugin.Options) error {
 	}
 }
 
+// groupOptions returns a map containing all non-empty groups
+// mapped to an array of the option names in that group in
+// sorted order.
+func groupOptions(vars variables) map[string][]string {
+	groups := make(map[string][]string)
+	for name, option := range vars {
+		group := option.group
+		if group != "" {
+			groups[group] = append(groups[group], name)
+		}
+	}
+	for _, names := range groups {
+		sort.Strings(names)
+	}
+	return groups
+}
+
 var generateReportWrapper = generateReport // For testing purposes.
 
 // greetings prints a brief welcome and some overall profile

internal/driver/interactive_test.go

@@ -16,12 +16,12 @@ package driver
 
 import (
 	"fmt"
-	"io"
 	"math/rand"
 	"strings"
 	"testing"
 
 	"github.com/google/pprof/internal/plugin"
+	"github.com/google/pprof/internal/proftest"
 	"github.com/google/pprof/internal/report"
 	"github.com/google/pprof/profile"
 )
@@ -70,6 +70,21 @@ func TestShell(t *testing.T) {
 		t.Error("second shortcut attempt:", err)
 	}
 
+	// Group with invalid value
+	pprofVariables = testVariables(savedVariables)
+	ui := &proftest.TestUI{
+		T:       t,
+		Input:   []string{"cumulative=this"},
+		AllowRx: `Unrecognized value for cumulative: "this". Use one of cum, flat`,
+	}
+	o.UI = ui
+	if err := interactive(p, o); err != nil {
+		t.Error("invalid group value:", err)
+	}
+	// Confirm error message written out once.
+	if ui.NumAllowRxMatches != 1 {
+		t.Errorf("want error message to be printed 1 time, got %v", ui.NumAllowRxMatches)
+	}
 	// Verify propagation of IO errors
 	pprofVariables = testVariables(savedVariables)
 	o.UI = newUI(t, []string{"**error**"})
@@ -144,45 +159,10 @@ func makeShortcuts(input []string, seed int) (shortcuts, []string) {
 }
 
 func newUI(t *testing.T, input []string) plugin.UI {
-	return &testUI{
-		t:     t,
-		input: input,
-	}
-}
-
-type testUI struct {
-	t     *testing.T
-	input []string
-	index int
-}
-
-func (ui *testUI) ReadLine(_ string) (string, error) {
-	if ui.index >= len(ui.input) {
-		return "", io.EOF
+	return &proftest.TestUI{
+		T:     t,
+		Input: input,
 	}
-	input := ui.input[ui.index]
-	if input == "**error**" {
-		return "", fmt.Errorf("Error: %s", input)
-	}
-	ui.index++
-	return input, nil
-}
-
-func (ui *testUI) Print(args ...interface{}) {
-}
-
-func (ui *testUI) PrintErr(args ...interface{}) {
-	output := fmt.Sprint(args)
-	if output != "" {
-		ui.t.Error(output)
-	}
-}
-
-func (ui *testUI) IsTerminal() bool {
-	return false
-}
-
-func (ui *testUI) SetAutoComplete(func(string) string) {
 }
 
 func checkValue(p *profile.Profile, cmd []string, vars variables, o *plugin.Options) error {

internal/proftest/proftest.go

@@ -19,6 +19,7 @@ package proftest
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -80,11 +81,21 @@ type TestUI struct {
 	Ignore            int
 	AllowRx           string
 	NumAllowRxMatches int
+	Input             []string
+	index             int
 }
 
 // ReadLine returns no input, as no input is expected during testing.
 func (ui *TestUI) ReadLine(_ string) (string, error) {
-	return "", fmt.Errorf("no input")
+	if ui.index >= len(ui.Input) {
+		return "", io.EOF
+	}
+	input := ui.Input[ui.index]
+	ui.index++
+	if input == "**error**" {
+		return "", fmt.Errorf("Error: %s", input)
+	}
+	return input, nil
 }
 
 // Print messages are discarded by the test UI.