From a89c0b9b88878b79a9378eba38ddd3700e7910f9 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Mon, 9 Nov 2015 16:25:49 +0000 Subject: [PATCH] Make an empty StringSet nil. --- render/filters.go | 2 +- render/filters_test.go | 110 ++++++++++++++++++++++++++++++++++++++++ render/render_test.go | 100 ------------------------------------ report/topology.go | 10 +++- report/topology_test.go | 4 +- 5 files changed, 122 insertions(+), 104 deletions(-) create mode 100644 render/filters_test.go diff --git a/render/filters.go b/render/filters.go index 6c08aed3f..84711d177 100644 --- a/render/filters.go +++ b/render/filters.go @@ -78,7 +78,7 @@ func (f Filter) render(rpt report.Report) (RenderableNodes, int) { // Deleted nodes also need to be cut as destinations in adjacency lists. for id, node := range output { - newAdjacency := make(report.IDList, 0, len(node.Adjacency)) + newAdjacency := report.MakeIDList() for _, dstID := range node.Adjacency { if _, ok := output[dstID]; ok { newAdjacency = newAdjacency.Add(dstID) diff --git a/render/filters_test.go b/render/filters_test.go new file mode 100644 index 000000000..905fea177 --- /dev/null +++ b/render/filters_test.go @@ -0,0 +1,110 @@ +package render_test + +import ( + "reflect" + "testing" + + "github.com/weaveworks/scope/render" + "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test" +) + +func TestFilterRender(t *testing.T) { + renderer := render.FilterUnconnected( + mockRenderer{RenderableNodes: render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, + "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("foo")}, + "baz": {ID: "baz", Node: report.MakeNode()}, + }}) + want := render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, + "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("foo")}, + } + have := renderer.Render(report.MakeReport()).Prune() + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +} + +func TestFilterRender2(t *testing.T) { + // Test adjacencies are removed for filtered nodes. + renderer := render.Filter{ + FilterFunc: func(node render.RenderableNode) bool { + return node.ID != "bar" + }, + Renderer: mockRenderer{RenderableNodes: render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, + "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("foo")}, + "baz": {ID: "baz", Node: report.MakeNode()}, + }}, + } + want := render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode()}, + "baz": {ID: "baz", Node: report.MakeNode()}, + } + have := renderer.Render(report.MakeReport()).Prune() + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +} + +func TestFilterUnconnectedPesudoNodes(t *testing.T) { + // Test pseudo nodes that are made unconnected by filtering + // are also removed. + { + nodes := render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, + "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("baz")}, + "baz": {ID: "baz", Node: report.MakeNode(), Pseudo: true}, + } + renderer := render.Filter{ + FilterFunc: func(node render.RenderableNode) bool { + return true + }, + Renderer: mockRenderer{RenderableNodes: nodes}, + } + want := nodes.Prune() + have := renderer.Render(report.MakeReport()).Prune() + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } + } + { + renderer := render.Filter{ + FilterFunc: func(node render.RenderableNode) bool { + return node.ID != "bar" + }, + Renderer: mockRenderer{RenderableNodes: render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, + "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("baz")}, + "baz": {ID: "baz", Node: report.MakeNode(), Pseudo: true}, + }}, + } + want := render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode()}, + } + have := renderer.Render(report.MakeReport()).Prune() + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } + } + { + renderer := render.Filter{ + FilterFunc: func(node render.RenderableNode) bool { + return node.ID != "bar" + }, + Renderer: mockRenderer{RenderableNodes: render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode()}, + "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("foo")}, + "baz": {ID: "baz", Node: report.MakeNode().WithAdjacent("bar"), Pseudo: true}, + }}, + } + want := render.RenderableNodes{ + "foo": {ID: "foo", Node: report.MakeNode()}, + } + have := renderer.Render(report.MakeReport()).Prune() + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } + } +} diff --git a/render/render_test.go b/render/render_test.go index 1604562ed..df2f19d86 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -180,104 +180,4 @@ func TestMapEdge(t *testing.T) { } } -func TestFilterRender(t *testing.T) { - renderer := render.FilterUnconnected( - mockRenderer{RenderableNodes: render.RenderableNodes{ - "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, - "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("foo")}, - "baz": {ID: "baz", Node: report.MakeNode()}, - }}) - want := render.RenderableNodes{ - "foo": {ID: "foo", Origins: report.IDList{}, Node: report.MakeNode().WithAdjacent("bar")}, - "bar": {ID: "bar", Origins: report.IDList{}, Node: report.MakeNode().WithAdjacent("foo")}, - }.Prune() - have := renderer.Render(report.MakeReport()).Prune() - if !reflect.DeepEqual(want, have) { - t.Error(test.Diff(want, have)) - } -} - -func TestFilterRender2(t *testing.T) { - // Test adjacencies are removed for filtered nodes. - renderer := render.Filter{ - FilterFunc: func(node render.RenderableNode) bool { - return node.ID != "bar" - }, - Renderer: mockRenderer{RenderableNodes: render.RenderableNodes{ - "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, - "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("foo")}, - "baz": {ID: "baz", Node: report.MakeNode()}, - }}, - } - want := render.RenderableNodes{ - "foo": {ID: "foo", Origins: report.IDList{}, Node: report.MakeNode()}, - "baz": {ID: "baz", Origins: report.IDList{}, Node: report.MakeNode()}, - }.Prune() - have := renderer.Render(report.MakeReport()).Prune() - if !reflect.DeepEqual(want, have) { - t.Error(test.Diff(want, have)) - } -} - -func TestFilterUnconnectedPesudoNodes(t *testing.T) { - // Test pseudo nodes that are made unconnected by filtering - // are also removed. - { - nodes := render.RenderableNodes{ - "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, - "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("baz")}, - "baz": {ID: "baz", Node: report.MakeNode(), Pseudo: true}, - } - renderer := render.Filter{ - FilterFunc: func(node render.RenderableNode) bool { - return true - }, - Renderer: mockRenderer{RenderableNodes: nodes}, - } - want := nodes.Prune() - have := renderer.Render(report.MakeReport()).Prune() - if !reflect.DeepEqual(want, have) { - t.Error(test.Diff(want, have)) - } - } - { - renderer := render.Filter{ - FilterFunc: func(node render.RenderableNode) bool { - return node.ID != "bar" - }, - Renderer: mockRenderer{RenderableNodes: render.RenderableNodes{ - "foo": {ID: "foo", Node: report.MakeNode().WithAdjacent("bar")}, - "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("baz")}, - "baz": {ID: "baz", Node: report.MakeNode(), Pseudo: true}, - }}, - } - want := render.RenderableNodes{ - "foo": {ID: "foo", Origins: report.IDList{}, Node: report.MakeNode()}, - }.Prune() - have := renderer.Render(report.MakeReport()).Prune() - if !reflect.DeepEqual(want, have) { - t.Error(test.Diff(want, have)) - } - } - { - renderer := render.Filter{ - FilterFunc: func(node render.RenderableNode) bool { - return node.ID != "bar" - }, - Renderer: mockRenderer{RenderableNodes: render.RenderableNodes{ - "foo": {ID: "foo", Node: report.MakeNode()}, - "bar": {ID: "bar", Node: report.MakeNode().WithAdjacent("foo")}, - "baz": {ID: "baz", Node: report.MakeNode().WithAdjacent("bar"), Pseudo: true}, - }}, - } - want := render.RenderableNodes{ - "foo": {ID: "foo", Origins: report.IDList{}, Node: report.MakeNode()}, - }.Prune() - have := renderer.Render(report.MakeReport()).Prune() - if !reflect.DeepEqual(want, have) { - t.Error(test.Diff(want, have)) - } - } -} - func newu64(value uint64) *uint64 { return &value } diff --git a/report/topology.go b/report/topology.go index 9fa6de15c..3700b1496 100644 --- a/report/topology.go +++ b/report/topology.go @@ -271,7 +271,7 @@ type StringSet []string // MakeStringSet makes a new StringSet with the given strings. func MakeStringSet(strs ...string) StringSet { if len(strs) <= 0 { - return StringSet{} + return nil } result := make([]string, len(strs)) copy(result, strs) @@ -305,8 +305,11 @@ func (s StringSet) Add(strs ...string) StringSet { // Merge combines the two StringSets and returns a new result. func (s StringSet) Merge(other StringSet) StringSet { - if len(other) == 0 { // Optimise special case, to avoid allocating + switch { + case len(other) <= 0: // Optimise special case, to avoid allocating return s // (note unit test DeepEquals breaks if we don't do this) + case len(s) <= 0: + return other } result := make(StringSet, len(s)+len(other)) for i, j, k := 0, 0, 0; ; k++ { @@ -333,6 +336,9 @@ func (s StringSet) Merge(other StringSet) StringSet { // Copy returns a value copy of the StringSet. func (s StringSet) Copy() StringSet { + if s == nil { + return s + } result := make(StringSet, len(s)) copy(result, s) return result diff --git a/report/topology_test.go b/report/topology_test.go index bd0a3a978..c965f9b7d 100644 --- a/report/topology_test.go +++ b/report/topology_test.go @@ -12,6 +12,7 @@ func TestMakeStringSet(t *testing.T) { input []string want report.StringSet }{ + {input: nil, want: nil}, {input: []string{}, want: report.MakeStringSet()}, {input: []string{"a"}, want: report.MakeStringSet("a")}, {input: []string{"a", "a"}, want: report.MakeStringSet("a")}, @@ -29,6 +30,7 @@ func TestStringSetAdd(t *testing.T) { strs []string want report.StringSet }{ + {input: report.StringSet(nil), strs: []string{}, want: report.StringSet(nil)}, {input: report.MakeStringSet(), strs: []string{}, want: report.MakeStringSet()}, {input: report.MakeStringSet("a"), strs: []string{}, want: report.MakeStringSet("a")}, {input: report.MakeStringSet(), strs: []string{"a"}, want: report.MakeStringSet("a")}, @@ -49,6 +51,7 @@ func TestStringSetMerge(t *testing.T) { other report.StringSet want report.StringSet }{ + {input: report.StringSet(nil), other: report.StringSet(nil), want: report.StringSet(nil)}, {input: report.MakeStringSet(), other: report.MakeStringSet(), want: report.MakeStringSet()}, {input: report.MakeStringSet("a"), other: report.MakeStringSet(), want: report.MakeStringSet("a")}, {input: report.MakeStringSet(), other: report.MakeStringSet("a"), want: report.MakeStringSet("a")}, @@ -62,5 +65,4 @@ func TestStringSetMerge(t *testing.T) { t.Errorf("%v + %v: want %v, have %v", testcase.input, testcase.other, want, have) } } - }