Review feedback

common/exec/exec.go

@@ -0,0 +1,28 @@
+package exec
+
+import (
+	"io"
+	"os"
+	"os/exec"
+)
+
+// Cmd is a hook for mocking
+type Cmd interface {
+	StdoutPipe() (io.ReadCloser, error)
+	Start() error
+	Wait() error
+	Process() *os.Process
+}
+
+// Command is a hook for mocking
+var Command = func(name string, args ...string) Cmd {
+	return &realCmd{exec.Command(name, args...)}
+}
+
+type realCmd struct {
+	*exec.Cmd
+}
+
+func (c *realCmd) Process() *os.Process {
+	return c.Cmd.Process
+}

probe/endpoint/conntrack.go

@@ -9,7 +9,7 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/weaveworks/scope/test/exec"
+	"github.com/weaveworks/scope/common/exec"
 )
 
 // Constants exported for testing

probe/endpoint/conntrack_test.go

@@ -7,9 +7,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/weaveworks/scope/common/exec"
 	. "github.com/weaveworks/scope/probe/endpoint"
 	"github.com/weaveworks/scope/test"
-	"github.com/weaveworks/scope/test/exec"
+	testExec "github.com/weaveworks/scope/test/exec"
 )
 
 func makeFlow(id int64, srcIP, dstIP string, srcPort, dstPort int, ty, state string) Flow {
@@ -72,7 +73,7 @@ func TestConntracker(t *testing.T) {
 
 	reader, writer := io.Pipe()
 	exec.Command = func(name string, args ...string) exec.Cmd {
-		return exec.NewMockCmd(reader)
+		return testExec.NewMockCmd(reader)
 	}
 
 	conntracker, err := NewConntracker()

probe/overlay/weave.go

@@ -11,9 +11,9 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/weaveworks/scope/common/exec"
 	"github.com/weaveworks/scope/probe/docker"
 	"github.com/weaveworks/scope/report"
-	"github.com/weaveworks/scope/test/exec"
 )
 
 const (

probe/overlay/weave_test.go

@@ -7,18 +7,19 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/weaveworks/scope/common/exec"
 	"github.com/weaveworks/scope/probe/docker"
 	"github.com/weaveworks/scope/probe/overlay"
 	"github.com/weaveworks/scope/report"
 	"github.com/weaveworks/scope/test"
-	"github.com/weaveworks/scope/test/exec"
+	testExec "github.com/weaveworks/scope/test/exec"
 )
 
 func TestWeaveTaggerOverlayTopology(t *testing.T) {
 	oldExecCmd := exec.Command
 	defer func() { exec.Command = oldExecCmd }()
 	exec.Command = func(name string, args ...string) exec.Cmd {
-		return exec.NewMockCmdString(fmt.Sprintf("%s %s %s/24\n", mockContainerID, mockContainerMAC, mockContainerIP))
+		return testExec.NewMockCmdString(fmt.Sprintf("%s %s %s/24\n", mockContainerID, mockContainerMAC, mockContainerIP))
 	}
 
 	s := httptest.NewServer(http.HandlerFunc(mockWeaveRouter))

render/mapping.go

@@ -25,12 +25,12 @@ const (
 )
 
 // LeafMapFunc is anything which can take an arbitrary NodeMetadata, which is
-// always one-to-one with nodes in a topology, and return a specific
-// representation of the referenced node, in the form of a node ID and a
-// human-readable major and minor labels.
+// always one-to-one with nodes in a topology, and return a set of RenderableNodes
+// - specific representations of the referenced node, in the form of a map of node
+// ID to a human-readable major and minor labels.
 //
 // A single NodeMetadata can yield arbitrary many representations, including
-// representations that reduce the cardinality of the set of nodes.
+// representations that reduce (or even increase) the cardinality of the set of nodes.
 type LeafMapFunc func(report.NodeMetadata) RenderableNodes
 
 // PseudoFunc creates RenderableNode representing pseudo nodes given the
@@ -41,13 +41,13 @@ type LeafMapFunc func(report.NodeMetadata) RenderableNodes
 type PseudoFunc func(srcNodeID, dstNodeID string, srcIsClient bool, local report.Networks) (RenderableNode, bool)
 
 // MapFunc is anything which can take an arbitrary RenderableNode and
-// return another RenderableNode.
+// return a set of other RenderableNodes.
 //
-// As with LeafMapFunc, if the final output parameter is false, the node
+// As with LeafMapFunc, if the output is empty, the node
 // shall be omitted from the rendered topology.
 type MapFunc func(RenderableNode) RenderableNodes
 
-// MapEndpointIdentity maps an endpoint topology node to an endpoint
+// MapEndpointIdentity maps an endpoint topology node to a single endpoint
 // renderable node. As it is only ever run on endpoint topology nodes, we
 // expect that certain keys are present.
 func MapEndpointIdentity(m report.NodeMetadata) RenderableNodes {

test/exec/exec.go

@@ -5,36 +5,16 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"os/exec"
+
+	"github.com/weaveworks/scope/common/exec"
 )
 
-// Cmd is a hook for mocking
-type Cmd interface {
-	StdoutPipe() (io.ReadCloser, error)
-	Start() error
-	Wait() error
-	Process() *os.Process
-}
-
-// Command is a hook for mocking
-var Command = func(name string, args ...string) Cmd {
-	return &realCmd{exec.Command(name, args...)}
-}
-
-type realCmd struct {
-	*exec.Cmd
-}
-
-func (c *realCmd) Process() *os.Process {
-	return c.Cmd.Process
-}
-
 type mockCmd struct {
 	io.ReadCloser
 }
 
 // NewMockCmdString creates a new mock Cmd which has s on its stdout pipe
-func NewMockCmdString(s string) Cmd {
+func NewMockCmdString(s string) exec.Cmd {
 	return &mockCmd{
 		struct {
 			io.Reader
@@ -47,7 +27,7 @@ func NewMockCmdString(s string) Cmd {
 }
 
 // NewMockCmd creates a new mock Cmd with rc as its stdout pipe
-func NewMockCmd(rc io.ReadCloser) Cmd {
+func NewMockCmd(rc io.ReadCloser) exec.Cmd {
 	return &mockCmd{rc}
 }