PR comments implemented

- removed overwritting setting of concurrencyCount to 3 - removed commented code - added case to config based routing unit tests
intelsdi-x · May 22, 2016 · 409b4c2 · 409b4c2
1 parent d6accfa
commit 409b4c2
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 44 deletions.
diff --git a/control/strategy/config_based_test.go b/control/strategy/config_based_test.go
@@ -27,28 +27,34 @@ import (
 )
 
 func TestConfigBasedRouter(t *testing.T) {
-	Convey("Given a sticky router", t, func() {
+	Convey("Given a config router", t, func() {
 		router := NewConfigBased(100 * time.Millisecond)
 		So(router, ShouldNotBeNil)
 		So(router.String(), ShouldResemble, "config-based")
 		Convey("Select a plugin when they are available", func() {
 			p1 := NewMockAvailablePlugin().WithName("p1")
 			p2 := NewMockAvailablePlugin().WithName("p2")
-			// select a plugin, for task1,  given a task and two available plugins
+			// select a plugin, for cfg1,  given a config and two available plugins
 			sp1, err := router.Select([]AvailablePlugin{p1, p2}, "cfg1")
 			So(err, ShouldBeNil)
 			So(sp1, ShouldNotBeNil)
 			So(sp1, ShouldEqual, p1)
 			// change the order of the plugins provided to the select
 			sp2, err := router.Select([]AvailablePlugin{p2, p1}, "cfg1")
 			So(err, ShouldBeNil)
-			So(sp1, ShouldNotBeNil)
+			So(sp2, ShouldNotBeNil)
 			So(sp2, ShouldEqual, p1)
-			// select the other (last) available plugin for task2
+			// select the other (last) available plugin for cfg2
 			sp3, err := router.Select([]AvailablePlugin{p2, p1}, "cfg2")
 			So(err, ShouldBeNil)
 			So(sp3, ShouldNotBeNil)
 			So(sp3, ShouldEqual, p2)
+			Convey("Select a plugin when there are NONE available", func() {
+				plugins := []AvailablePlugin{p1, p2}
+				sp, err := router.Select(plugins, "cfg3")
+				So(sp, ShouldBeNil)
+				So(err, ShouldEqual, ErrCouldNotSelect)
+			})
 		})
 
 	})

diff --git a/control/strategy/fixtures/fixtures.go b/control/strategy/fixtures/fixtures.go
@@ -181,34 +181,3 @@ func (m MockAvailablePlugin) Name() string {
 func (m MockAvailablePlugin) Version() int {
 	return m.version
 }
-
-/*
-func NewMockMetricType(ns string) MockMetricType {
-	return MockMetricType{
-		namespace: strings.Split(ns, "/"),
-	}
-}
-
-type MockMetricType struct {
-	namespace []string
-}
-
-func (m MockMetricType) Namespace() []string { return m.namespace }
-
-func (m MockMetricType) LastAdvertisedTime() time.Time { return time.Now() }
-
-func (m MockMetricType) Version() int { return 1 }
-
-func (m MockMetricType) Config() *cdata.ConfigDataNode { return nil }
-
-func (m MockMetricType) Data() interface{} { return nil }
-
-func (m MockMetricType) Source() string { return "" }
-
-func (m MockMetricType) Tags() map[string]string { return nil }
-
-func (m MockMetricType) Labels() []core.Label { return nil }
-
-func (m MockMetricType) Timestamp() time.Time { return time.Time{} }
-
-*/
diff --git a/control/strategy/pool.go b/control/strategy/pool.go
@@ -232,7 +232,6 @@ func (p *pool) applyPluginMeta(a AvailablePlugin) error {
 		p.concurrencyCount = 1
 	case plugin.ConfigRouting:
 		p.RoutingAndCaching = NewConfigBased(cacheTTL)
-		p.concurrencyCount = 3
 	default:
 		return ErrBadStrategy
 	}

diff --git a/control/strategy/pool_test.go b/control/strategy/pool_test.go
@@ -163,16 +163,21 @@ func TestPoolEligibility(t *testing.T) {
 				{plugin.CollectorPluginType, plugin.StickyRouting, 999, 1, false, false},
 				{plugin.CollectorPluginType, plugin.StickyRouting, 999, 2, false, true},
 				{plugin.CollectorPluginType, plugin.StickyRouting, 999, 3, false, true},
-				{plugin.CollectorPluginType, plugin.StickyRouting, 999, 4, false, false},
+				{plugin.CollectorPluginType, plugin.StickyRouting, 999, 4, false, true},
 				{plugin.CollectorPluginType, plugin.StickyRouting, 999, 2, true, false},
 
-				{plugin.CollectorPluginType, plugin.ConfigRouting, 999, 0, false, false},
-				{plugin.CollectorPluginType, plugin.ConfigRouting, 999, 1, false, false},
-				{plugin.CollectorPluginType, plugin.ConfigRouting, 999, 2, false, false},
-				{plugin.CollectorPluginType, plugin.ConfigRouting, 999, 3, false, false},
-				{plugin.CollectorPluginType, plugin.ConfigRouting, 999, 4, false, true},
-				{plugin.CollectorPluginType, plugin.ConfigRouting, 999, 5, false, true},
-				{plugin.CollectorPluginType, plugin.ConfigRouting, 999, 4, true, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 1, 0, false, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 1, 1, false, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 1, 2, false, true},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 2, 1, false, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 2, 2, false, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 2, 3, false, true},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 2, 3, true, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 2, 4, false, true},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 3, 1, false, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 3, 2, false, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 3, 3, false, false},
+				{plugin.CollectorPluginType, plugin.ConfigRouting, 3, 4, false, true},
 			}
 			Convey("Then new pool eligibility is defined", func() {
 				for i, tc := range tcs {