render: fixes for new merge semantics

render/detailed_node.go

@@ -109,9 +109,11 @@ func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode {
 	}
 }
 
-func getRenderingContext(r report.Report, n RenderableNode) (multiContainer bool, multiHost bool) {
-	originHosts := make(map[string]struct{})
-	originContainers := make(map[string]struct{})
+func getRenderingContext(r report.Report, n RenderableNode) (multiContainer, multiHost bool) {
+	var (
+		originHosts      = map[string]struct{}{}
+		originContainers = map[string]struct{}{}
+	)
 	for _, id := range n.Origins {
 		for _, topology := range r.Topologies() {
 			if nmd, ok := topology.NodeMetadatas[id]; ok {

render/expected/expected.go

@@ -186,6 +186,7 @@ var (
 			LabelMinor: "1 process",
 			Rank:       "bash",
 			Pseudo:     false,
+			Adjacency:  report.MakeIDList(),
 			Origins: report.MakeIDList(
 				test.NonContainerProcessNodeID,
 				test.ServerHostNodeID,
@@ -245,6 +246,7 @@ var (
 			LabelMinor: test.ServerHostName,
 			Rank:       "",
 			Pseudo:     true,
+			Adjacency:  report.MakeIDList(),
 			Origins: report.MakeIDList(
 				test.NonContainerProcessNodeID,
 				test.ServerHostNodeID,
@@ -303,6 +305,7 @@ var (
 			LabelMinor: test.ServerHostName,
 			Rank:       "",
 			Pseudo:     true,
+			Adjacency:  report.MakeIDList(),
 			Origins: report.MakeIDList(
 				test.NonContainerProcessNodeID,
 				test.ServerHostNodeID,

render/render.go

@@ -16,26 +16,6 @@ type Renderer interface {
 // other renderers.
 type Reduce []Renderer
 
-// Map is a Renderer which produces a set of RenderableNodes from the set of
-// RenderableNodes produced by another Renderer.
-type Map struct {
-	MapFunc
-	Renderer
-}
-
-// LeafMap is a Renderer which produces a set of RenderableNodes from a report.Topology
-// by using a map function and topology selector.
-type LeafMap struct {
-	Selector report.TopologySelector
-	Mapper   LeafMapFunc
-	Pseudo   PseudoFunc
-}
-
-// FilterUnconnected is a Renderer which filters out unconnected nodes.
-type FilterUnconnected struct {
-	Renderer
-}
-
 // MakeReduce is the only sane way to produce a Reduce Renderer.
 func MakeReduce(renderers ...Renderer) Renderer {
 	return Reduce(renderers)
@@ -54,11 +34,18 @@ func (r Reduce) Render(rpt report.Report) RenderableNodes {
 func (r Reduce) EdgeMetadata(rpt report.Report, localID, remoteID string) report.EdgeMetadata {
 	metadata := report.EdgeMetadata{}
 	for _, renderer := range r {
-		metadata.Merge(renderer.EdgeMetadata(rpt, localID, remoteID))
+		metadata = metadata.Merge(renderer.EdgeMetadata(rpt, localID, remoteID))
 	}
 	return metadata
 }
 
+// Map is a Renderer which produces a set of RenderableNodes from the set of
+// RenderableNodes produced by another Renderer.
+type Map struct {
+	MapFunc
+	Renderer
+}
+
 // Render transforms a set of RenderableNodes produces by another Renderer.
 // using a map function
 func (m Map) Render(rpt report.Report) RenderableNodes {
@@ -133,11 +120,19 @@ func (m Map) EdgeMetadata(rpt report.Report, srcRenderableID, dstRenderableID st
 	output := report.EdgeMetadata{}
 	for _, edge := range oldEdges {
 		metadata := m.Renderer.EdgeMetadata(rpt, edge.src, edge.dst)
-		output.Merge(metadata)
+		output = output.Merge(metadata)
 	}
 	return output
 }
 
+// LeafMap is a Renderer which produces a set of RenderableNodes from a report.Topology
+// by using a map function and topology selector.
+type LeafMap struct {
+	Selector report.TopologySelector
+	Mapper   LeafMapFunc
+	Pseudo   PseudoFunc
+}
+
 // Render transforms a given Report into a set of RenderableNodes, which
 // the UI will render collectively as a graph. Note that a RenderableNode will
 // always be rendered with other nodes, and therefore contains limited detail.
@@ -234,8 +229,8 @@ func (m LeafMap) Render(rpt report.Report) RenderableNodes {
 			// We propagate edge metadata to nodes on both ends of the edges.
 			// TODO we should 'reverse' one end of the edge meta data - ingress -> egress etc.
 			if md, ok := t.EdgeMetadatas[report.MakeEdgeID(srcNodeID, dstNodeID)]; ok {
-				srcRenderableNode.EdgeMetadata.Merge(md)
-				dstRenderableNode.EdgeMetadata.Merge(md)
+				srcRenderableNode.EdgeMetadata = srcRenderableNode.EdgeMetadata.Merge(md)
+				dstRenderableNode.EdgeMetadata = dstRenderableNode.EdgeMetadata.Merge(md)
 				nodes[dstRenderableID] = dstRenderableNode
 			}
 		}
@@ -268,12 +263,17 @@ func (m LeafMap) EdgeMetadata(rpt report.Report, srcRenderableID, dstRenderableI
 			dst = mapped.ID
 		}
 		if src == srcRenderableID && dst == dstRenderableID {
-			metadata.Flatten(edgeMeta)
+			metadata = metadata.Flatten(edgeMeta)
 		}
 	}
 	return metadata
 }
 
+// FilterUnconnected is a Renderer which filters out unconnected nodes.
+type FilterUnconnected struct {
+	Renderer
+}
+
 // Render produces a set of RenderableNodes given a Report
 func (f FilterUnconnected) Render(rpt report.Report) RenderableNodes {
 	return OnlyConnected(f.Renderer.Render(rpt))

render/render_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/report"
+	"github.com/weaveworks/scope/test"
 )
 
 type mockRenderer struct {
@@ -76,12 +77,12 @@ func TestMapRender2(t *testing.T) {
 			"baz": {ID: "baz"},
 		}},
 	}
-	want := render.RenderableNodes{
+	want := sterilize(render.RenderableNodes{
 		"bar": render.RenderableNode{ID: "bar"},
-	}
+	}, false)
 	have := mapper.Render(report.MakeReport())
 	if !reflect.DeepEqual(want, have) {
-		t.Errorf("want %+v, have %+v", want, have)
+		t.Error(test.Diff(want, have))
 	}
 }
 
@@ -129,8 +130,8 @@ func TestMapEdge(t *testing.T) {
 	}
 
 	mapper := render.Map{
-		MapFunc: func(nodes render.RenderableNode) (render.RenderableNode, bool) {
-			return render.RenderableNode{ID: "_" + nodes.ID}, true
+		MapFunc: func(n render.RenderableNode) (render.RenderableNode, bool) {
+			return render.RenderableNode{ID: "_" + n.ID}, true
 		},
 		Renderer: render.LeafMap{
 			Selector: selector,
@@ -143,7 +144,7 @@ func TestMapEdge(t *testing.T) {
 		EgressPacketCount: newu64(1),
 		EgressByteCount:   newu64(2),
 	}), mapper.EdgeMetadata(report.MakeReport(), "_foo", "_bar"); !reflect.DeepEqual(want, have) {
-		t.Errorf("want %+v, have %+v", want, have)
+		t.Error(test.Diff(want, have))
 	}
 }
 

render/renderable_node.go

@@ -16,8 +16,8 @@ type RenderableNode struct {
 	Adjacency  report.IDList `json:"adjacency,omitempty"`   // Node IDs (in the same topology domain)
 	Origins    report.IDList `json:"origins,omitempty"`     // Core node IDs that contributed information
 
-	report.EdgeMetadata `json:"metadata"` // Numeric sums
-	report.NodeMetadata `json:"-"`        // merged NodeMetadata of the nodes used to build this
+	report.EdgeMetadata `json:"metadata"`        // Numeric sums
+	report.NodeMetadata `json:"XXXNODEMETADATA"` // merged NodeMetadata of the nodes used to build this // TODO ###
 }
 
 // RenderableNodes is a set of RenderableNodes
@@ -56,8 +56,8 @@ func (rn *RenderableNode) Merge(other RenderableNode) {
 	rn.Adjacency = rn.Adjacency.Merge(other.Adjacency)
 	rn.Origins = rn.Origins.Merge(other.Origins)
 
-	rn.EdgeMetadata.Merge(other.EdgeMetadata)
-	rn.NodeMetadata.Merge(other.NodeMetadata)
+	rn.EdgeMetadata = rn.EdgeMetadata.Merge(other.EdgeMetadata)
+	rn.NodeMetadata = rn.NodeMetadata.Merge(other.NodeMetadata)
 }
 
 // NewRenderableNode makes a new RenderableNode
@@ -80,8 +80,8 @@ func newDerivedNode(id string, node RenderableNode) RenderableNode {
 		LabelMinor:   "",
 		Rank:         "",
 		Pseudo:       node.Pseudo,
-		EdgeMetadata: node.EdgeMetadata,
-		Origins:      node.Origins,
+		Origins:      node.Origins.Copy(),
+		EdgeMetadata: node.EdgeMetadata.Copy(),
 		NodeMetadata: report.MakeNodeMetadata(),
 	}
 }
@@ -105,8 +105,8 @@ func newDerivedPseudoNode(id, major string, node RenderableNode) RenderableNode
 		LabelMinor:   "",
 		Rank:         "",
 		Pseudo:       true,
-		EdgeMetadata: node.EdgeMetadata,
-		Origins:      node.Origins,
+		Origins:      node.Origins.Copy(),
+		EdgeMetadata: node.EdgeMetadata.Copy(),
 		NodeMetadata: report.MakeNodeMetadata(),
 	}
 }

render/renderable_node_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/report"
+	"github.com/weaveworks/scope/test"
 )
 
 func TestMergeRenderableNodes(t *testing.T) {
@@ -17,19 +18,45 @@ func TestMergeRenderableNodes(t *testing.T) {
 		"bar": render.RenderableNode{ID: "bar"},
 		"baz": render.RenderableNode{ID: "baz"},
 	}
-
-	want := render.RenderableNodes{
+	want := sterilize(render.RenderableNodes{
 		"foo": render.RenderableNode{ID: "foo"},
 		"bar": render.RenderableNode{ID: "bar"},
 		"baz": render.RenderableNode{ID: "baz"},
-	}
+	}, false)
 	nodes1.Merge(nodes2)
-
-	if !reflect.DeepEqual(want, nodes1) {
-		t.Errorf("want %+v, have %+v", want, nodes1)
+	if have := sterilize(nodes1, false); !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
 	}
 }
 
+func sterilize(r render.RenderableNodes, destructive bool) render.RenderableNodes {
+	// Since introducing new map fields to the report.NodeMetadata type, its
+	// zero value is •not valid• -- every time you need one, you need to use
+	// the report.MakeNodeMetadata constructor. (Similarly, but not exactly
+	// the same, is that a zero-value Adjacency is not the same as a created
+	// but empty Adjacency.)
+	//
+	// But we're not doing this in tests. So this function sterilizes invalid
+	// RenderableNodes by fixing all nil Metadata fields. The proper fix
+	// involves lots of annoying changes to instantiation.
+	//
+	// The extra destructive parameter is to support a historical test use
+	// case where we explicitly don't compare node metadata.
+	for id, n := range r {
+		if n.Adjacency == nil {
+			n.Adjacency = report.IDList{}
+		}
+		if destructive || n.NodeMetadata.Metadata == nil {
+			n.NodeMetadata.Metadata = map[string]string{}
+		}
+		if destructive || n.NodeMetadata.Counters == nil {
+			n.NodeMetadata.Counters = map[string]int{}
+		}
+		r[id] = n
+	}
+	return r
+}
+
 func TestMergeRenderableNode(t *testing.T) {
 	node1 := render.RenderableNode{
 		ID:         "foo",
@@ -49,19 +76,19 @@ func TestMergeRenderableNode(t *testing.T) {
 		Adjacency:  report.MakeIDList("a2"),
 		Origins:    report.MakeIDList("o2"),
 	}
-
 	want := render.RenderableNode{
-		ID:         "foo",
-		LabelMajor: "major",
-		LabelMinor: "minor",
-		Rank:       "rank",
-		Pseudo:     false,
-		Adjacency:  report.MakeIDList("a1", "a2"),
-		Origins:    report.MakeIDList("o1", "o2"),
+		ID:           "foo",
+		LabelMajor:   "major",
+		LabelMinor:   "minor",
+		Rank:         "rank",
+		Pseudo:       false,
+		Adjacency:    report.MakeIDList("a1", "a2"),
+		Origins:      report.MakeIDList("o1", "o2"),
+		NodeMetadata: report.MakeNodeMetadata(),
+		EdgeMetadata: report.EdgeMetadata{},
 	}
 	node1.Merge(node2)
-
-	if !reflect.DeepEqual(want, node1) {
-		t.Errorf("want %+v, have %+v", want, node1)
+	if have := node1; !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
 	}
 }

render/theinternet_test.go

@@ -12,11 +12,16 @@ import (
 )
 
 func TestReportLocalNetworks(t *testing.T) {
-	r := report.MakeReport()
-	r.Merge(report.Report{Host: report.Topology{NodeMetadatas: report.NodeMetadatas{
-		"nonets": report.MakeNodeMetadata(),
-		"foo":    report.MakeNodeMetadataWith(map[string]string{host.LocalNetworks: "10.0.0.1/8 192.168.1.1/24 10.0.0.1/8 badnet/33"}),
-	}}})
+	r := report.MakeReport().Merge(report.Report{
+		Host: report.Topology{
+			NodeMetadatas: report.NodeMetadatas{
+				"nonets": report.MakeNodeMetadata(),
+				"foo": report.MakeNodeMetadataWith(map[string]string{
+					host.LocalNetworks: "10.0.0.1/8 192.168.1.1/24 10.0.0.1/8 badnet/33",
+				}),
+			},
+		},
+	})
 	want := report.Networks([]*net.IPNet{
 		mustParseCIDR("10.0.0.1/8"),
 		mustParseCIDR("192.168.1.1/24"),

render/topologies_test.go

@@ -6,55 +6,45 @@ import (
 
 	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/render/expected"
-	"github.com/weaveworks/scope/report"
 	"github.com/weaveworks/scope/test"
 )
 
-func trimNodeMetadata(rns render.RenderableNodes) render.RenderableNodes {
-	result := render.RenderableNodes{}
-	for id, rn := range rns {
-		rn.NodeMetadata = report.MakeNodeMetadata()
-		result[id] = rn
-	}
-	return result
-}
-
 func TestProcessRenderer(t *testing.T) {
-	have := render.ProcessRenderer.Render(test.Report)
-	have = trimNodeMetadata(have)
-	if !reflect.DeepEqual(expected.RenderedProcesses, have) {
-		t.Error(test.Diff(expected.RenderedProcesses, have))
+	have := sterilize(render.ProcessRenderer.Render(test.Report), true)
+	want := expected.RenderedProcesses
+	if !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
 	}
 }
 
 func TestProcessNameRenderer(t *testing.T) {
-	have := render.ProcessNameRenderer.Render(test.Report)
-	have = trimNodeMetadata(have)
-	if !reflect.DeepEqual(expected.RenderedProcessNames, have) {
-		t.Error(test.Diff(expected.RenderedProcessNames, have))
+	have := sterilize(render.ProcessNameRenderer.Render(test.Report), true)
+	want := expected.RenderedProcessNames
+	if !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
 	}
 }
 
 func TestContainerRenderer(t *testing.T) {
-	have := render.ContainerRenderer.Render(test.Report)
-	have = trimNodeMetadata(have)
-	if !reflect.DeepEqual(expected.RenderedContainers, have) {
-		t.Error(test.Diff(expected.RenderedContainers, have))
+	have := sterilize(render.ContainerRenderer.Render(test.Report), true)
+	want := expected.RenderedContainers
+	if !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
 	}
 }
 
 func TestContainerImageRenderer(t *testing.T) {
-	have := render.ContainerImageRenderer.Render(test.Report)
-	have = trimNodeMetadata(have)
-	if !reflect.DeepEqual(expected.RenderedContainerImages, have) {
-		t.Error(test.Diff(expected.RenderedContainerImages, have))
+	have := sterilize(render.ContainerImageRenderer.Render(test.Report), true)
+	want := expected.RenderedContainerImages
+	if !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
 	}
 }
 
 func TestHostRenderer(t *testing.T) {
-	have := render.HostRenderer.Render(test.Report)
-	have = trimNodeMetadata(have)
-	if !reflect.DeepEqual(expected.RenderedHosts, have) {
-		t.Error(test.Diff(expected.RenderedHosts, have))
+	have := sterilize(render.HostRenderer.Render(test.Report), true)
+	want := expected.RenderedHosts
+	if !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
 	}
 }

report/merge_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/weaveworks/scope/report"
+	"github.com/weaveworks/scope/test"
 )
 
 const (
@@ -191,11 +192,30 @@ func TestMergeEdgeMetadatas(t *testing.T) {
 		},
 	} {
 		if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) {
-			t.Errorf("%s: want\n\t%#v, have\n\t%#v", name, c.want, have)
+			t.Errorf("%s:\n%s", name, test.Diff(c.want, have))
 		}
 	}
 }
 
+func TestFlattenEdgeMetadata(t *testing.T) {
+	have := (report.EdgeMetadata{
+		EgressPacketCount: newu64(1),
+		MaxConnCountTCP:   newu64(2),
+	}).Flatten(report.EdgeMetadata{
+		EgressPacketCount: newu64(4),
+		EgressByteCount:   newu64(8),
+		MaxConnCountTCP:   newu64(16),
+	})
+	want := report.EdgeMetadata{
+		EgressPacketCount: newu64(1 + 4),
+		EgressByteCount:   newu64(8),
+		MaxConnCountTCP:   newu64(2 + 16), // flatten should sum MaxConnCountTCP
+	}
+	if !reflect.DeepEqual(want, have) {
+		t.Error(test.Diff(want, have))
+	}
+}
+
 func TestMergeNodeMetadatas(t *testing.T) {
 	for name, c := range map[string]struct {
 		a, b, want report.NodeMetadatas