services/device/device: introduce test for glob functionality

glob_test.go tests the glob logic directly, instead of relying on the behavior
of various subcommand tests.  To make testing the glob internal library easy,
exported a bunch of identifiers in glob.go (since it's all part of the main
package, there are no practical implications to exporting the
identifiers). While at it, made GlobResults be passed by pointer inside the
library for efficiency.

For now, the only test case verifies that all results appear as expected, and in
the expected order. I'll add more test cases in future cls.

Change-Id: Iea22ddde3db17be3a6e0a6840ef55d54591a9ff7
diff --git a/services/device/device/debug.go b/services/device/device/debug.go
index 1b06b3c..176b46e 100644
--- a/services/device/device/debug.go
+++ b/services/device/device/debug.go
@@ -24,10 +24,10 @@
 }
 
 func init() {
-	globify(cmdDebug, runDebug, new(globSettings))
+	globify(cmdDebug, runDebug, new(GlobSettings))
 }
 
-func runDebug(entry globResult, ctx *context.T, stdout, _ io.Writer) error {
+func runDebug(entry GlobResult, ctx *context.T, stdout, _ io.Writer) error {
 	if description, err := device.DeviceClient(entry.name).Debug(ctx); err != nil {
 		return fmt.Errorf("Debug failed: %v", err)
 	} else {
diff --git a/services/device/device/glob.go b/services/device/device/glob.go
index 523e3ec..2ad6a6a 100644
--- a/services/device/device/glob.go
+++ b/services/device/device/glob.go
@@ -28,7 +28,7 @@
 	"v.io/x/ref/lib/v23cmd"
 )
 
-// globHandler is implemented by each command that wants to execute against name
+// GlobHandler is implemented by each command that wants to execute against name
 // patterns.  The handler is expected to be invoked against each glob result,
 // and can be run concurrently. The handler should direct its output to the
 // given stdout and stderr writers.
@@ -38,7 +38,7 @@
 //
 // (1) Most control
 //
-// func myCmdHandler(entry globResult, ctx *context.T, stdout, stderr io.Writer) error {
+// func myCmdHandler(entry GlobResult, ctx *context.T, stdout, stderr io.Writer) error {
 //   output := myCmdProcessing(entry)
 //   fmt.Fprintf(stdout, output)
 //   ...
@@ -46,7 +46,7 @@
 //
 // func runMyCmd(ctx *context.T, env *cmdline.Env, args []string) error {
 //   ...
-//   err := run(ctx, env, args, myCmdHandler, globSettings{})
+//   err := Run(ctx, env, args, myCmdHandler, GlobSettings{})
 //   ...
 // }
 //
@@ -57,18 +57,18 @@
 //
 // (2) Pre-packaged runner
 //
-// If all runMyCmd does is to call run, you can use globRunner to avoid having
+// If all runMyCmd does is to call Run, you can use globRunner to avoid having
 // to define runMyCmd:
 //
 // var cmdMyCmd = &cmdline.Command {
-//   Runner: globRunner(myCmdHandler, &globSettings{}),
+//   Runner: globRunner(myCmdHandler, &GlobSettings{}),
 //   Name: "mycmd",
 //   ...
 // }
 //
 // (3) Pre-packaged runner and glob settings flag configuration
 //
-// If, additionally, you want the globSettings to be configurable with
+// If, additionally, you want the GlobSettings to be configurable with
 // command-line flags, you can use globify instead:
 //
 // var cmdMyCmd = &cmdline.Command {
@@ -77,13 +77,15 @@
 // }
 //
 // func init() {
-//   globify(cmdMyCmd, myCmdHandler, &globSettings{}),
+//   globify(cmdMyCmd, myCmdHandler, &GlobSettings{}),
 // }
-type globHandler func(entry globResult, ctx *context.T, stdout, stderr io.Writer) error
+//
+// The GlobHandler identifier is exported for use in unit tests.
+type GlobHandler func(entry GlobResult, ctx *context.T, stdout, stderr io.Writer) error
 
-func globRunner(handler globHandler, s *globSettings) cmdline.Runner {
+func globRunner(handler GlobHandler, s *GlobSettings) cmdline.Runner {
 	return v23cmd.RunnerFunc(func(ctx *context.T, env *cmdline.Env, args []string) error {
-		return run(ctx, env, args, handler, *s)
+		return Run(ctx, env, args, handler, *s)
 	})
 }
 
@@ -109,13 +111,34 @@
 	}
 }
 
-type globResult struct {
+// GlobResult defines the input to a GlobHandler.
+// The identifier is exported for use in unit tests.
+type GlobResult struct {
 	name   string
 	status device.Status
 	kind   objectKind
 }
 
-type byTypeAndName []globResult
+func NewGlobResult(name string, status device.Status) (*GlobResult, error) {
+	var kind objectKind
+	switch status.(type) {
+	case device.StatusInstallation:
+		kind = applicationInstallationObject
+	case device.StatusInstance:
+		kind = applicationInstanceObject
+	case device.StatusDevice:
+		kind = deviceServiceObject
+	default:
+		return nil, fmt.Errorf("Status(%v) returned unrecognized status type %T\n", name, status)
+	}
+	return &GlobResult{
+		name:   name,
+		status: status,
+		kind:   kind,
+	}, nil
+}
+
+type byTypeAndName []*GlobResult
 
 func (a byTypeAndName) Len() int      { return len(a) }
 func (a byTypeAndName) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
@@ -128,19 +151,20 @@
 	return r1.name < r2.name
 }
 
-// run runs the given handler in parallel against each of the results obtained
+// Run runs the given handler in parallel against each of the results obtained
 // by globbing args, after performing filtering based on type
 // (instance/installation) and state.  No de-duping of results is performed.
 // The outputs from each of the handlers are sorted: installations first, then
 // instances (and alphabetically by object name for each group).
-func run(ctx *context.T, env *cmdline.Env, args []string, handler globHandler, s globSettings) error {
+// The identifier is exported for use in unit tests.
+func Run(ctx *context.T, env *cmdline.Env, args []string, handler GlobHandler, s GlobSettings) error {
 	results := glob(ctx, env, args)
 	sort.Sort(byTypeAndName(results))
 	results = filterResults(results, s)
 	stdouts, stderrs := make([]bytes.Buffer, len(results)), make([]bytes.Buffer, len(results))
 	var errorCounter uint32 = 0
-	perResult := func(r globResult, index int) {
-		if err := handler(r, ctx, &stdouts[index], &stderrs[index]); err != nil {
+	perResult := func(r *GlobResult, index int) {
+		if err := handler(*r, ctx, &stdouts[index], &stderrs[index]); err != nil {
 			fmt.Fprintf(&stderrs[index], "ERROR for \"%s\": %v.\n", r.name, err)
 			atomic.AddUint32(&errorCounter, 1)
 		}
@@ -151,7 +175,7 @@
 		var wg sync.WaitGroup
 		for i, r := range results {
 			wg.Add(1)
-			go func(r globResult, i int) {
+			go func(r *GlobResult, i int) {
 				perResult(r, i)
 				wg.Done()
 			}(r, i)
@@ -171,7 +195,7 @@
 				}
 				wg.Add(1)
 				processed++
-				go func(r globResult, i int) {
+				go func(r *GlobResult, i int) {
 					perResult(r, i)
 					wg.Done()
 				}(r, i)
@@ -192,8 +216,8 @@
 	return nil
 }
 
-func filterResults(results []globResult, s globSettings) []globResult {
-	var ret []globResult
+func filterResults(results []*GlobResult, s GlobSettings) []*GlobResult {
+	var ret []*GlobResult
 	for _, r := range results {
 		switch status := r.status.(type) {
 		case device.StatusInstance:
@@ -216,7 +240,7 @@
 // put them under a __debug suffix (like it works for services).
 var debugNameRE = regexp.MustCompile("/apps/[^/]+/[^/]+/[^/]+/(logs|stats|pprof)(/|$)")
 
-func getStatus(ctx *context.T, env *cmdline.Env, name string, resultsCh chan<- globResult) {
+func getStatus(ctx *context.T, env *cmdline.Env, name string, resultsCh chan<- *GlobResult) {
 	status, err := device.DeviceClient(name).Status(ctx)
 	// Skip non-instances/installations.
 	if verror.ErrorID(err) == deviceimpl.ErrInvalidSuffix.ID {
@@ -226,22 +250,14 @@
 		fmt.Fprintf(env.Stderr, "Status(%v) failed: %v\n", name, err)
 		return
 	}
-	var kind objectKind
-	switch status.(type) {
-	case device.StatusInstallation:
-		kind = applicationInstallationObject
-	case device.StatusInstance:
-		kind = applicationInstanceObject
-	case device.StatusDevice:
-		kind = deviceServiceObject
-	default:
-		fmt.Fprintf(env.Stderr, "Status(%v) returned unrecognized status type %T\n", name, status)
-		return
+	if r, err := NewGlobResult(name, status); err != nil {
+		fmt.Fprintf(env.Stderr, "%v\n", err)
+	} else {
+		resultsCh <- r
 	}
-	resultsCh <- globResult{name, status, kind}
 }
 
-func globOne(ctx *context.T, env *cmdline.Env, pattern string, resultsCh chan<- globResult) {
+func globOne(ctx *context.T, env *cmdline.Env, pattern string, resultsCh chan<- *GlobResult) {
 	globCh, err := v23.GetNamespace(ctx).Glob(ctx, pattern)
 	if err != nil {
 		fmt.Fprintf(env.Stderr, "Glob(%v) failed: %v\n", pattern, err)
@@ -270,11 +286,11 @@
 }
 
 // glob globs the given arguments and returns the results in arbitrary order.
-func glob(ctx *context.T, env *cmdline.Env, args []string) []globResult {
+func glob(ctx *context.T, env *cmdline.Env, args []string) []*GlobResult {
 	ctx, cancel := context.WithTimeout(ctx, time.Minute)
 	defer cancel()
 	var wg sync.WaitGroup
-	resultsCh := make(chan globResult, 100)
+	resultsCh := make(chan *GlobResult, 100)
 	// For each pattern.
 	for _, a := range args {
 		wg.Add(1)
@@ -284,7 +300,7 @@
 		}(a)
 	}
 	// Collect the glob results into a slice.
-	var results []globResult
+	var results []*GlobResult
 	done := make(chan struct{})
 	go func() {
 		for r := range resultsCh {
@@ -405,27 +421,29 @@
 	return fmt.Errorf("unrecognized parallelism type: %v", s)
 }
 
-type globSettings struct {
+// GlobSettings specifies flag-settable options and filters for globbing.
+// The identifier is exported for use in unit tests.
+type GlobSettings struct {
 	instanceStateFilter     instanceStateFlag
 	installationStateFilter installationStateFlag
 	onlyInstances           bool
 	onlyInstallations       bool
 	handlerParallelism      parallelismFlag
-	defaults                *globSettings
+	defaults                *GlobSettings
 }
 
-func (s *globSettings) reset() {
+func (s *GlobSettings) reset() {
 	d := s.defaults
 	*s = *d
 	s.defaults = d
 }
 
-func (s *globSettings) setDefaults(d globSettings) {
-	s.defaults = new(globSettings)
+func (s *GlobSettings) setDefaults(d GlobSettings) {
+	s.defaults = new(GlobSettings)
 	*s.defaults = d
 }
 
-var allGlobSettings []*globSettings
+var allGlobSettings []*GlobSettings
 
 // ResetGlobSettings is meant for tests to restore the values of flag-configured
 // variables when running multiple commands in the same process.
@@ -435,7 +453,7 @@
 	}
 }
 
-func defineGlobFlags(fs *flag.FlagSet, s *globSettings) {
+func defineGlobFlags(fs *flag.FlagSet, s *GlobSettings) {
 	fs.Var(&s.instanceStateFilter, "instance-state", fmt.Sprintf("If non-empty, specifies allowed instance states (all other instances get filtered out). The value of the flag is a comma-separated list of values from among: %v.", device.InstanceStateAll))
 	fs.Var(&s.installationStateFilter, "installation-state", fmt.Sprintf("If non-empty, specifies allowed installation states (all others installations get filtered out). The value of the flag is a comma-separated list of values from among: %v.", device.InstallationStateAll))
 	fs.BoolVar(&s.onlyInstances, "only-instances", false, "If set, only consider instances.")
@@ -448,7 +466,7 @@
 	fs.Var(&s.handlerParallelism, "parallelism", fmt.Sprintf("Specifies the level of parallelism for the handler execution. One of: %v.", parallelismValues))
 }
 
-func globify(c *cmdline.Command, handler globHandler, s *globSettings) {
+func globify(c *cmdline.Command, handler GlobHandler, s *GlobSettings) {
 	s.setDefaults(*s)
 	defineGlobFlags(&c.Flags, s)
 	c.Runner = globRunner(handler, s)
diff --git a/services/device/device/glob_test.go b/services/device/device/glob_test.go
new file mode 100644
index 0000000..1a2a395
--- /dev/null
+++ b/services/device/device/glob_test.go
@@ -0,0 +1,126 @@
+// Copyright 2015 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main_test
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"strings"
+	"testing"
+
+	"v.io/v23/context"
+	"v.io/v23/naming"
+	"v.io/v23/services/device"
+
+	"v.io/x/lib/cmdline"
+	"v.io/x/ref/test"
+
+	cmd_device "v.io/x/ref/services/device/device"
+)
+
+func simplePrintHandler(entry cmd_device.GlobResult, ctx *context.T, stdout, stderr io.Writer) error {
+	fmt.Fprintf(stdout, "%v\n", entry)
+	return nil
+}
+
+func TestGlob(t *testing.T) {
+	ctx, shutdown := test.V23Init()
+	defer shutdown()
+	tapes := newTapeMap()
+	rootTape := tapes.forSuffix("")
+	server, endpoint, err := startServer(t, ctx, tapes)
+	if err != nil {
+		return
+	}
+	appName := naming.JoinAddressName(endpoint.String(), "app")
+	defer stopServer(t, server)
+
+	allGlobArgs := []string{"glob1", "glob2"}
+	allGlobResponses := [][]string{
+		[]string{"app/3", "app/4", "app/6", "app/5", "app/9", "app/7"},
+		[]string{"app/2", "app/1", "app/8"},
+	}
+	allStatusResponses := map[string][]interface{}{
+		"app/1": []interface{}{instanceRunning},
+		"app/2": []interface{}{installationUninstalled},
+		"app/3": []interface{}{instanceUpdating},
+		"app/4": []interface{}{installationActive},
+		"app/5": []interface{}{instanceNotRunning},
+		"app/6": []interface{}{deviceService},
+		"app/7": []interface{}{installationActive},
+		"app/8": []interface{}{deviceUpdating},
+		"app/9": []interface{}{instanceUpdating},
+	}
+	outLine := func(suffix string, s device.Status) string {
+		r, err := cmd_device.NewGlobResult(appName+"/"+suffix, s)
+		if err != nil {
+			t.Errorf("NewGlobResult failed: %v", err)
+			return ""
+		}
+		return fmt.Sprintf("%v", *r)
+	}
+	var (
+		app1Out = outLine("1", instanceRunning)
+		app2Out = outLine("2", installationUninstalled)
+		app3Out = outLine("3", instanceUpdating)
+		app4Out = outLine("4", installationActive)
+		app5Out = outLine("5", instanceNotRunning)
+		app6Out = outLine("6", deviceService)
+		app7Out = outLine("7", installationActive)
+		app8Out = outLine("8", deviceUpdating)
+		app9Out = outLine("9", instanceUpdating)
+	)
+
+	for _, c := range []struct {
+		handler         cmd_device.GlobHandler
+		globResponses   [][]string
+		statusResponses map[string][]interface{}
+		gs              cmd_device.GlobSettings
+		globPatterns    []string
+		expected        string
+	}{
+		{
+			simplePrintHandler,
+			allGlobResponses,
+			allStatusResponses,
+			cmd_device.GlobSettings{},
+			allGlobArgs,
+			// First installations, then instances, then device services.
+			joinLines(app2Out, app4Out, app7Out, app1Out, app3Out, app5Out, app9Out, app6Out, app8Out),
+		},
+		// TODO(caprita): Test the following cases:
+		// Various filters.
+		// Parallelism options.
+		// No glob arguments.
+		// No glob results.
+		// Error in glob.
+		// Error in status.
+		// Error in handler.
+	} {
+		tapes.rewind()
+		var rootTapeResponses []interface{}
+		for _, r := range c.globResponses {
+			rootTapeResponses = append(rootTapeResponses, GlobResponse{r})
+		}
+		rootTape.SetResponses(rootTapeResponses...)
+		for n, r := range c.statusResponses {
+			tapes.forSuffix(n).SetResponses(r...)
+		}
+		var stdout, stderr bytes.Buffer
+		env := &cmdline.Env{Stdout: &stdout, Stderr: &stderr}
+		var args []string
+		for _, p := range c.globPatterns {
+			args = append(args, naming.JoinAddressName(endpoint.String(), p))
+		}
+		err := cmd_device.Run(ctx, env, args, c.handler, c.gs)
+		if err != nil {
+			t.Errorf("%v", err)
+		}
+		if expected, got := c.expected, strings.TrimSpace(stdout.String()); got != expected {
+			t.Errorf("Unexpected output. Got:\n%v\nExpected:\n%v\n", got, expected)
+		}
+	}
+}
diff --git a/services/device/device/ls.go b/services/device/device/ls.go
index 9767abc..3ed7b7a 100644
--- a/services/device/device/ls.go
+++ b/services/device/device/ls.go
@@ -23,10 +23,10 @@
 }
 
 func init() {
-	globify(cmdLs, runLs, new(globSettings))
+	globify(cmdLs, runLs, new(GlobSettings))
 }
 
-func runLs(entry globResult, _ *context.T, stdout, _ io.Writer) error {
+func runLs(entry GlobResult, _ *context.T, stdout, _ io.Writer) error {
 	fmt.Fprintf(stdout, "%v\n", entry.name)
 	return nil
 }
diff --git a/services/device/device/ls_test.go b/services/device/device/ls_test.go
index 1db54f4..975239d 100644
--- a/services/device/device/ls_test.go
+++ b/services/device/device/ls_test.go
@@ -6,7 +6,6 @@
 
 import (
 	"bytes"
-	"fmt"
 	"strings"
 	"testing"
 
@@ -46,9 +45,6 @@
 		"app/5": []interface{}{instanceNotRunning},
 		"app/6": []interface{}{installationActive},
 	}
-	joinLines := func(args ...string) string {
-		return strings.Join(args, "\n")
-	}
 	for _, c := range []struct {
 		globResponses   [][]string
 		statusResponses map[string][]interface{}
@@ -152,7 +148,6 @@
 			args = append(args, naming.JoinAddressName(endpoint.String(), p))
 		}
 		if err := v23cmd.ParseAndRunForTest(cmd, ctx, env, args); err != nil {
-			fmt.Println("run test case error", err)
 			t.Errorf("%v", err)
 		}
 
diff --git a/services/device/device/status.go b/services/device/device/status.go
index 6123ec1..34a17d2 100644
--- a/services/device/device/status.go
+++ b/services/device/device/status.go
@@ -23,10 +23,10 @@
 }
 
 func init() {
-	globify(cmdStatus, runStatus, new(globSettings))
+	globify(cmdStatus, runStatus, new(GlobSettings))
 }
 
-func runStatus(entry globResult, _ *context.T, stdout, _ io.Writer) error {
+func runStatus(entry GlobResult, _ *context.T, stdout, _ io.Writer) error {
 	switch s := entry.status.(type) {
 	case device.StatusInstance:
 		fmt.Fprintf(stdout, "Instance %v [State:%v,Version:%v]\n", entry.name, s.Value.State, s.Value.Version)
diff --git a/services/device/device/update.go b/services/device/device/update.go
index 33d7ecd..0f83e23 100644
--- a/services/device/device/update.go
+++ b/services/device/device/update.go
@@ -29,7 +29,7 @@
 }
 
 func init() {
-	globify(cmdUpdate, runUpdate, &globSettings{handlerParallelism: kindParallelism})
+	globify(cmdUpdate, runUpdate, &GlobSettings{handlerParallelism: kindParallelism})
 }
 
 var cmdRevert = &cmdline.Command{
@@ -42,7 +42,7 @@
 }
 
 func init() {
-	globify(cmdRevert, runRevert, &globSettings{handlerParallelism: kindParallelism})
+	globify(cmdRevert, runRevert, &GlobSettings{handlerParallelism: kindParallelism})
 }
 
 func instanceIsRunning(ctx *context.T, von string) (bool, error) {
@@ -131,7 +131,7 @@
 	}
 }
 
-func changeVersion(entry globResult, ctx *context.T, stdout, stderr io.Writer, revert bool) error {
+func changeVersion(entry GlobResult, ctx *context.T, stdout, stderr io.Writer, revert bool) error {
 	switch entry.kind {
 	case applicationInstanceObject:
 		return changeVersionInstance(ctx, stdout, stderr, entry.name, entry.status.(device.StatusInstance), revert)
@@ -144,10 +144,10 @@
 	}
 }
 
-func runUpdate(entry globResult, ctx *context.T, stdout, stderr io.Writer) error {
+func runUpdate(entry GlobResult, ctx *context.T, stdout, stderr io.Writer) error {
 	return changeVersion(entry, ctx, stdout, stderr, false)
 }
 
-func runRevert(entry globResult, ctx *context.T, stdout, stderr io.Writer) error {
+func runRevert(entry GlobResult, ctx *context.T, stdout, stderr io.Writer) error {
 	return changeVersion(entry, ctx, stdout, stderr, true)
 }
diff --git a/services/device/device/util_test.go b/services/device/device/util_test.go
index 01bd228..c1e67c6 100644
--- a/services/device/device/util_test.go
+++ b/services/device/device/util_test.go
@@ -37,7 +37,7 @@
 	}}
 	instanceRunning = device.StatusInstance{device.InstanceStatus{
 		State:   device.InstanceStateRunning,
-		Version: "special edition",
+		Version: "tv version",
 	}}
 	instanceNotRunning = device.StatusInstance{device.InstanceStatus{
 		State:   device.InstanceStateNotRunning,
@@ -47,6 +47,10 @@
 		State:   device.InstanceStateRunning,
 		Version: "han shot first",
 	}}
+	deviceUpdating = device.StatusDevice{device.DeviceStatus{
+		State:   device.InstanceStateUpdating,
+		Version: "international release",
+	}}
 )
 
 func testHelper(t *testing.T, lower, upper string) {
@@ -113,3 +117,7 @@
 		t.Errorf("invalid call sequence. Got %v, want %v", got, expected)
 	}
 }
+
+func joinLines(args ...string) string {
+	return strings.Join(args, "\n")
+}