Improvements to the Controller unit tests.

Adapted relevant tests to use OF1.3 in addition to 1.0.

Change-Id: I7069d91fbf5382a2d34d8772722de90b9f51e097
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index 2152875..3550297 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -423,7 +423,7 @@
      */
     protected void removeConnectedSwitch(long dpid) {
         releaseRegistryControl(dpid);
-        connectedSwitches.remove(dpid);
+        OFChannelHandler ch = connectedSwitches.remove(dpid);
         IOFSwitch sw = activeMasterSwitches.remove(dpid);
         if (sw == null) {
             sw = activeEqualSwitches.remove(dpid);
@@ -434,7 +434,9 @@
         }
         evSwitch.updateEventWithFlush(new SwitchEvent(dpid, "disconnected"));
         counters.switchDisconnected.updateCounterWithFlush();
-        addUpdateToQueue(new SwitchUpdate(dpid, SwitchUpdateType.DISCONNECTED));
+        if (ch != null) {
+            addUpdateToQueue(new SwitchUpdate(dpid, SwitchUpdateType.DISCONNECTED));
+        }
     }
 
     /**
@@ -446,7 +448,7 @@
     protected void notifyPortChanged(long dpid, OFPortDesc port,
             PortChangeType changeType) {
         if (port == null || changeType == null) {
-            String msg = String.format("Switch port or changetType must not "
+            String msg = String.format("Switch port or changeType must not "
                     + "be null in port change notification");
             throw new NullPointerException(msg);
         }
diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
index 339c300..54321e5 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
@@ -19,7 +19,6 @@
 
 import static org.easymock.EasyMock.anyObject;
 import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.createStrictMock;
 import static org.easymock.EasyMock.eq;
 import static org.easymock.EasyMock.expect;
@@ -31,17 +30,16 @@
 import static org.easymock.EasyMock.verify;
 import static org.junit.Assert.assertArrayEquals;
 
+import java.io.IOException;
 import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
 import net.floodlightcontroller.core.FloodlightContext;
 import net.floodlightcontroller.core.FloodlightProvider;
 import net.floodlightcontroller.core.IFloodlightProviderService;
-import net.floodlightcontroller.core.IFloodlightProviderService.Role;
 import net.floodlightcontroller.core.IListener;
 import net.floodlightcontroller.core.IListener.Command;
 import net.floodlightcontroller.core.IOFMessageListener;
@@ -66,180 +64,168 @@
 import net.onrc.onos.core.packet.IPacket;
 import net.onrc.onos.core.packet.IPv4;
 import net.onrc.onos.core.registry.IControllerRegistryService;
-import net.onrc.onos.core.registry.StandaloneRegistry;
 
+import org.easymock.EasyMock;
 import org.junit.Before;
 import org.junit.Test;
-import org.projectfloodlight.openflow.protocol.OFDescStatsReply;
 import org.projectfloodlight.openflow.protocol.OFFactories;
 import org.projectfloodlight.openflow.protocol.OFFactory;
-import org.projectfloodlight.openflow.protocol.OFFeaturesReply;
-import org.projectfloodlight.openflow.protocol.OFFlowMod;
 import org.projectfloodlight.openflow.protocol.OFFlowStatsEntry;
+import org.projectfloodlight.openflow.protocol.OFFlowStatsReply;
 import org.projectfloodlight.openflow.protocol.OFPacketIn;
+import org.projectfloodlight.openflow.protocol.OFPacketIn.Builder;
 import org.projectfloodlight.openflow.protocol.OFPacketInReason;
 import org.projectfloodlight.openflow.protocol.OFPortDesc;
-import org.projectfloodlight.openflow.protocol.OFPortState;
 import org.projectfloodlight.openflow.protocol.OFStatsReply;
+import org.projectfloodlight.openflow.protocol.OFStatsReplyFlags;
+import org.projectfloodlight.openflow.protocol.OFStatsType;
 import org.projectfloodlight.openflow.protocol.OFType;
 import org.projectfloodlight.openflow.protocol.OFVersion;
-import org.projectfloodlight.openflow.protocol.ver10.OFStatsReplyFlagsSerializerVer10;
-import org.projectfloodlight.openflow.types.DatapathId;
+import org.projectfloodlight.openflow.protocol.match.Match;
+import org.projectfloodlight.openflow.protocol.match.MatchField;
 import org.projectfloodlight.openflow.types.OFBufferId;
 import org.projectfloodlight.openflow.types.OFPort;
 import org.projectfloodlight.openflow.util.HexString;
 
+import com.google.common.collect.HashMultiset;
+import com.google.common.collect.Multiset;
+
 /**
- * @author David Erickson (daviderickson@cs.stanford.edu)
+ * Unit tests for the Controller class.
  */
 public class ControllerTest extends FloodlightTestCase {
 
     private Controller controller;
-    private MockThreadPoolService tp;
-    protected OFFactory factory10 = OFFactories.getFactory(OFVersion.OF_10);
-    private IPacket testPacket;
-    private OFPacketIn pi;
+    private MockThreadPoolService threadPool;
 
     @Override
     @Before
     public void setUp() throws Exception {
-        doSetUp(Role.MASTER);
-    }
-
-    public void doSetUp(Role role) throws Exception {
         super.setUp();
         FloodlightModuleContext fmc = new FloodlightModuleContext();
 
-        FloodlightProvider cm = new FloodlightProvider();
+        FloodlightProvider floodlightProvider = new FloodlightProvider();
 
-        controller = (Controller) cm.getServiceImpls().get(
+        controller = (Controller) floodlightProvider.getServiceImpls().get(
                 IFloodlightProviderService.class);
         fmc.addService(IFloodlightProviderService.class, controller);
 
         RestApiServer restApi = new RestApiServer();
         fmc.addService(IRestApiService.class, restApi);
 
-        // TODO replace with mock if further testing is needed.
         DebugCounter counterService = new DebugCounter();
         fmc.addService(IDebugCounterService.class, counterService);
 
-        tp = new MockThreadPoolService();
-        fmc.addService(IThreadPoolService.class, tp);
+        threadPool = new MockThreadPoolService();
+        fmc.addService(IThreadPoolService.class, threadPool);
 
-        // Following added by ONOS
-        // TODO replace with mock if further testing is needed.
-        StandaloneRegistry sr = new StandaloneRegistry();
-        fmc.addService(IControllerRegistryService.class, sr);
+        IControllerRegistryService registry =
+                createMock(IControllerRegistryService.class);
+        fmc.addService(IControllerRegistryService.class, registry);
         LinkDiscoveryManager linkDiscovery = new LinkDiscoveryManager();
         fmc.addService(ILinkDiscoveryService.class, linkDiscovery);
 
         restApi.init(fmc);
-        cm.init(fmc);
-        tp.init(fmc);
-        sr.init(fmc);
+        floodlightProvider.init(fmc);
+        threadPool.init(fmc);
         linkDiscovery.init(fmc);
         restApi.startUp(fmc);
-        cm.startUp(fmc);
-        tp.startUp(fmc);
-        sr.startUp(fmc);
-        // linkDiscovery.startUp(fmc);
+        floodlightProvider.startUp(fmc);
+        threadPool.startUp(fmc);
+    }
 
-        testPacket = new Ethernet()
-                .setSourceMACAddress("00:44:33:22:11:00")
-                .setDestinationMACAddress("00:11:22:33:44:55")
-                .setEtherType(Ethernet.TYPE_ARP)
-                .setPayload(
-                        new ARP()
-                                .setHardwareType(ARP.HW_TYPE_ETHERNET)
-                                .setProtocolType(ARP.PROTO_TYPE_IP)
-                                .setHardwareAddressLength((byte) 6)
-                                .setProtocolAddressLength((byte) 4)
-                                .setOpCode(ARP.OP_REPLY)
-                                .setSenderHardwareAddress(
-                                        Ethernet.toMACAddress("00:44:33:22:11:00"))
-                                .setSenderProtocolAddress(
-                                        IPv4.toIPv4AddressBytes("192.168.1.1"))
-                                .setTargetHardwareAddress(
-                                        Ethernet.toMACAddress("00:11:22:33:44:55"))
-                                .setTargetProtocolAddress(
-                                        IPv4.toIPv4AddressBytes("192.168.1.2")));
+    private OFPacketIn buildPacketIn(short inPort, OFVersion version) {
+        IPacket testPacket = new Ethernet()
+        .setSourceMACAddress("00:44:33:22:11:00")
+        .setDestinationMACAddress("00:11:22:33:44:55")
+        .setEtherType(Ethernet.TYPE_ARP)
+        .setPayload(
+                new ARP()
+                        .setHardwareType(ARP.HW_TYPE_ETHERNET)
+                        .setProtocolType(ARP.PROTO_TYPE_IP)
+                        .setHardwareAddressLength((byte) 6)
+                        .setProtocolAddressLength((byte) 4)
+                        .setOpCode(ARP.OP_REPLY)
+                        .setSenderHardwareAddress(
+                                Ethernet.toMACAddress("00:44:33:22:11:00"))
+                        .setSenderProtocolAddress(
+                                IPv4.toIPv4AddressBytes("192.168.1.1"))
+                        .setTargetHardwareAddress(
+                                Ethernet.toMACAddress("00:11:22:33:44:55"))
+                        .setTargetProtocolAddress(
+                                IPv4.toIPv4AddressBytes("192.168.1.2")));
         byte[] testPacketSerialized = testPacket.serialize();
 
-        pi = factory10.buildPacketIn()
-                .setBufferId(OFBufferId.NO_BUFFER)
-                .setInPort(OFPort.of(1))
-                .setData(testPacketSerialized)
-                .setReason(OFPacketInReason.NO_MATCH)
-                .setTotalLen((short) testPacketSerialized.length).build();
+        OFFactory factory = OFFactories.getFactory(version);
 
+        Builder piBuilder = factory.buildPacketIn()
+                .setBufferId(OFBufferId.NO_BUFFER)
+                .setData(testPacketSerialized)
+                .setReason(OFPacketInReason.NO_MATCH);
+
+        if (version == OFVersion.OF_10) {
+            piBuilder.setInPort(OFPort.of(inPort));
+        } else {
+            Match match = factory.buildMatch()
+                        .setExact(MatchField.IN_PORT, OFPort.ofShort(inPort))
+                        .build();
+            piBuilder.setMatch(match);
+        }
+
+        return piBuilder.build();
     }
 
     public Controller getController() {
         return controller;
     }
 
-    protected OFStatsReply getStatisticsReply(int transactionId,
-            int count, boolean moreReplies) {
+    private OFStatsReply getStatisticsReply(int transactionId,
+            int count, boolean moreReplies, OFVersion version) {
+        OFFactory factory = OFFactories.getFactory(version);
+
         List<OFFlowStatsEntry> statistics = new ArrayList<OFFlowStatsEntry>();
         for (int i = 0; i < count; ++i) {
-            statistics.add(factory10.buildFlowStatsEntry().build());
+            statistics.add(factory.buildFlowStatsEntry().build());
         }
         assertEquals(statistics.size(), count);
-        OFStatsReply sr;
+
+        org.projectfloodlight.openflow.protocol.OFStatsReply.Builder
+                statsReplyBuilder = factory.buildFlowStatsReply()
+                    .setXid(transactionId)
+                    .setEntries(statistics);
+
         if (moreReplies) {
-            sr = (factory10.buildFlowStatsReply()
-                    .setXid(transactionId)
-                    .setEntries(statistics)
-                    .setFlags(OFStatsReplyFlagsSerializerVer10.ofWireValue((short) 1))
-                    .build());
-        }
-        else {
-            sr = (factory10.buildFlowStatsReply()
-                    .setXid(transactionId)
-                    .setEntries(statistics).build());
+            statsReplyBuilder.setFlags(
+                    Collections.singleton(OFStatsReplyFlags.REPLY_MORE));
         }
 
-        return sr;
+        return statsReplyBuilder.build();
     }
 
-    private OFDescStatsReply createOFDescStatsReply() {
-        OFDescStatsReply desc = factory10.buildDescStatsReply()
-                .setHwDesc("")
-                .setMfrDesc("")
-                .setDpDesc("")
-                .setMfrDesc("")
-                .setSwDesc("")
-                .setSerialNum("").build();
-        return desc;
-    }
-
-    private OFFeaturesReply createOFFeaturesReply() {
-        OFFeaturesReply fr = factory10.buildFeaturesReply()
-                .setPorts(new ArrayList<OFPortDesc>())
-                .build();
-        return fr;
-
+    private IOFSwitch createMockSwitch(long dpid, OFVersion version) {
+        IOFSwitch sw = createMock(IOFSwitch.class);
+        expect(sw.getId()).andReturn(dpid).anyTimes();
+        expect(sw.getStringId()).andReturn(HexString.toHexString(dpid)).anyTimes();
+        expect(sw.getPorts()).andReturn(
+                Collections.<OFPortDesc>emptySet()).anyTimes();
+        expect(sw.getOFVersion()).andReturn(version).anyTimes();
+        return sw;
     }
 
     /**
-     * Set the mock expectations for sw when sw is passed to addSwitch The same
-     * expectations can be used when a new SwitchSyncRepresentation is created
-     * from the given mocked switch
+     * Set up expectations for the callback ordering methods for mocked
+     * listener classes.
+     *
+     * @param listener the mock listener to set up expectations for
      */
-    protected void setupSwitchForAddSwitch(IOFSwitch sw, long dpid,
-            OFDescStatsReply desc, OFFeaturesReply featuresReply) {
-        String dpidString = HexString.toHexString(dpid);
+    private void setUpNoOrderingRequirements(IListener<OFType> listener) {
+        listener.isCallbackOrderingPostreq(EasyMock.<OFType>anyObject(),
+                anyObject(String.class));
+        expectLastCall().andReturn(false).anyTimes();
 
-        if (desc == null) {
-            desc = createOFDescStatsReply();
-        }
-        if (featuresReply == null) {
-            featuresReply = createOFFeaturesReply();
-            featuresReply.createBuilder().setDatapathId(DatapathId.of(dpid));
-
-        }
-        expect(sw.getId()).andReturn(dpid).anyTimes();
-        expect(sw.getStringId()).andReturn(dpidString).anyTimes();
+        listener.isCallbackOrderingPrereq(EasyMock.<OFType>anyObject(),
+                anyObject(String.class));
+        expectLastCall().andReturn(false).anyTimes();
     }
 
     /**
@@ -253,197 +239,90 @@
         }
     }
 
-    @SuppressWarnings("unchecked")
-    private <T> void setupListenerOrdering(IListener<T> listener) {
-        listener.isCallbackOrderingPostreq((T) anyObject(),
-                anyObject(String.class));
-        expectLastCall().andReturn(false).anyTimes();
-
-        listener.isCallbackOrderingPrereq((T) anyObject(),
-                anyObject(String.class));
-        expectLastCall().andReturn(false).anyTimes();
-    }
-
     /**
-     * Verify that a listener that throws an exception halts further execution,
-     * and verify that the Commands STOP and CONTINUE are honored.
-     *
-     * @throws Exception
-     */
-
-    @Test
-    public void testHandleMessagesNoListeners() throws Exception {
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        expect(sw.getId()).andReturn(0L).anyTimes();
-        expect(sw.getStringId()).andReturn("00:00:00:00:00:00:00").anyTimes();
-        expect(sw.getOFVersion()).andReturn(OFVersion.OF_10).anyTimes();
-        replay(sw);
-        controller.handleMessage(sw, pi, null);
-        verify(sw);
-    }
-
-    /**
-     * Test message dispatching to OFMessageListeners. Test ordering of
-     * listeners for different types (we do this implicitly by using STOP and
-     * CONTINUE and making sure the processing stops at the right place) Verify
-     * that a listener that throws an exception halts further execution, and
-     * verify that the Commands STOP and CONTINUE are honored.
+     * Verifies that a listener that throws an exception halts further
+     * execution, and verifies that the Commands STOP and CONTINUE are honored.
      *
      * @throws Exception
      */
     @Test
     public void testHandleMessages() throws Exception {
+        Controller controller = getController();
         controller.removeOFMessageListeners(OFType.PACKET_IN);
 
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        expect(sw.getId()).andReturn(0L).anyTimes();
-        expect(sw.getStringId()).andReturn("00:00:00:00:00:00:00").anyTimes();
-        expect(sw.getOFVersion()).andReturn(OFVersion.OF_10).anyTimes();
-        // Setup listener orderings
+        // Just test 1.0 here. We test with 1.3 in testHandleMessageWithContext.
+        OFVersion version = OFVersion.OF_10;
+
+        IOFSwitch sw = createMockSwitch(1L, version);
+        OFPacketIn pi = buildPacketIn((short) 1, version);
+
         IOFMessageListener test1 = createMock(IOFMessageListener.class);
         expect(test1.getName()).andReturn("test1").anyTimes();
-        setupListenerOrdering(test1);
-
+        setUpNoOrderingRequirements(test1);
+        expect(test1.receive(eq(sw), eq(pi), isA(FloodlightContext.class)))
+                .andThrow(new RuntimeException(
+                "This is NOT an error! We are testing exception catching."));
         IOFMessageListener test2 = createMock(IOFMessageListener.class);
         expect(test2.getName()).andReturn("test2").anyTimes();
-        // using a postreq and a prereq ordering here
-        expect(test2.isCallbackOrderingPrereq(OFType.PACKET_IN, "test1"))
-                .andReturn(true).atLeastOnce();
-        expect(test2.isCallbackOrderingPostreq(OFType.FLOW_MOD, "test1"))
-                .andReturn(true).atLeastOnce();
-        setupListenerOrdering(test2);
+        setUpNoOrderingRequirements(test2);
+        // expect no calls to test2.receive() since test1.receive() threw an
+        // exception
 
-        IOFMessageListener test3 = createMock(IOFMessageListener.class);
-        expect(test3.getName()).andReturn("test3").anyTimes();
-        expect(test3.isCallbackOrderingPrereq((OFType) anyObject(), eq("test1")))
-                .andReturn(true).atLeastOnce();
-        expect(test3.isCallbackOrderingPrereq((OFType) anyObject(), eq("test2")))
-                .andReturn(true).atLeastOnce();
-        setupListenerOrdering(test3);
-
-        // Ordering: PacketIn: test1 -> test2 -> test3
-        // FlowMod: test2 -> test1
-        replay(test1, test2, test3);
+        replay(test1, test2, sw);
         controller.addOFMessageListener(OFType.PACKET_IN, test1);
-        controller.addOFMessageListener(OFType.PACKET_IN, test3);
         controller.addOFMessageListener(OFType.PACKET_IN, test2);
-        controller.addOFMessageListener(OFType.FLOW_MOD, test1);
-        controller.addOFMessageListener(OFType.FLOW_MOD, test2);
-        verify(test1);
-        verify(test2);
-        verify(test3);
-
-        replay(sw);
-
-        // ------------------
-        // Test PacketIn handling: all listeners return CONTINUE
-        reset(test1, test2, test3);
-        expect(test1.receive(eq(sw), eq(pi), isA(FloodlightContext.class)))
-                .andReturn(Command.CONTINUE);
-        expect(test2.receive(eq(sw), eq(pi), isA(FloodlightContext.class)))
-                .andReturn(Command.CONTINUE);
-        expect(test3.receive(eq(sw), eq(pi), isA(FloodlightContext.class)))
-                .andReturn(Command.CONTINUE);
-        replay(test1, test2, test3);
-        controller.handleMessage(sw, pi, null);
-        verify(test1);
-        verify(test2);
-        verify(test3);
-
-        // ------------------
-        // Test PacketIn handling: with a thrown exception.
-        reset(test1, test2, test3);
-        expect(test1.receive(eq(sw), eq(pi), isA(FloodlightContext.class)))
-                .andReturn(Command.CONTINUE);
-        expect(test2.receive(eq(sw), eq(pi), isA(FloodlightContext.class)))
-                .andThrow(new RuntimeException("This is NOT an error! We " +
-                        "are testing exception catching."));
-        // expect no calls to test3.receive() since test2.receive throws
-        // an exception
-        replay(test1, test2, test3);
         try {
             controller.handleMessage(sw, pi, null);
-            fail("Expected exception was not thrown!");
         } catch (RuntimeException e) {
-            assertTrue("The caught exception was not the expected one",
-                    e.getMessage().startsWith("This is NOT an error!"));
+            assertEquals(e.getMessage().startsWith("This is NOT an error!"), true);
         }
-        verify(test1);
-        verify(test2);
-        verify(test3);
 
-        // ------------------
-        // Test PacketIn handling: test1 return Command.STOP
-        reset(test1, test2, test3);
+        verify(test1, test2);
+
+        // verify STOP works
+        reset(test1, test2);
         expect(test1.receive(eq(sw), eq(pi), isA(FloodlightContext.class)))
                 .andReturn(Command.STOP);
-        // expect no calls to test3.receive() and test2.receive since
-        // test1.receive returns STOP
-        replay(test1, test2, test3);
+
+        replay(test1, test2);
         controller.handleMessage(sw, pi, null);
-        verify(test1);
-        verify(test2);
-        verify(test3);
-
-        OFFlowMod fm = factory10.buildFlowAdd().build();
-
-        // ------------------
-        // Test FlowMod handling: all listeners return CONTINUE
-        reset(test1, test2, test3);
-        expect(test1.receive(eq(sw), eq(fm), isA(FloodlightContext.class)))
-                .andReturn(Command.CONTINUE);
-        expect(test2.receive(eq(sw), eq(fm), isA(FloodlightContext.class)))
-                .andReturn(Command.CONTINUE);
-        // test3 is not a listener for FlowMod
-        replay(test1, test2, test3);
-        controller.handleMessage(sw, fm, null);
-        verify(test1);
-        verify(test2);
-        verify(test3);
-
-        // ------------------
-        // Test FlowMod handling: test2 (first listener) return STOP
-        reset(test1, test2, test3);
-        expect(test2.receive(eq(sw), eq(fm), isA(FloodlightContext.class)))
-                .andReturn(Command.STOP);
-        // test2 will not be called
-        // test3 is not a listener for FlowMod
-        replay(test1, test2, test3);
-        controller.handleMessage(sw, fm, null);
-        verify(test1);
-        verify(test2);
-        verify(test3);
-
-        verify(sw);
+        verify(test1, test2);
     }
 
+
+    /**
+     * Tests message handling when providing a FloodlightContext object.
+     * Checks that the correct values are set in the context before the message
+     * is dispatched to the listeners.
+     *
+     * @throws Exception
+     */
     @Test
     public void testHandleMessageWithContext() throws Exception {
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        expect(sw.getId()).andReturn(0L).anyTimes();
-        expect(sw.getStringId()).andReturn("00:00:00:00:00:00:00").anyTimes();
-        expect(sw.getOFVersion()).andReturn(OFVersion.OF_10).anyTimes();
+        doTestHandleMessageWithContext(OFVersion.OF_10);
+        doTestHandleMessageWithContext(OFVersion.OF_13);
+    }
+
+    private void doTestHandleMessageWithContext(OFVersion version)
+                                                throws IOException {
+        controller.messageListeners.clear();
+
+        short inPort = (short) 1;
+        FloodlightContext cntx = new FloodlightContext();
+
+        IOFSwitch sw = createMockSwitch(1L, version);
+        OFPacketIn pi = buildPacketIn(inPort, version);
 
         IOFMessageListener test1 = createMock(IOFMessageListener.class);
         expect(test1.getName()).andReturn("test1").anyTimes();
-        expect(test1.isCallbackOrderingPrereq((OFType) anyObject(),
-                (String) anyObject()))
-                .andReturn(false).anyTimes();
-        expect(test1.isCallbackOrderingPostreq((OFType) anyObject(),
-                (String) anyObject()))
-                .andReturn(false).anyTimes();
-        FloodlightContext cntx = new FloodlightContext();
+        setUpNoOrderingRequirements(test1);
+
         expect(test1.receive(same(sw), same(pi), same(cntx)))
                 .andReturn(Command.CONTINUE);
 
         IOFMessageListener test2 = createMock(IOFMessageListener.class);
         expect(test2.getName()).andReturn("test2").anyTimes();
-        expect(test2.isCallbackOrderingPrereq((OFType) anyObject(),
-                (String) anyObject()))
-                .andReturn(false).anyTimes();
-        expect(test2.isCallbackOrderingPostreq((OFType) anyObject(),
-                (String) anyObject()))
-                .andReturn(false).anyTimes();
+        setUpNoOrderingRequirements(test2);
         // test2 will not receive any message!
 
         replay(test1, test2, sw);
@@ -454,9 +333,18 @@
 
         Ethernet eth = IFloodlightProviderService.bcStore.get(cntx,
                 IFloodlightProviderService.CONTEXT_PI_PAYLOAD);
-        assertArrayEquals(testPacket.serialize(), eth.serialize());
+        assertArrayEquals(pi.getData(), eth.serialize());
+
+        short actualInPort = (short) cntx.getStorage()
+                .get(IFloodlightProviderService.CONTEXT_PI_INPORT);
+        assertEquals(inPort, actualInPort);
     }
 
+    /**
+     * Task used to get the value from a Future in a different thread.
+     *
+     * @param <E> the type of value returned by the Future
+     */
     public class FutureFetcher<E> implements Runnable {
         public E value;
         public Future<E> future;
@@ -475,6 +363,8 @@
         }
 
         /**
+         * Gets the value from the Future.
+         *
          * @return the value
          */
         public E getValue() {
@@ -482,6 +372,8 @@
         }
 
         /**
+         * Gets the Future.
+         *
          * @return the future
          */
         public Future<E> getFuture() {
@@ -490,51 +382,76 @@
     }
 
     /**
+     * Test that the OFStatisticsFuture correctly returns statistics replies.
+     *
      * @throws Exception
      */
     @Test
     public void testOFStatisticsFuture() throws Exception {
+        // Test both OF 1.0 and 1.3 stats messages
+        doTestOFStatisticsFuture(OFVersion.OF_10);
+        doTestOFStatisticsFuture(OFVersion.OF_13);
+    }
+
+    private void doTestOFStatisticsFuture(OFVersion version) throws Exception {
         // Test for a single stats reply
-        OFSwitchImplBase sw = createMock(OFSwitchImplBase.class);
+        IOFSwitch sw = createMock(IOFSwitch.class);
         sw.cancelStatisticsReply(1);
-        OFStatisticsFuture sf = new OFStatisticsFuture(tp, sw, 1);
+        OFStatisticsFuture sf = new OFStatisticsFuture(threadPool, sw, 1);
 
         replay(sw);
         List<OFStatsReply> stats;
-        FutureFetcher<List<OFStatsReply>> ff = new FutureFetcher<List<OFStatsReply>>(sf);
+        FutureFetcher<List<OFStatsReply>> ff =
+                new FutureFetcher<List<OFStatsReply>>(sf);
         Thread t = new Thread(ff);
         t.start();
-        sf.deliverFuture(sw, getStatisticsReply(1, 10, false));
+        sf.deliverFuture(sw, getStatisticsReply(1, 10, false, version));
 
         t.join();
         stats = ff.getValue();
         verify(sw);
-        // TODO: temporary fix: size = 1 ?
+
+        // We expect 1 flow stats reply message containing 10 flow stats entries
         assertEquals(1, stats.size());
+        assertEquals(OFStatsType.FLOW, stats.get(0).getStatsType());
+        assertEquals(Collections.EMPTY_SET, stats.get(0).getFlags());
+        OFFlowStatsReply flowStatsReply = (OFFlowStatsReply) stats.get(0);
+        assertEquals(10, flowStatsReply.getEntries().size());
 
         // Test multiple stats replies
         reset(sw);
         sw.cancelStatisticsReply(1);
 
-        sf = new OFStatisticsFuture(tp, sw, 1);
+        sf = new OFStatisticsFuture(threadPool, sw, 1);
 
         replay(sw);
         ff = new FutureFetcher<List<OFStatsReply>>(sf);
         t = new Thread(ff);
         t.start();
-        sf.deliverFuture(sw, getStatisticsReply(1, 10, true));
-        sf.deliverFuture(sw, getStatisticsReply(1, 5, false));
+        sf.deliverFuture(sw, getStatisticsReply(1, 10, true, version));
+        sf.deliverFuture(sw, getStatisticsReply(1, 5, false, version));
         t.join();
 
         stats = sf.get();
         verify(sw);
-        // TODO: temporary fix: size = 2 ?
+
+        // We expect 2 flow stats replies. The first one has 10 entries and has
+        // the REPLY_MORE flag set, and the second one has 5 entries with no flag.
         assertEquals(2, stats.size());
+        assertEquals(OFStatsType.FLOW, stats.get(0).getStatsType());
+        assertEquals(Collections.singleton(OFStatsReplyFlags.REPLY_MORE),
+                stats.get(0).getFlags());
+        assertEquals(OFStatsType.FLOW, stats.get(1).getStatsType());
+        assertEquals(Collections.EMPTY_SET, stats.get(1).getFlags());
+        OFFlowStatsReply flowStatsReply2 = (OFFlowStatsReply) stats.get(0);
+        assertEquals(10, flowStatsReply2.getEntries().size());
+        OFFlowStatsReply flowStatsReply3 = (OFFlowStatsReply) stats.get(1);
+        assertEquals(5, flowStatsReply3.getEntries().size());
 
         // Test cancellation
         reset(sw);
         sw.cancelStatisticsReply(1);
-        sf = new OFStatisticsFuture(tp, sw, 1);
+        sf = new OFStatisticsFuture(threadPool, sw, 1);
 
         replay(sw);
         ff = new FutureFetcher<List<OFStatsReply>>(sf);
@@ -550,7 +467,7 @@
         // Test self timeout
         reset(sw);
         sw.cancelStatisticsReply(1);
-        sf = new OFStatisticsFuture(tp, sw, 1, 75, TimeUnit.MILLISECONDS);
+        sf = new OFStatisticsFuture(threadPool, sw, 1, 75, TimeUnit.MILLISECONDS);
 
         replay(sw);
         ff = new FutureFetcher<List<OFStatsReply>>(sf);
@@ -564,170 +481,199 @@
     }
 
     /**
-     * Test switchActivated for a new switch, i.e., a switch that was not
-     * previously known to the controller cluser. We expect that all flow mods
-     * are cleared and we expect a switchAdded
+     * Test adding a new switch in the MASTER role.
+     * We expect a switchActivatedMaster event fired to the switch listeners.
      */
     @Test
-    public void testNewSwitchActivated() throws Exception {
+    public void testSwitchActivatedMaster() throws Exception {
+        long dpid = 1L;
+
         controller.setAlwaysClearFlowsOnSwActivate(false);
         controller.setAlwaysClearFlowsOnSwAdd(false);
 
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        expect(sw.getPorts()).andReturn(new HashSet<OFPortDesc>()).anyTimes();
-        setupSwitchForAddSwitch(sw, 0L, null, null);
+        // Create a 1.0 switch. There's no difference between 1.0 and 1.3 here.
+        IOFSwitch sw = createMockSwitch(dpid, OFVersion.OF_10);
 
         // strict mock. Order of events matters!
         IOFSwitchListener listener = createStrictMock(IOFSwitchListener.class);
-        listener.switchActivatedMaster(0L);
+        listener.switchActivatedMaster(dpid);
         expectLastCall().once();
         replay(listener);
         controller.addOFSwitchListener(listener);
 
         replay(sw);
-        controller.addConnectedSwitch(0L, new OFChannelHandler(controller));
-        controller.addActivatedMasterSwitch(0L, sw);
+        controller.addConnectedSwitch(dpid, new OFChannelHandler(controller));
+        controller.addActivatedMasterSwitch(dpid, sw);
         verify(sw);
-        assertEquals(sw, controller.getMasterSwitch(0L));
+        assertEquals(sw, controller.getMasterSwitch(dpid));
         controller.processUpdateQueueForTesting();
         verify(listener);
     }
 
     /**
-     * Test switchActivated for a new switch while in equal: a no-op
+     * Test adding a new switch in the EQUAL role.
+     * We expect a switchActivatedEqual event fired to the switch listeners.
      */
     @Test
-    public void testNewSwitchActivatedWhileSlave() throws Exception {
-        doSetUp(Role.EQUAL);
-        IOFSwitch sw = createMock(IOFSwitch.class);
+    public void testSwitchActivatedEqual() throws Exception {
+        long dpid = 1L;
+        // Create a 1.0 switch. There's no difference between 1.0 and 1.3 here.
+        IOFSwitch sw = createMockSwitch(dpid, OFVersion.OF_10);
 
-        IOFSwitchListener listener = createMock(IOFSwitchListener.class);
+        IOFSwitchListener listener = createStrictMock(IOFSwitchListener.class);
         controller.addOFSwitchListener(listener);
 
+        listener.switchActivatedEqual(dpid);
         replay(sw, listener); // nothing recorded
-        controller.addConnectedSwitch(0L, new OFChannelHandler(controller));
-        controller.addActivatedEqualSwitch(0L, sw);
+        controller.addConnectedSwitch(dpid, new OFChannelHandler(controller));
+        controller.addActivatedEqualSwitch(dpid, sw);
         verify(sw);
+        controller.processUpdateQueueForTesting();
         verify(listener);
     }
 
     /**
-     * Disconnect a switch. normal program flow
+     * Disconnect a switch which was connected in the MASTER role.
+     * Check the correct cleanup methods are called on the switch, that the
+     * switch is removed from the Controller data structures, and that the
+     * correct switch listener method is called.
      */
     @Test
-    private void doTestSwitchConnectReconnect(boolean reconnect)
-            throws Exception {
-        IOFSwitch sw = doActivateNewSwitch(1L, null, null);
-        expect(sw.getId()).andReturn(1L).anyTimes();
-        expect(sw.getStringId()).andReturn(HexString.toHexString(1L)).anyTimes();
-        sw.setConnected(false);
+    public void testDisconnectMasterSwitch() {
+        long dpid = 1L;
+
+        // Create a 1.0 switch. There's no difference between 1.0 and 1.3 here.
+        IOFSwitch sw = createMockSwitch(dpid, OFVersion.OF_10);
+        replay(sw);
+
+        // Add switch to controller as MASTER
+        controller.addConnectedSwitch(dpid, new OFChannelHandler(controller));
+        controller.addActivatedMasterSwitch(dpid, sw);
+
+        // Check the switch is in the controller's lists
+        assertEquals(sw, controller.getMasterSwitch(dpid));
+
+        IOFSwitchListener listener = createStrictMock(IOFSwitchListener.class);
+        listener.switchDisconnected(dpid);
         expectLastCall().once();
+        replay(listener);
+        // Drain the update queue
+        controller.processUpdateQueueForTesting();
+        // Add the listener
+        controller.addOFSwitchListener(listener);
+
+        reset(sw);
         sw.cancelAllStatisticsReplies();
         expectLastCall().once();
-        IOFSwitchListener listener = createMock(IOFSwitchListener.class);
-        listener.switchDisconnected(1L);
+        sw.setConnected(false);
         expectLastCall().once();
-        controller.addOFSwitchListener(listener);
-        replay(sw, listener);
-        controller.removeConnectedSwitch(1L);
+        replay(sw);
+
+        // Disconnect switch
+        controller.removeConnectedSwitch(dpid);
+
+        assertNull(controller.getMasterSwitch(dpid));
+
         controller.processUpdateQueueForTesting();
-        verify(sw, listener);
-
-        assertNull(controller.getSwitch(1L));
-        if (reconnect) {
-            controller.removeOFSwitchListener(listener);
-            sw = doActivateOldSwitch(1L, null, null);
-        }
-    }
-
-    @Test
-    public void testSwitchDisconnected() throws Exception {
-        doTestSwitchConnectReconnect(false);
+        verify(listener, sw);
     }
 
     /**
-     * Disconnect a switch and reconnect, verify no clearAllFlowmods()
+     * Disconnect a switch which was connected in the EQUAL role.
+     * Check the correct cleanup methods are called on the switch, that the
+     * switch is removed from the Controller data structures, and that the
+     * correct switch listener method is called.
      */
     @Test
-    public void testSwitchReconnect() throws Exception {
-        doTestSwitchConnectReconnect(true);
+    public void testDisconnectEqualSwitch() {
+        long dpid = 1L;
+
+        // Create a 1.0 switch. There's no difference between 1.0 and 1.3 here.
+        IOFSwitch sw = createMockSwitch(dpid, OFVersion.OF_10);
+        replay(sw);
+
+        // Add switch to controller as EQUAL
+        controller.addConnectedSwitch(dpid, new OFChannelHandler(controller));
+        controller.addActivatedEqualSwitch(dpid, sw);
+
+        // Check the switch is in the controller's lists
+        assertEquals(sw, controller.getEqualSwitch(dpid));
+
+        IOFSwitchListener listener = createStrictMock(IOFSwitchListener.class);
+        listener.switchDisconnected(dpid);
+        expectLastCall().once();
+        replay(listener);
+        // Drain the update queue
+        controller.processUpdateQueueForTesting();
+        // Add the listener
+        controller.addOFSwitchListener(listener);
+
+        reset(sw);
+        sw.cancelAllStatisticsReplies();
+        expectLastCall().once();
+        sw.setConnected(false);
+        expectLastCall().once();
+        replay(sw);
+
+        // Disconnect switch
+        controller.removeConnectedSwitch(dpid);
+
+        assertNull(controller.getEqualSwitch(dpid));
+
+        controller.processUpdateQueueForTesting();
+        verify(listener, sw);
     }
 
-    /* /**
-     * Remove a nonexisting switch. should be ignored
+    /**
+     * Remove a nonexistent switch.
+     * Check the switch listeners don't receive a disconnected event.
      */
     @Test
     public void testNonexistingSwitchDisconnected() throws Exception {
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        expect(sw.getId()).andReturn(1L).anyTimes();
-        expect(sw.getStringId()).andReturn(HexString.toHexString(1L)).anyTimes();
-        IOFSwitchListener listener = createMock(IOFSwitchListener.class);
+        long dpid = 1L;
+        // Create a 1.0 switch. There's no difference between 1.0 and 1.3 here.
+        IOFSwitch sw = createMockSwitch(1L, OFVersion.OF_10);
+        IOFSwitchListener listener = createStrictMock(IOFSwitchListener.class);
         controller.addOFSwitchListener(listener);
         replay(sw, listener);
-        controller.removeConnectedSwitch(sw.getId());
-        // controller.processUpdateQueueForTesting();
+        controller.removeConnectedSwitch(dpid);
+        controller.processUpdateQueueForTesting();
         verify(sw, listener);
 
-        assertNull(controller.getSwitch(1L));
+        assertNull(controller.getSwitch(dpid));
     }
 
     /**
-     * Try to activate a switch that's already active (which can happen if two
-     * different switches have the same DPIP or if a switch reconnects while the
-     * old TCP connection is still alive
+     * Try to connect a switch with the same DPID as an already active switch.
+     * This could happen if two different switches have the same DPID or if a
+     * switch reconnects while the old TCP connection is still alive.
+     * Check that {@link Controller#addConnectedSwitch(long, OFChannelHandler)}
+     * returns false and that no modification is made to the connectedSwitches
+     * map.
      */
-    // TODO: I do not if it represents the expected behaviour
     @Test
-    public void testSwitchActivatedWithAlreadyActiveSwitch() throws Exception {
-        OFDescStatsReply oldDesc = createOFDescStatsReply();
-        oldDesc.createBuilder().setDpDesc("Ye Olde Switch");
-        OFDescStatsReply newDesc = createOFDescStatsReply();
-        oldDesc.createBuilder().setDpDesc("The new Switch");
-        OFFeaturesReply featuresReply = createOFFeaturesReply();
+    public void testConnectSwitchWithSameDpid() {
+        long dpid = 1L;
 
         // Setup: add a switch to the controller
-        IOFSwitch oldsw = createMock(IOFSwitch.class);
-        setupSwitchForAddSwitch(oldsw, 0L, oldDesc, featuresReply);
-        expect(oldsw.getPorts()).andReturn(new HashSet<OFPortDesc>()).anyTimes();
-        // oldsw.clearAllFlowMods();
-        // expectLastCall().once();
+        // Create a 1.0 switch. There's no difference between 1.0 and 1.3 here.
+        IOFSwitch oldsw = createMockSwitch(dpid, OFVersion.OF_10);
+
         replay(oldsw);
-        controller.addConnectedSwitch(oldsw.getId(), new OFChannelHandler(controller));
-        controller.addActivatedMasterSwitch(oldsw.getId(), oldsw);
-        verify(oldsw);
-        // drain the queue, we don't care what's in it
-        controller.processUpdateQueueForTesting();
-        assertEquals(oldsw, controller.getSwitch(0L));
+        OFChannelHandler oldChannel = new OFChannelHandler(controller);
 
-        // Now the actual test: add a new switch with the same dpid to
-        // the controller
-        reset(oldsw);
-        expect(oldsw.getId()).andReturn(0L).anyTimes();
-        // oldsw.cancelAllStatisticsReplies();
-        // expectLastCall().once();
-        // oldsw.disconnectOutputStream();
-        // expectLastCall().once();
+        assertTrue(controller.addConnectedSwitch(dpid, oldChannel));
+        controller.addActivatedMasterSwitch(dpid, oldsw);
 
-        IOFSwitch newsw = createMock(IOFSwitch.class);
-        setupSwitchForAddSwitch(newsw, 0L, newDesc, featuresReply);
-        // newsw.clearAllFlowMods();
-        // expectLastCall().once();
+        assertSame(oldChannel, controller.connectedSwitches.get(dpid));
+        assertEquals(oldsw, controller.getSwitch(dpid));
 
-        // Strict mock. We need to get the removed notification before the
-        // add notification
-        IOFSwitchListener listener = createStrictMock(IOFSwitchListener.class);
-        // listener.switchDisconnected(0L);
-        // listener.switchActivatedMaster(0L);
-        replay(listener);
-        controller.addOFSwitchListener(listener);
-
-        replay(newsw, oldsw);
-        controller.addActivatedMasterSwitch(0L, newsw);
-        verify(newsw, oldsw);
-
-        assertEquals(oldsw, controller.getSwitch(0L));
-        controller.processUpdateQueueForTesting();
-        verify(listener);
+        // Now try to add a new switch (OFChannelHandler) with the same dpid to
+        // the controller. #addConnectedSwitch should return false and no
+        // modification should be made to the connected switches map.
+        assertFalse(controller.addConnectedSwitch(
+                dpid, new OFChannelHandler(controller)));
+        assertSame(oldChannel, controller.connectedSwitches.get(dpid));
     }
 
     /**
@@ -736,9 +682,10 @@
      */
     @Test
     public void testRemoveActiveSwitch() {
-        IOFSwitch sw = createNiceMock(IOFSwitch.class);
-        expect(sw.getPorts()).andReturn(new ArrayList<OFPortDesc>()).anyTimes();
-        setupSwitchForAddSwitch(sw, 1L, null, null);
+        long dpid = 1L;
+
+        // Create a 1.0 switch. There's no difference between 1.0 and 1.3 here.
+        IOFSwitch sw = createMockSwitch(dpid, OFVersion.OF_10);
         replay(sw);
         controller.addConnectedSwitch(1L, new OFChannelHandler(controller));
         controller.addActivatedMasterSwitch(1L, sw);
@@ -746,294 +693,193 @@
         controller.getAllSwitchDpids().remove(1L);
         assertEquals(sw, getController().getSwitch(1L));
         verify(sw);
-        // we don't care for updates. drain queue.
-        controller.processUpdateQueueForTesting();
     }
 
     /**
-     * Create and activate a switch, either completely new or reconnected The
-     * mocked switch instance will be returned. It wil be reset.
+     * Implementation of an IOFSwitchListener that counts the number of events
+     * it has received of each different event type.
      */
-    private IOFSwitch doActivateSwitchInt(long dpid,
-            OFDescStatsReply desc,
-            OFFeaturesReply featuresReply,
-            boolean clearFlows)
-            throws Exception {
-        controller.setAlwaysClearFlowsOnSwActivate(false);
+    private static class DummySwitchListener implements IOFSwitchListener {
+        // The multisets will record a count of the number of times each update
+        // has been seen by the listener
+        private Multiset<SwitchUpdateType> updateCount = HashMultiset.create();
+        private Multiset<PortChangeType> portUpdateCount = HashMultiset.create();
 
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        if (featuresReply == null) {
-            featuresReply = createOFFeaturesReply();
-            featuresReply.createBuilder().setDatapathId(DatapathId.of(dpid));
+        /**
+         * Gets the number of times a switch update event of the specified type
+         * has been received.
+         *
+         * @param type SwitchUpdateType to get the count for
+         * @return number of times the event has been received
+         */
+        public int getSwitchUpdateCount(SwitchUpdateType type) {
+            return updateCount.count(type);
         }
-        if (desc == null) {
-            desc = createOFDescStatsReply();
-        }
-        setupSwitchForAddSwitch(sw, dpid, desc, featuresReply);
-        if (clearFlows) {
-            sw.clearAllFlowMods();
-            expectLastCall().once();
-        }
-        expect(sw.getPorts()).andReturn(new HashSet<OFPortDesc>()).anyTimes();
 
-        replay(sw);
-        controller.addConnectedSwitch(dpid, new OFChannelHandler(controller));
-        controller.addActivatedMasterSwitch(dpid, sw);
-        verify(sw);
-        assertEquals(sw, controller.getSwitch(dpid));
-        // drain updates and ignore
-        controller.processUpdateQueueForTesting();
+        /**
+         * Gets the number of times a port update event of the specified type
+         * has been received.
+         *
+         * @param type PortChangeType to get the count for
+         * @return number of times the event has been received
+         */
+        public int getPortUpdateCount(PortChangeType type) {
+            return portUpdateCount.count(type);
+        }
 
-        // SwitchSyncRepresentation storedSwitch = storeClient.getValue(dpid);
-        // assertEquals(featuresReply, storedSwitch.getFeaturesReply());
-        // assertEquals(desc, storedSwitch.getDescription());
-        reset(sw);
-        return sw;
+        @Override
+        public String getName() {
+            return "dummy";
+        }
+
+        @Override
+        public synchronized void switchActivatedMaster(long swId) {
+            updateCount.add(SwitchUpdateType.ACTIVATED_MASTER);
+            notifyAll();
+        }
+
+        @Override
+        public synchronized void switchActivatedEqual(long swId) {
+            updateCount.add(SwitchUpdateType.ACTIVATED_EQUAL);
+            notifyAll();
+        }
+
+        @Override
+        public synchronized void switchMasterToEqual(long swId) {
+            updateCount.add(SwitchUpdateType.MASTER_TO_EQUAL);
+            notifyAll();
+        }
+
+        @Override
+        public synchronized void switchEqualToMaster(long swId) {
+            updateCount.add(SwitchUpdateType.EQUAL_TO_MASTER);
+            notifyAll();
+        }
+
+        @Override
+        public synchronized void switchDisconnected(long swId) {
+            updateCount.add(SwitchUpdateType.DISCONNECTED);
+            notifyAll();
+        }
+
+        @Override
+        public synchronized void switchPortChanged(long swId, OFPortDesc port,
+                PortChangeType changeType) {
+            portUpdateCount.add(changeType);
+            notifyAll();
+        }
     }
 
     /**
-     * Create and activate a new switch with the given dpid, features reply and
-     * description. If description and/or features reply are null we'll allocate
-     * the default one The mocked switch instance will be returned. It wil be
-     * reset.
+     * Tests that updates sent into the Controller updates queue are dispatched
+     * to the listeners correctly.
+     *
+     * @throws InterruptedException
      */
-    private IOFSwitch doActivateNewSwitch(long dpid,
-            OFDescStatsReply desc,
-            OFFeaturesReply featuresReply)
-            throws Exception {
-        return doActivateSwitchInt(dpid, desc, featuresReply, false);
-    }
-
-    /**
-     * Create and activate a switch that's just been disconnected. The mocked
-     * switch instance will be returned. It wil be reset.
-     */
-    private IOFSwitch doActivateOldSwitch(long dpid,
-            OFDescStatsReply desc,
-            OFFeaturesReply featuresReply)
-            throws Exception {
-        return doActivateSwitchInt(dpid, desc, featuresReply, false);
-    }
-
     @Test
-    public void testUpdateQueue() throws Exception {
-        class DummySwitchListener implements IOFSwitchListener {
-            public int nAddedMaster;
-            public int nAddedEqual;
-            public int nDisconnected;
-            public int nPortChanged;
-            public int nPortAdded;
-            public int nPortDeleted;
+    public void testUpdateQueue() throws InterruptedException {
+        // No difference between OpenFlow versions here
+        OFVersion version = OFVersion.OF_10;
+        OFPortDesc port = OFFactories.getFactory(version)
+                .buildPortDesc().build();
+        long dpid = 1L;
 
-            public DummySwitchListener() {
-                nAddedMaster = 0;
-                nAddedEqual = 0;
-                nDisconnected = 0;
-                nPortChanged = 0;
-                nPortAdded = 0;
-                nPortDeleted = 0;
-            }
-
-            @Override
-            public String getName() {
-                return "dummy";
-            }
-
-            @Override
-            public void switchActivatedMaster(long swId) {
-                nAddedMaster++;
-                notifyAll();
-
-            }
-
-            @Override
-            public void switchActivatedEqual(long swId) {
-                nAddedEqual++;
-                notifyAll();
-
-            }
-
-            @Override
-            public void switchMasterToEqual(long swId) {
-                // TODO Auto-generated method stub
-
-            }
-
-            @Override
-            public void switchEqualToMaster(long swId) {
-                // TODO Auto-generated method stub
-            }
-
-            @Override
-            public void switchDisconnected(long swId) {
-                nDisconnected++;
-                notifyAll();
-
-            }
-
-            @Override
-            public void switchPortChanged(long swId, OFPortDesc port,
-                    PortChangeType changeType) {
-                switch (changeType) {
-                case ADD:
-                    nPortAdded++;
-                    notifyAll();
-                    break;
-                case DELETE:
-                    nPortDeleted++;
-                    notifyAll();
-                    break;
-
-                case OTHER_UPDATE:
-                    nPortChanged++;
-                    notifyAll();
-                    break;
-
-                }
-            }
-        }
         DummySwitchListener switchListener = new DummySwitchListener();
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        expect(sw.getId()).andReturn(1L).anyTimes();
-        expect(sw.getPort(1)).andReturn(factory10.buildPortDesc().build()).anyTimes();
+        IOFSwitch sw = createMockSwitch(dpid, version);
         replay(sw);
         ControllerRunThread t = new ControllerRunThread();
         t.start();
 
         controller.addOFSwitchListener(switchListener);
-        synchronized (switchListener) {
-            controller.updates.put(controller.new SwitchUpdate(sw.getId(),
-                    Controller.SwitchUpdateType.ACTIVATED_MASTER));
-            switchListener.wait(500);
-            assertTrue("IOFSwitchListener.addedSwitch() was not called",
-                    switchListener.nAddedMaster == 1);
-            controller.addOFSwitchListener(switchListener);
-            synchronized (switchListener) {
-                controller.updates.put(controller.new SwitchUpdate(sw.getId(),
-                        Controller.SwitchUpdateType.ACTIVATED_EQUAL));
-                switchListener.wait(500);
-                assertTrue("IOFSwitchListener.addedSwitch() was not called",
-                        switchListener.nAddedEqual == 1);
-                controller.updates.put(controller.new SwitchUpdate(sw.getId(),
-                        Controller.SwitchUpdateType.DISCONNECTED));
-                switchListener.wait(500);
-                assertTrue("IOFSwitchListener.removedSwitch() was not called",
-                        switchListener.nDisconnected == 1);
-                controller.updates.put(controller.new SwitchUpdate(sw.getId(),
-                        Controller.SwitchUpdateType.PORTCHANGED, sw.getPort(1),
-                        PortChangeType.ADD));
-                switchListener.wait(500);
-                assertTrue(
-                        "IOFSwitchListener.switchPortChanged() with PortChangeType.ADD was not called",
-                        switchListener.nPortAdded == 1);
-                controller.updates.put(controller.new SwitchUpdate(sw.getId(),
-                        Controller.SwitchUpdateType.PORTCHANGED, sw.getPort(1),
-                        PortChangeType.DELETE));
-                switchListener.wait(500);
-                assertTrue(
-                        "IOFSwitchListener.switchPortChanged() with PortChangeType.DELETE was not called",
-                        switchListener.nPortDeleted == 1);
-                controller.updates.put(controller.new SwitchUpdate(sw.getId(),
-                        Controller.SwitchUpdateType.PORTCHANGED, sw.getPort(1),
-                        PortChangeType.OTHER_UPDATE));
-                switchListener.wait(500);
-                assertTrue(
-                        "IOFSwitchListener.switchPortChanged() with PortChangeType.OTHER_UPDATE was not called",
-                        switchListener.nPortChanged == 1);
-            }
+
+        // Switch updates
+        doTestUpdateQueueWithUpdate(dpid, SwitchUpdateType.ACTIVATED_MASTER,
+                switchListener);
+        doTestUpdateQueueWithUpdate(dpid, SwitchUpdateType.ACTIVATED_EQUAL,
+                switchListener);
+        doTestUpdateQueueWithUpdate(dpid, SwitchUpdateType.EQUAL_TO_MASTER,
+                switchListener);
+        doTestUpdateQueueWithUpdate(dpid, SwitchUpdateType.MASTER_TO_EQUAL,
+                switchListener);
+        doTestUpdateQueueWithUpdate(dpid, SwitchUpdateType.DISCONNECTED,
+                switchListener);
+
+        // Port updates
+        doTestUpdateQueueWithPortUpdate(dpid, port, PortChangeType.ADD,
+                switchListener);
+        doTestUpdateQueueWithPortUpdate(dpid, port, PortChangeType.OTHER_UPDATE,
+                switchListener);
+        doTestUpdateQueueWithPortUpdate(dpid, port, PortChangeType.DELETE,
+                switchListener);
+        doTestUpdateQueueWithPortUpdate(dpid, port, PortChangeType.UP,
+                switchListener);
+        doTestUpdateQueueWithPortUpdate(dpid, port, PortChangeType.DOWN,
+                switchListener);
+    }
+
+    private void doTestUpdateQueueWithUpdate(long dpid, SwitchUpdateType type,
+            DummySwitchListener listener) throws InterruptedException {
+        controller.updates.put(controller.new SwitchUpdate(dpid, type));
+        synchronized (listener) {
+            listener.wait(500);
         }
+        // Test that the update was seen by the listener 1 time
+        assertEquals(1, listener.getSwitchUpdateCount(type));
     }
 
-
-    public void verifyPortChangedUpdateInQueue(IOFSwitch sw) throws Exception {
-        assertEquals(1, controller.updates.size());
-        IUpdate update = controller.updates.take();
-        assertEquals(true, update instanceof SwitchUpdate);
-        SwitchUpdate swUpdate = (SwitchUpdate) update;
-        assertEquals(sw.getId(), swUpdate.getSwId());
-        assertEquals(SwitchUpdateType.PORTCHANGED, swUpdate.getSwitchUpdateType());
-        assertEquals(PortChangeType.OTHER_UPDATE, swUpdate.getPortChangeType());
+    private void doTestUpdateQueueWithPortUpdate(long dpid, OFPortDesc port,
+            PortChangeType type,
+            DummySwitchListener listener) throws InterruptedException {
+        controller.updates.put(controller.new SwitchUpdate(dpid,
+                SwitchUpdateType.PORTCHANGED, port, type));
+        synchronized (listener) {
+            listener.wait(500);
+        }
+        // Test that the update was seen by the listener 1 time
+        assertEquals(1, listener.getPortUpdateCount(type));
     }
 
-    public void verifyPortDownUpdateInQueue(IOFSwitch sw) throws Exception {
-        assertEquals(1, controller.updates.size());
-        IUpdate update = controller.updates.take();
-        assertEquals(true, update instanceof SwitchUpdate);
-        SwitchUpdate swUpdate = (SwitchUpdate) update;
-        assertEquals(sw.getId(), swUpdate.getSwId());
-        assertEquals(SwitchUpdateType.PORTCHANGED, swUpdate.getSwitchUpdateType());
-        assertEquals(PortChangeType.DOWN, swUpdate.getPortChangeType());
-    }
-
-    public void verifyPortAddedUpdateInQueue(IOFSwitch sw) throws Exception {
-        assertEquals(1, controller.updates.size());
-        IUpdate update = controller.updates.take();
-        assertEquals(true, update instanceof SwitchUpdate);
-        SwitchUpdate swUpdate = (SwitchUpdate) update;
-        assertEquals(sw.getId(), swUpdate.getSwId());
-        assertEquals(SwitchUpdateType.PORTCHANGED, swUpdate.getSwitchUpdateType());
-        assertEquals(PortChangeType.ADD, swUpdate.getPortChangeType());
-    }
-
-    public void verifyPortRemovedUpdateInQueue(IOFSwitch sw) throws Exception {
-        assertEquals(1, controller.updates.size());
-        IUpdate update = controller.updates.take();
-        assertEquals(true, update instanceof SwitchUpdate);
-        SwitchUpdate swUpdate = (SwitchUpdate) update;
-        assertEquals(sw.getId(), swUpdate.getSwId());
-        assertEquals(SwitchUpdateType.PORTCHANGED, swUpdate.getSwitchUpdateType());
-        assertEquals(PortChangeType.DELETE, swUpdate.getPortChangeType());
-    }
-
-    // * Test handlePortStatus()
-    // *
+    /**
+     * Test the method to notify the controller of a port change.
+     * The controller should send out an update to the switch listeners.
+     *
+     * @throws InterruptedException
+     */
     @Test
-    public void testHandlePortStatus() throws Exception {
-        IOFSwitch sw = createMock(IOFSwitch.class);
-        expect(sw.getId()).andReturn(1L).anyTimes();
-        //expect(sw.getPorts()).andReturn(new HashSet<OFPortDesc>()).anyTimes();
-        OFPortDesc port = factory10.buildPortDesc()
+    public void testNotifyPortChanged() throws InterruptedException {
+        long dpid = 1L;
+
+        // No difference between OpenFlow versions here
+        OFVersion version = OFVersion.OF_10;
+        IOFSwitch sw = createMockSwitch(dpid, version);
+        OFPortDesc port = OFFactories.getFactory(version).buildPortDesc()
                 .setName("myPortName1")
                 .setPortNo(OFPort.of(42))
                 .build();
 
+        replay(sw);
+
         controller.connectedSwitches.put(1L, new OFChannelHandler(controller));
         controller.activeMasterSwitches.put(1L, sw);
 
-        replay(sw);
-        controller.notifyPortChanged(sw.getId(), port, PortChangeType.ADD);
-        verify(sw);
-        verifyPortAddedUpdateInQueue(sw);
-        reset(sw);
+        doTestNotifyPortChanged(dpid, port, PortChangeType.ADD);
+        doTestNotifyPortChanged(dpid, port, PortChangeType.OTHER_UPDATE);
+        doTestNotifyPortChanged(dpid, port, PortChangeType.DELETE);
+        doTestNotifyPortChanged(dpid, port, PortChangeType.UP);
+        doTestNotifyPortChanged(dpid, port, PortChangeType.DOWN);
 
-        expect(sw.getId()).andReturn(1L).anyTimes();
+    }
 
-        Set<OFPortState> ofPortStates = new HashSet<OFPortState>();
-        ofPortStates.add(OFPortState.LINK_DOWN);
-        port.createBuilder().setState(ofPortStates);
-        replay(sw);
-        controller.notifyPortChanged(sw.getId(), port, PortChangeType.OTHER_UPDATE);
-        verify(sw);
-        verifyPortChangedUpdateInQueue(sw);
-        reset(sw);
-        ofPortStates = new HashSet<OFPortState>();
-        port.createBuilder().setState(ofPortStates);
+    private void doTestNotifyPortChanged(long dpid, OFPortDesc port,
+            PortChangeType changeType) throws InterruptedException {
+        controller.notifyPortChanged(dpid, port, changeType);
 
-        expect(sw.getId()).andReturn(1L).anyTimes();
-
-        port.createBuilder().setState(ofPortStates);
-        replay(sw);
-        controller.notifyPortChanged(sw.getId(), port, PortChangeType.DOWN);
-        verify(sw);
-        verifyPortDownUpdateInQueue(sw);
-        reset(sw);
-
-        expect(sw.getId()).andReturn(1L).anyTimes();
-        replay(sw);
-        controller.notifyPortChanged(sw.getId(), port, PortChangeType.DELETE);
-        verify(sw);
-        verifyPortRemovedUpdateInQueue(sw);
-        reset(sw);
-
+        assertEquals(1, controller.updates.size());
+        IUpdate update = controller.updates.take();
+        assertEquals(true, update instanceof SwitchUpdate);
+        SwitchUpdate swUpdate = (SwitchUpdate) update;
+        assertEquals(dpid, swUpdate.getSwId());
+        assertEquals(SwitchUpdateType.PORTCHANGED, swUpdate.getSwitchUpdateType());
+        assertEquals(changeType, swUpdate.getPortChangeType());
     }
 }