diff --git a/app/api_topology_test.go b/app/api_topology_test.go index 7edf67430..8bd90038b 100644 --- a/app/api_topology_test.go +++ b/app/api_topology_test.go @@ -16,19 +16,6 @@ import ( "github.com/weaveworks/scope/test" ) -// A copy of sanitize from the render package. -func sanitize(nodes render.RenderableNodes) render.RenderableNodes { - for id, n := range nodes { - if n.Adjacency == nil { - n.Adjacency = report.IDList{} - } - n.NodeMetadata.Metadata = map[string]string{} - n.NodeMetadata.Counters = map[string]int{} - nodes[id] = n - } - return nodes -} - func TestAll(t *testing.T) { ts := httptest.NewServer(Router(StaticReport{})) defer ts.Close() @@ -73,7 +60,7 @@ func TestAPITopologyContainers(t *testing.T) { t.Fatal(err) } - if want, have := expected.RenderedContainers, sanitize(topo.Nodes); !reflect.DeepEqual(want, have) { + if want, have := expected.RenderedContainers, expected.Sterilize(topo.Nodes); !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } } @@ -121,7 +108,7 @@ func TestAPITopologyHosts(t *testing.T) { t.Fatal(err) } - if want, have := expected.RenderedHosts, sanitize(topo.Nodes); !reflect.DeepEqual(want, have) { + if want, have := expected.RenderedHosts, expected.Sterilize(topo.Nodes); !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } } diff --git a/render/expected/expected.go b/render/expected/expected.go index d90cd9ba9..f6e7c22ad 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -8,6 +8,28 @@ import ( "github.com/weaveworks/scope/test" ) +func Sterilize(r render.RenderableNodes) 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. We also don't particularly care + // about the output of NodeMetadata for the rendering pipeline, as this + // is never serialised to json. So this function sterilizes invalid + // RenderableNodes by setting an empty NodeMetadata from the proper + // constructor. + for id, n := range r { + if n.Adjacency == nil { + n.Adjacency = report.MakeIDList() + } + n.NodeMetadata = report.MakeNodeMetadata() + r[id] = n + } + return r +} + // Exported for testing. var ( uncontainedServerID = render.MakePseudoNodeID(render.UncontainedID, test.ServerHostName) @@ -68,7 +90,7 @@ var ( ServerProcessID = render.MakeProcessID(test.ServerHostID, test.ServerPID) nonContainerProcessID = render.MakeProcessID(test.ServerHostID, test.NonContainerPID) - RenderedProcesses = render.RenderableNodes{ + RenderedProcesses = Sterilize(render.RenderableNodes{ ClientProcess1ID: { ID: ClientProcess1ID, LabelMajor: test.Client1Comm, @@ -141,9 +163,9 @@ var ( unknownPseudoNode1ID: unknownPseudoNode1(report.MakeIDList(ServerProcessID)), unknownPseudoNode2ID: unknownPseudoNode2(report.MakeIDList(ServerProcessID)), render.TheInternetID: theInternetNode(report.MakeIDList(ServerProcessID)), - } + }) - RenderedProcessNames = render.RenderableNodes{ + RenderedProcessNames = Sterilize(render.RenderableNodes{ "curl": { ID: "curl", LabelMajor: "curl", @@ -200,9 +222,9 @@ var ( unknownPseudoNode1ID: unknownPseudoNode1(report.MakeIDList("apache")), unknownPseudoNode2ID: unknownPseudoNode2(report.MakeIDList("apache")), render.TheInternetID: theInternetNode(report.MakeIDList("apache")), - } + }) - RenderedContainers = render.RenderableNodes{ + RenderedContainers = Sterilize(render.RenderableNodes{ test.ClientContainerID: { ID: test.ClientContainerID, LabelMajor: "client", @@ -259,9 +281,9 @@ var ( EdgeMetadata: report.EdgeMetadata{}, }, render.TheInternetID: theInternetNode(report.MakeIDList(test.ServerContainerID)), - } + }) - RenderedContainerImages = render.RenderableNodes{ + RenderedContainerImages = Sterilize(render.RenderableNodes{ test.ClientContainerImageName: { ID: test.ClientContainerImageName, LabelMajor: test.ClientContainerImageName, @@ -319,14 +341,14 @@ var ( EdgeMetadata: report.EdgeMetadata{}, }, render.TheInternetID: theInternetNode(report.MakeIDList(test.ServerContainerImageName)), - } + }) ServerHostRenderedID = render.MakeHostID(test.ServerHostID) ClientHostRenderedID = render.MakeHostID(test.ClientHostID) pseudoHostID1 = render.MakePseudoNodeID(test.UnknownClient1IP, test.ServerIP) pseudoHostID2 = render.MakePseudoNodeID(test.UnknownClient3IP, test.ServerIP) - RenderedHosts = render.RenderableNodes{ + RenderedHosts = Sterilize(render.RenderableNodes{ ServerHostRenderedID: { ID: ServerHostRenderedID, LabelMajor: "server", // before first . @@ -386,7 +408,7 @@ var ( EdgeMetadata: report.EdgeMetadata{}, Origins: report.MakeIDList(test.RandomAddressNodeID), }, - } + }) ) func newu64(value uint64) *uint64 { return &value } diff --git a/render/render_test.go b/render/render_test.go index ddbfa44ab..d881f5761 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/weaveworks/scope/render" + "github.com/weaveworks/scope/render/expected" "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test" ) @@ -77,9 +78,9 @@ func TestMapRender2(t *testing.T) { "baz": {ID: "baz"}, }}, } - want := sterilize(render.RenderableNodes{ + want := expected.Sterilize(render.RenderableNodes{ "bar": render.RenderableNode{ID: "bar"}, - }, false) + }) have := mapper.Render(report.MakeReport()) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -171,7 +172,7 @@ func TestFilterRender(t *testing.T) { "foo": {ID: "foo", Adjacency: report.MakeIDList("bar"), NodeMetadata: report.MakeNodeMetadata()}, "bar": {ID: "bar", Adjacency: report.MakeIDList("foo"), NodeMetadata: report.MakeNodeMetadata()}, } - have := sterilize(renderer.Render(report.MakeReport()), true) + have := expected.Sterilize(renderer.Render(report.MakeReport())) if !reflect.DeepEqual(want, have) { t.Errorf("want %+v, have %+v", want, have) } diff --git a/render/renderable_node_test.go b/render/renderable_node_test.go index 4901ec65c..bbc39c6d1 100644 --- a/render/renderable_node_test.go +++ b/render/renderable_node_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/weaveworks/scope/render" + "github.com/weaveworks/scope/render/expected" "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test" ) @@ -18,48 +19,17 @@ func TestMergeRenderableNodes(t *testing.T) { "bar": render.RenderableNode{ID: "bar"}, "baz": render.RenderableNode{ID: "baz"}, } - want := sterilize(render.RenderableNodes{ + want := expected.Sterilize(render.RenderableNodes{ "foo": render.RenderableNode{ID: "foo"}, "bar": render.RenderableNode{ID: "bar"}, "baz": render.RenderableNode{ID: "baz"}, - }, false) + }) nodes1.Merge(nodes2) - if have := sterilize(nodes1, false); !reflect.DeepEqual(want, have) { + if have := expected.Sterilize(nodes1); !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.MakeIDList() - } - if destructive || n.NodeMetadata.Adjacency == nil { - n.NodeMetadata.Adjacency = report.MakeIDList() - } - 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", diff --git a/render/topologies_test.go b/render/topologies_test.go index f46ec0061..25f37bd5c 100644 --- a/render/topologies_test.go +++ b/render/topologies_test.go @@ -10,7 +10,7 @@ import ( ) func TestProcessRenderer(t *testing.T) { - have := sterilize(render.ProcessRenderer.Render(test.Report), true) + have := expected.Sterilize(render.ProcessRenderer.Render(test.Report)) want := expected.RenderedProcesses if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -18,7 +18,7 @@ func TestProcessRenderer(t *testing.T) { } func TestProcessNameRenderer(t *testing.T) { - have := sterilize(render.ProcessNameRenderer.Render(test.Report), true) + have := expected.Sterilize(render.ProcessNameRenderer.Render(test.Report)) want := expected.RenderedProcessNames if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -26,7 +26,7 @@ func TestProcessNameRenderer(t *testing.T) { } func TestContainerRenderer(t *testing.T) { - have := sterilize(render.ContainerRenderer.Render(test.Report), true) + have := expected.Sterilize(render.ContainerRenderer.Render(test.Report)) want := expected.RenderedContainers if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -34,7 +34,7 @@ func TestContainerRenderer(t *testing.T) { } func TestContainerImageRenderer(t *testing.T) { - have := sterilize(render.ContainerImageRenderer.Render(test.Report), true) + have := expected.Sterilize(render.ContainerImageRenderer.Render(test.Report)) want := expected.RenderedContainerImages if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -42,7 +42,7 @@ func TestContainerImageRenderer(t *testing.T) { } func TestHostRenderer(t *testing.T) { - have := sterilize(render.HostRenderer.Render(test.Report), true) + have := expected.Sterilize(render.HostRenderer.Render(test.Report)) want := expected.RenderedHosts if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have))