From 0b7f7ff4561dcf49a707bdca1c75e64f4d69085c Mon Sep 17 00:00:00 2001
From: QingWang0909 <lucifer0909@gmail.com>
Date: Sun, 21 May 2017 13:48:32 -0400
Subject: [PATCH] Update : Fix a bug for StatisticCollector.java causes by a
 change of OpenFlow protocols, with Parameterized JUnit test

---
 .../statistics/StatisticsCollector.java       |  12 +
 .../statistics/GetCurrentPortSpeedTest.java   | 145 +++++++++++
 .../statistics/StatisticsTest.java            | 228 ------------------
 3 files changed, 157 insertions(+), 228 deletions(-)
 create mode 100644 src/test/java/net/floodlightcontroller/statistics/GetCurrentPortSpeedTest.java
 delete mode 100644 src/test/java/net/floodlightcontroller/statistics/StatisticsTest.java

diff --git a/src/main/java/net/floodlightcontroller/statistics/StatisticsCollector.java b/src/main/java/net/floodlightcontroller/statistics/StatisticsCollector.java
index d802fe6e5..58ce31b0c 100644
--- a/src/main/java/net/floodlightcontroller/statistics/StatisticsCollector.java
+++ b/src/main/java/net/floodlightcontroller/statistics/StatisticsCollector.java
@@ -124,6 +124,17 @@ public class StatisticsCollector implements IFloodlightModule, IStatisticsServic
 			}
 		}
 
+		/* This is the original Code with Bugs */
+//		protected  long getSpeed(NodePortTuple npt) {
+//			IOFSwitch sw = switchService.getSwitch(npt.getNodeId());
+//			long speed = 0;
+//			if(sw != null){
+//				speed = sw.getPort(npt.getPortId()).getCurrSpeed();
+//			}
+//			return speed;
+//		}
+
+		/* Fix bug from here */
 		protected long getSpeed(NodePortTuple npt) {
 			IOFSwitch sw = switchService.getSwitch(npt.getNodeId());
 			long speed = 0;
@@ -160,6 +171,7 @@ public class StatisticsCollector implements IFloodlightModule, IStatisticsServic
 			return speed;
 
 		}
+
 	}
 
 	/**
diff --git a/src/test/java/net/floodlightcontroller/statistics/GetCurrentPortSpeedTest.java b/src/test/java/net/floodlightcontroller/statistics/GetCurrentPortSpeedTest.java
new file mode 100644
index 000000000..5192973d1
--- /dev/null
+++ b/src/test/java/net/floodlightcontroller/statistics/GetCurrentPortSpeedTest.java
@@ -0,0 +1,145 @@
+package net.floodlightcontroller.statistics;
+
+import net.floodlightcontroller.core.FloodlightContext;
+import net.floodlightcontroller.core.IOFSwitch;
+import net.floodlightcontroller.core.internal.IOFSwitchService;
+import net.floodlightcontroller.core.module.FloodlightModuleContext;
+import net.floodlightcontroller.core.test.MockThreadPoolService;
+import net.floodlightcontroller.core.types.NodePortTuple;
+import net.floodlightcontroller.test.FloodlightTestCase;
+import org.easymock.EasyMock;
+import org.projectfloodlight.openflow.protocol.*;
+
+import org.projectfloodlight.openflow.types.DatapathId;
+import org.projectfloodlight.openflow.types.OFPort;
+import org.projectfloodlight.openflow.protocol.OFFactories;
+import net.floodlightcontroller.threadpool.IThreadPoolService;
+
+import java.util.*;
+import static org.easymock.EasyMock.*;
+import static org.junit.Assert.*;
+import org.junit.*;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Created by qing wang on 5/18/17.
+ */
+@RunWith(Parameterized.class)
+public class GetCurrentPortSpeedTest extends FloodlightTestCase {
+
+    private FloodlightContext cntx;
+    private FloodlightModuleContext fmc;
+    private MockThreadPoolService threadpool;
+    private IOFSwitchService switchService;
+    private StatisticsCollector statsCollector;
+
+    private static final OFFactory factory11 = OFFactories.getFactory(OFVersion.OF_11);
+    private static final OFFactory factory12 = OFFactories.getFactory(OFVersion.OF_12);
+    private static final OFFactory factory13 = OFFactories.getFactory(OFVersion.OF_13);
+    private static final OFFactory factory14 = OFFactories.getFactory(OFVersion.OF_14);
+    private static final OFFactory factory15 = OFFactories.getFactory(OFVersion.OF_15);
+    private static OFFactory inputFactory;
+    private static Long expectedSpeed;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+
+        cntx = new FloodlightContext();
+        fmc = new FloodlightModuleContext();
+
+        // Module loader setup
+        threadpool = new MockThreadPoolService();
+        switchService = getMockSwitchService();
+        statsCollector = new StatisticsCollector();
+
+        fmc.addService(IThreadPoolService.class, threadpool);
+        fmc.addService(IOFSwitchService.class, switchService);
+
+        threadpool.init(fmc);
+        statsCollector.init(fmc);
+        threadpool.startUp(fmc);
+
+    }
+
+    /**
+     * This constructor will be called for each row of test data collection
+     * @param inputFactory
+     * @param expectedSpeed
+     */
+    public GetCurrentPortSpeedTest(OFFactory inputFactory, Long expectedSpeed) {
+        this.inputFactory = inputFactory;
+        this.expectedSpeed = expectedSpeed;
+    }
+
+    /**
+     * A Collection of Junit Test with various "inputFactory" and "expectedSpeed"
+     * @return
+     */
+    @Parameterized.Parameters
+    public static Iterable<Object[]> testData() {
+        return Arrays.asList(new Object[][] {
+            { factory11, 100L },
+            { factory12, 100L },
+            { factory13, 100L },
+            { factory14, 100L },
+            { factory15, 100L },
+        });
+    }
+
+    /**
+     * Test getSpeed() method works with different Openflow versions
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testGetCurrentPortSpeed() throws Exception {
+        IOFSwitch sw = getSwitchByOFVersion(inputFactory);
+        NodePortTuple npt = new NodePortTuple(DatapathId.of(1), OFPort.of(1));
+        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
+        switchMap.put(sw.getId(), sw);
+        getMockSwitchService().setSwitches(switchMap);
+
+        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
+        Long speed = statsSpeedCollector.getSpeed(npt);
+
+        assertEquals(speed, expectedSpeed);
+
+    }
+
+    private IOFSwitch getSwitchByOFVersion(OFFactory inputFactory) {
+        IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
+        reset(sw);
+        expect(sw.getId()).andReturn(DatapathId.of(1L)).anyTimes();
+        expect(sw.getOFFactory()).andReturn(inputFactory).anyTimes();
+
+        OFVersion factoryVersion = inputFactory.getVersion();
+        switch (factoryVersion){
+            case OF_11:
+            case OF_12:
+            case OF_13:
+                expect(sw.getPort(OFPort.of(1))).andReturn(inputFactory.buildPortDesc()
+                        .setPortNo(OFPort.of(1))
+                        .setName("eth1")
+                        .setCurrSpeed(100L)
+                        .build()).anyTimes();
+                replay(sw);
+                break;
+
+            case OF_14:
+            case OF_15:
+                expect(sw.getPort(OFPort.of(1))).andReturn(inputFactory.buildPortDesc()
+                        .setPortNo(OFPort.of(1))
+                        .setName("eth1")
+                        .setProperties(Collections.singletonList(inputFactory.buildPortDescPropEthernet().
+                                setCurrSpeed(100L).
+                                build()))
+                        .build()).anyTimes();
+                replay(sw);
+
+        }
+        return sw;
+    }
+
+}
diff --git a/src/test/java/net/floodlightcontroller/statistics/StatisticsTest.java b/src/test/java/net/floodlightcontroller/statistics/StatisticsTest.java
deleted file mode 100644
index 266aa0bfb..000000000
--- a/src/test/java/net/floodlightcontroller/statistics/StatisticsTest.java
+++ /dev/null
@@ -1,228 +0,0 @@
-package net.floodlightcontroller.statistics;
-
-import net.floodlightcontroller.core.FloodlightContext;
-import net.floodlightcontroller.core.IOFSwitch;
-import net.floodlightcontroller.core.internal.IOFSwitchService;
-import net.floodlightcontroller.core.module.FloodlightModuleContext;
-import net.floodlightcontroller.core.test.MockThreadPoolService;
-import net.floodlightcontroller.core.types.NodePortTuple;
-import net.floodlightcontroller.test.FloodlightTestCase;
-import org.easymock.EasyMock;
-import org.projectfloodlight.openflow.protocol.*;
-import static org.projectfloodlight.openflow.protocol.OFVersion.*;
-import org.projectfloodlight.openflow.types.DatapathId;
-import org.projectfloodlight.openflow.types.OFPort;
-import org.projectfloodlight.openflow.protocol.OFFactories;
-import net.floodlightcontroller.threadpool.IThreadPoolService;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
-import static org.easymock.EasyMock.*;
-import static org.junit.Assert.*;
-import org.junit.*;
-
-/**
- * Created by qing wang on 5/18/17.
- */
-
-public class StatisticsTest extends FloodlightTestCase {
-
-    private FloodlightContext cntx;
-    private FloodlightModuleContext fmc;
-    private MockThreadPoolService threadpool;
-    private IOFSwitchService switchService;
-
-    private static final OFFactory factory10 = OFFactories.getFactory(OFVersion.OF_10);
-    private static final OFFactory factory11 = OFFactories.getFactory(OFVersion.OF_11);
-    private static final OFFactory factory12 = OFFactories.getFactory(OFVersion.OF_12);
-    private static final OFFactory factory13 = OFFactories.getFactory(OFVersion.OF_13);
-    private static final OFFactory factory14 = OFFactories.getFactory(OFVersion.OF_14);
-    private static final OFFactory factory15 = OFFactories.getFactory(OFVersion.OF_15);
-    private StatisticsCollector statsCollector;
-    public static IOFSwitch sw_OF11, sw_OF12, sw_OF13, sw_OF14, sw_OF15;
-    private DatapathId switchDPID = DatapathId.of(1L);
-
-    @Override
-    public void setUp() throws Exception {
-        super.setUp();
-
-        cntx = new FloodlightContext();
-        fmc = new FloodlightModuleContext();
-
-        // Module loader setup
-        threadpool = new MockThreadPoolService();
-        switchService = getMockSwitchService();
-        statsCollector = new StatisticsCollector();
-
-        fmc.addService(IThreadPoolService.class, threadpool);
-        fmc.addService(IOFSwitchService.class, switchService);
-
-        threadpool.init(fmc);
-        statsCollector.init(fmc);
-        threadpool.startUp(fmc);
-
-        // Create OpenFlow11 Mock Switch
-        sw_OF11 = EasyMock.createMock(IOFSwitch.class);
-        sw_OF11 = getSwitchByOFVersion(factory11);
-
-        // Create OpenFlow12 Mock Switch
-        sw_OF12 = EasyMock.createMock(IOFSwitch.class);
-        sw_OF12 = getSwitchByOFVersion(factory12);
-
-        // Create OpenFlow13 Mock Switch
-        sw_OF13 = EasyMock.createMock(IOFSwitch.class);
-        sw_OF13 = getSwitchByOFVersion(factory13);
-
-        // Create OpenFlow14 Mock Switch
-        sw_OF14 = EasyMock.createMock(IOFSwitch.class);
-        sw_OF14 = getSwitchByOFVersion(factory14);
-
-        // Create OpenFlow15 Mock Switch
-        sw_OF15 = EasyMock.createMock(IOFSwitch.class);
-        sw_OF15 = getSwitchByOFVersion(factory15);
-
-    }
-
-    private IOFSwitch getSwitchByOFVersion(OFFactory factory) {
-        IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
-
-        OFVersion openflowVer = factory.getVersion();
-        switch (openflowVer) {
-            case OF_11:
-            case OF_12:
-            case OF_13:
-                reset(sw);
-                expect(sw.getId()).andReturn(switchDPID).anyTimes();
-                expect(sw.getOFFactory()).andReturn(factory).anyTimes();
-                expect(sw.getPort(OFPort.of(1))).andReturn(factory.buildPortDesc()
-                        .setPortNo(OFPort.of(1))
-                        .setName("eth1")
-                        .setCurrSpeed(100L)
-                        .build()).anyTimes();
-                replay(sw);
-                break;
-
-            case OF_14:
-            case OF_15:
-                reset(sw);
-                expect(sw.getId()).andReturn(switchDPID).anyTimes();
-                expect(sw.getOFFactory()).andReturn(factory).anyTimes();
-                expect(sw.getPort(OFPort.of(1))).andReturn(factory.buildPortDesc()
-                        .setPortNo(OFPort.of(1))
-                        .setName("eth1")
-                        .setProperties(Collections.singletonList(factory.buildPortDescPropEthernet().setCurrSpeed(100L).build()))
-                        .build()).anyTimes();
-                replay(sw);
-                break;
-
-            default:
-                break;
-
-        }
-
-        return sw;
-    }
-
-
-    /**
-     * Test getSpeed() method can work with Openflow 1.1
-     *
-     * @throws Exception
-     */
-    @Test
-    public void testGetCurrentPortSpeedOF11() throws Exception {
-        NodePortTuple npt = new NodePortTuple(sw_OF11.getId(), OFPort.of(1));
-
-        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
-        switchMap.put(sw_OF11.getId(), sw_OF11);
-        getMockSwitchService().setSwitches(switchMap);
-
-        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
-
-        long speed = statsSpeedCollector.getSpeed(npt);
-        assertEquals(speed, 100L);
-
-    }
-
-    /**
-     * Test getSpeed() method can work with Openflow 1.2
-     *
-     * @throws Exception
-     */
-    @Test
-    public void testGetCurrentPortSpeedOF12() throws Exception {
-        NodePortTuple npt = new NodePortTuple(sw_OF12.getId(), OFPort.of(1));
-
-        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
-        switchMap.put(sw_OF12.getId(), sw_OF12);
-        getMockSwitchService().setSwitches(switchMap);
-
-        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
-
-        long speed = statsSpeedCollector.getSpeed(npt);
-        assertEquals(speed, 100L);
-
-    }
-
-    /**
-     * Test getSpeed() method can work with Openflow 1.3
-     *
-     * @throws Exception
-     */
-    @Test
-    public void testGetCurrentPortSpeedOF13() throws Exception {
-        NodePortTuple npt = new NodePortTuple(sw_OF13.getId(), OFPort.of(1));
-
-        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
-        switchMap.put(sw_OF13.getId(), sw_OF13);
-        getMockSwitchService().setSwitches(switchMap);
-
-        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
-
-        long speed = statsSpeedCollector.getSpeed(npt);
-        assertEquals(speed, 100L);
-
-    }
-
-    /**
-     * Test getSpeed() method can work with Openflow 1.4
-     *
-     * @throws Exception
-     */
-    @Test
-    public void testGetCurrentPortSpeedOF14() throws Exception {
-        NodePortTuple npt = new NodePortTuple(sw_OF14.getId(), OFPort.of(1));
-
-        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
-        switchMap.put(sw_OF14.getId(), sw_OF14);
-        getMockSwitchService().setSwitches(switchMap);
-
-        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
-
-        long speed = statsSpeedCollector.getSpeed(npt);
-        assertEquals(speed, 100L);
-
-    }
-
-    /**
-     * Test getSpeed() method can work with Openflow 1.5
-     *
-     * @throws Exception
-     */
-    @Test
-    public void testGetCurrentPortSpeedOF15() throws Exception {
-        NodePortTuple npt = new NodePortTuple(sw_OF15.getId(), OFPort.of(1));
-
-        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
-        switchMap.put(sw_OF15.getId(), sw_OF15);
-        getMockSwitchService().setSwitches(switchMap);
-
-        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
-
-        long speed = statsSpeedCollector.getSpeed(npt);
-        assertEquals(speed, 100L);
-
-    }
-
-}
-- 
GitLab