Committed by
Ali "The Bomb" Al-Shabibi
added unit tests
Change-Id: Ic743a05b907456e1414a9bc587696de631d3f382 commented the controller test class Change-Id: Id9afb0e60afb3839f65a41b04e7129db1010ca19 added OFChannelHandler tests Change-Id: I45169988f0e4242a6e1c0baf34b1104f53873bb7
Showing
8 changed files
with
175 additions
and
61 deletions
... | @@ -44,7 +44,7 @@ | ... | @@ -44,7 +44,7 @@ |
44 | 44 | ||
45 | <module name="Checker"> | 45 | <module name="Checker"> |
46 | <module name="SuppressionFilter"> | 46 | <module name="SuppressionFilter"> |
47 | - <property name="file" value="${samedir}/suppressions.xml"/> | 47 | + <property name="file" value="${config_loc}/suppressions.xml"/> |
48 | </module> | 48 | </module> |
49 | <!-- | 49 | <!-- |
50 | If you set the basedir property below, then all reported file | 50 | If you set the basedir property below, then all reported file | ... | ... |
... | @@ -174,7 +174,6 @@ | ... | @@ -174,7 +174,6 @@ |
174 | https://issues.jboss.org/browse/JASSIST-228 --> | 174 | https://issues.jboss.org/browse/JASSIST-228 --> |
175 | <argLine>-XX:MaxPermSize=256m -XX:-UseSplitVerifier</argLine> | 175 | <argLine>-XX:MaxPermSize=256m -XX:-UseSplitVerifier</argLine> |
176 | <redirectTestOutputToFile>false</redirectTestOutputToFile> | 176 | <redirectTestOutputToFile>false</redirectTestOutputToFile> |
177 | - <excludedGroups>net.onrc.onos.core.util.IntegrationTest</excludedGroups> | ||
178 | </configuration> | 177 | </configuration> |
179 | </plugin> | 178 | </plugin> |
180 | <!-- TODO exec:java no longer used remove at some point? --> | 179 | <!-- TODO exec:java no longer used remove at some point? --> | ... | ... |
... | @@ -69,8 +69,6 @@ public interface IOFSwitch { | ... | @@ -69,8 +69,6 @@ public interface IOFSwitch { |
69 | 69 | ||
70 | /** | 70 | /** |
71 | * Writes to the OFMessage to the output stream. | 71 | * Writes to the OFMessage to the output stream. |
72 | - * The message will be handed to the floodlightProvider for possible filtering | ||
73 | - * and processing by message listeners | ||
74 | * | 72 | * |
75 | * @param m | 73 | * @param m |
76 | * @param bc | 74 | * @param bc |
... | @@ -80,8 +78,6 @@ public interface IOFSwitch { | ... | @@ -80,8 +78,6 @@ public interface IOFSwitch { |
80 | 78 | ||
81 | /** | 79 | /** |
82 | * Writes the list of messages to the output stream. | 80 | * Writes the list of messages to the output stream. |
83 | - * The message will be handed to the floodlightProvider for possible filtering | ||
84 | - * and processing by message listeners. | ||
85 | * | 81 | * |
86 | * @param msglist | 82 | * @param msglist |
87 | * @param bc | 83 | * @param bc |
... | @@ -333,8 +329,7 @@ public interface IOFSwitch { | ... | @@ -333,8 +329,7 @@ public interface IOFSwitch { |
333 | 329 | ||
334 | /** | 330 | /** |
335 | * Add or modify a switch port. This is called by the core controller | 331 | * Add or modify a switch port. This is called by the core controller |
336 | - * code in response to a OFPortStatus message. It should not typically be | 332 | + * code in response to a OFPortStatus message. |
337 | - * called by other floodlight applications. | ||
338 | * | 333 | * |
339 | * OFPPR_MODIFY and OFPPR_ADD will be treated as equivalent. The OpenFlow | 334 | * OFPPR_MODIFY and OFPPR_ADD will be treated as equivalent. The OpenFlow |
340 | * spec is not clear on whether portNames are portNumbers are considered | 335 | * spec is not clear on whether portNames are portNumbers are considered |
... | @@ -402,29 +397,6 @@ public interface IOFSwitch { | ... | @@ -402,29 +397,6 @@ public interface IOFSwitch { |
402 | public OrderedCollection<PortChangeEvent> | 397 | public OrderedCollection<PortChangeEvent> |
403 | setPorts(Collection<OFPortDesc> ports); | 398 | setPorts(Collection<OFPortDesc> ports); |
404 | 399 | ||
405 | -// XXX S The odd use of providing an API call to 'set ports' (above) would | ||
406 | -// logically suggest that there should be a way to delete or unset the ports. | ||
407 | -// Right now we forbid this. We should probably not use setPorts too. | ||
408 | -// | ||
409 | -// /** | ||
410 | -// * Delete a port for the switch. This is called by the core controller | ||
411 | -// * code in response to a OFPortStatus message. It should not typically be | ||
412 | -// * called by other floodlight applications. | ||
413 | -// * | ||
414 | -// * @param portNumber | ||
415 | -// */ | ||
416 | -// public void deletePort(short portNumber); | ||
417 | -// | ||
418 | -// /** | ||
419 | -// * Delete a port for the switch. This is called by the core controller | ||
420 | -// * code in response to a OFPortStatus message. It should not typically be | ||
421 | -// * called by other floodlight applications. | ||
422 | -// * | ||
423 | -// * @param portName | ||
424 | -// */ | ||
425 | -// public void deletePort(String portName); | ||
426 | - | ||
427 | - | ||
428 | //******************************************* | 400 | //******************************************* |
429 | // IOFSwitch object attributes | 401 | // IOFSwitch object attributes |
430 | //************************ | 402 | //************************ | ... | ... |
... | @@ -127,7 +127,7 @@ public interface IDebugCounterService { | ... | @@ -127,7 +127,7 @@ public interface IDebugCounterService { |
127 | /** | 127 | /** |
128 | * Flush all thread-local counter values (from the current thread) | 128 | * Flush all thread-local counter values (from the current thread) |
129 | * to the global counter store. This method is not intended for use by any | 129 | * to the global counter store. This method is not intended for use by any |
130 | - * module. It's typical usage is from floodlight core and it is meant | 130 | + * module. It's typical usage is from core and it is meant |
131 | * to flush those counters that are updated in the packet-processing pipeline, | 131 | * to flush those counters that are updated in the packet-processing pipeline, |
132 | * typically with the 'updateCounterNoFlush" methods in IDebugCounter. | 132 | * typically with the 'updateCounterNoFlush" methods in IDebugCounter. |
133 | */ | 133 | */ | ... | ... |
... | @@ -144,8 +144,8 @@ public class Controller { | ... | @@ -144,8 +144,8 @@ public class Controller { |
144 | + "activated: dpid {}. Found in activeMaster: {} " | 144 | + "activated: dpid {}. Found in activeMaster: {} " |
145 | + "Found in activeEqual: {}. Aborting ..", new Object[] { | 145 | + "Found in activeEqual: {}. Aborting ..", new Object[] { |
146 | HexString.toHexString(dpid), | 146 | HexString.toHexString(dpid), |
147 | - (activeMasterSwitches.get(dpid) == null) ? 'Y' : 'N', | 147 | + (activeMasterSwitches.get(dpid) == null) ? 'N' : 'Y', |
148 | - (activeEqualSwitches.get(dpid) == null) ? 'Y' : 'N'}); | 148 | + (activeEqualSwitches.get(dpid) == null) ? 'N' : 'Y'}); |
149 | counters.switchWithSameDpidActivated.updateCounterWithFlush(); | 149 | counters.switchWithSameDpidActivated.updateCounterWithFlush(); |
150 | return false; | 150 | return false; |
151 | } | 151 | } |
... | @@ -372,17 +372,13 @@ public class Controller { | ... | @@ -372,17 +372,13 @@ public class Controller { |
372 | HexString.toHexString(dpidLong)); | 372 | HexString.toHexString(dpidLong)); |
373 | return; | 373 | return; |
374 | } | 374 | } |
375 | - if (h.controlRequested) { | 375 | + if (registryService != null && h.controlRequested) { |
376 | + //TODO the above is not good for testing need to change controlrequest to method call. | ||
376 | registryService.releaseControl(dpidLong); | 377 | registryService.releaseControl(dpidLong); |
377 | } | 378 | } |
378 | } | 379 | } |
379 | 380 | ||
380 | 381 | ||
381 | - | ||
382 | - // *************** | ||
383 | - // IFloodlightProviderService | ||
384 | - // *************** | ||
385 | - | ||
386 | // FIXME: remove this method | 382 | // FIXME: remove this method |
387 | public Map<Long, IOFSwitch> getSwitches() { | 383 | public Map<Long, IOFSwitch> getSwitches() { |
388 | return getMasterSwitches(); | 384 | return getMasterSwitches(); |
... | @@ -472,11 +468,6 @@ public class Controller { | ... | @@ -472,11 +468,6 @@ public class Controller { |
472 | } | 468 | } |
473 | 469 | ||
474 | 470 | ||
475 | - public void setAlwaysClearFlowsOnSwAdd(boolean value) { | ||
476 | - this.alwaysClearFlowsOnSwAdd = value; | ||
477 | - } | ||
478 | - | ||
479 | - | ||
480 | public InstanceId getInstanceId() { | 471 | public InstanceId getInstanceId() { |
481 | return instanceId; | 472 | return instanceId; |
482 | } | 473 | } |
... | @@ -587,15 +578,6 @@ public class Controller { | ... | @@ -587,15 +578,6 @@ public class Controller { |
587 | this.counters = new Counters(); | 578 | this.counters = new Counters(); |
588 | this.multiCacheLock = new Object(); | 579 | this.multiCacheLock = new Object(); |
589 | 580 | ||
590 | - | ||
591 | - String option = configParams.get("flushSwitchesOnReconnect"); | ||
592 | - if (option != null && option.equalsIgnoreCase("true")) { | ||
593 | - this.setAlwaysClearFlowsOnSwActivate(true); | ||
594 | - log.info("Flush switches on reconnect -- Enabled."); | ||
595 | - } else { | ||
596 | - this.setAlwaysClearFlowsOnSwActivate(false); | ||
597 | - log.info("Flush switches on reconnect -- Disabled"); | ||
598 | - } | ||
599 | } | 581 | } |
600 | 582 | ||
601 | /** | 583 | /** |
... | @@ -819,12 +801,6 @@ public class Controller { | ... | @@ -819,12 +801,6 @@ public class Controller { |
819 | // Utility methods | 801 | // Utility methods |
820 | // ************** | 802 | // ************** |
821 | 803 | ||
822 | - | ||
823 | - public void setAlwaysClearFlowsOnSwActivate(boolean value) { | ||
824 | - //this.alwaysClearFlowsOnSwActivate = value; | ||
825 | - // XXX S need to be a little more careful about this | ||
826 | - } | ||
827 | - | ||
828 | public Map<String, Long> getMemory() { | 804 | public Map<String, Long> getMemory() { |
829 | Map<String, Long> m = new HashMap<String, Long>(); | 805 | Map<String, Long> m = new HashMap<String, Long>(); |
830 | Runtime runtime = Runtime.getRuntime(); | 806 | Runtime runtime = Runtime.getRuntime(); | ... | ... |
... | @@ -785,7 +785,7 @@ class OFChannelHandler extends IdleStateAwareChannelHandler { | ... | @@ -785,7 +785,7 @@ class OFChannelHandler extends IdleStateAwareChannelHandler { |
785 | * description stats message, we: | 785 | * description stats message, we: |
786 | * - use the switch driver to bind the switch and get an IOFSwitch instance | 786 | * - use the switch driver to bind the switch and get an IOFSwitch instance |
787 | * - setup the IOFSwitch instance | 787 | * - setup the IOFSwitch instance |
788 | - * - add switch to FloodlightProvider(Controller) and send the initial role | 788 | + * - add switch controller and send the initial role |
789 | * request to the switch. | 789 | * request to the switch. |
790 | * Next state: WAIT_INITIAL_ROLE | 790 | * Next state: WAIT_INITIAL_ROLE |
791 | * In the typical case, where switches support role request messages | 791 | * In the typical case, where switches support role request messages | ... | ... |
1 | +/** | ||
2 | + * Copyright 2011, Big Switch Networks, Inc. | ||
3 | + * Originally created by David Erickson, Stanford University | ||
4 | + * | ||
5 | + * Licensed under the Apache License, Version 2.0 (the "License"); you may | ||
6 | + * not use this file except in compliance with the License. You may obtain | ||
7 | + * a copy of the License at | ||
8 | + * | ||
9 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
10 | + * | ||
11 | + * Unless required by applicable law or agreed to in writing, software | ||
12 | + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
13 | + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
14 | + * License for the specific language governing permissions and limitations | ||
15 | + * under the License. | ||
16 | + **/ | ||
17 | + | ||
18 | +package net.onrc.onos.of.ctl.internal; | ||
19 | + | ||
20 | +import junit.framework.TestCase; | ||
21 | +import net.onrc.onos.of.ctl.IOFSwitch; | ||
22 | + | ||
23 | +import org.easymock.EasyMock; | ||
24 | +import org.junit.After; | ||
25 | +import org.junit.Before; | ||
26 | +import org.junit.Test; | ||
27 | + | ||
28 | + | ||
29 | +public class ControllerTest extends TestCase { | ||
30 | + | ||
31 | + private Controller controller; | ||
32 | + private IOFSwitch sw; | ||
33 | + private OFChannelHandler h; | ||
34 | + | ||
35 | + @Override | ||
36 | + @Before | ||
37 | + public void setUp() throws Exception { | ||
38 | + super.setUp(); | ||
39 | + sw = EasyMock.createMock(IOFSwitch.class); | ||
40 | + h = EasyMock.createMock(OFChannelHandler.class); | ||
41 | + controller = new Controller(); | ||
42 | + ControllerRunThread t = new ControllerRunThread(); | ||
43 | + t.start(); | ||
44 | + /* | ||
45 | + * Making sure the thread is properly started before making calls | ||
46 | + * to controller class. | ||
47 | + */ | ||
48 | + Thread.sleep(200); | ||
49 | + } | ||
50 | + | ||
51 | + /** | ||
52 | + * Starts the base mocks used in these tests. | ||
53 | + */ | ||
54 | + private void startMocks() { | ||
55 | + EasyMock.replay(sw, h); | ||
56 | + } | ||
57 | + | ||
58 | + /** | ||
59 | + * Reset the mocks to a known state. | ||
60 | + * Automatically called after tests. | ||
61 | + */ | ||
62 | + @After | ||
63 | + private void resetMocks() { | ||
64 | + EasyMock.reset(sw); | ||
65 | + } | ||
66 | + | ||
67 | + /** | ||
68 | + * Fetches the controller instance. | ||
69 | + * @return the controller | ||
70 | + */ | ||
71 | + public Controller getController() { | ||
72 | + return controller; | ||
73 | + } | ||
74 | + | ||
75 | + /** | ||
76 | + * Run the controller's main loop so that updates are processed. | ||
77 | + */ | ||
78 | + protected class ControllerRunThread extends Thread { | ||
79 | + @Override | ||
80 | + public void run() { | ||
81 | + controller.openFlowPort = 0; // Don't listen | ||
82 | + controller.activate(); | ||
83 | + } | ||
84 | + } | ||
85 | + | ||
86 | + /** | ||
87 | + * Verify that we are able to add a switch that just connected. | ||
88 | + * If it already exists then this should fail | ||
89 | + * | ||
90 | + * @throws Exception error | ||
91 | + */ | ||
92 | + @Test | ||
93 | + public void testAddConnectedSwitches() throws Exception { | ||
94 | + startMocks(); | ||
95 | + assertTrue(controller.addConnectedSwitch(0, h)); | ||
96 | + assertFalse(controller.addConnectedSwitch(0, h)); | ||
97 | + } | ||
98 | + | ||
99 | + /** | ||
100 | + * Add active master but cannot re-add active master. | ||
101 | + * @throws Exception an error occurred. | ||
102 | + */ | ||
103 | + @Test | ||
104 | + public void testAddActivatedMasterSwitch() throws Exception { | ||
105 | + startMocks(); | ||
106 | + controller.addConnectedSwitch(0, h); | ||
107 | + assertTrue(controller.addActivatedMasterSwitch(0, sw)); | ||
108 | + assertFalse(controller.addActivatedMasterSwitch(0, sw)); | ||
109 | + } | ||
110 | + | ||
111 | + /** | ||
112 | + * Tests that an activated switch can be added but cannot be re-added. | ||
113 | + * | ||
114 | + * @throws Exception an error occurred | ||
115 | + */ | ||
116 | + @Test | ||
117 | + public void testAddActivatedEqualSwitch() throws Exception { | ||
118 | + startMocks(); | ||
119 | + controller.addConnectedSwitch(0, h); | ||
120 | + assertTrue(controller.addActivatedEqualSwitch(0, sw)); | ||
121 | + assertFalse(controller.addActivatedEqualSwitch(0, sw)); | ||
122 | + } | ||
123 | + | ||
124 | + /** | ||
125 | + * Move an equal switch to master. | ||
126 | + * @throws Exception an error occurred | ||
127 | + */ | ||
128 | + @Test | ||
129 | + public void testTranstitionToMaster() throws Exception { | ||
130 | + startMocks(); | ||
131 | + controller.addConnectedSwitch(0, h); | ||
132 | + controller.addActivatedEqualSwitch(0, sw); | ||
133 | + controller.transitionToMasterSwitch(0); | ||
134 | + assertNotNull(controller.getMasterSwitch(0)); | ||
135 | + } | ||
136 | + | ||
137 | + /** | ||
138 | + * Transition a master switch to equal state. | ||
139 | + * @throws Exception an error occurred | ||
140 | + */ | ||
141 | + @Test | ||
142 | + public void testTranstitionToEqual() throws Exception { | ||
143 | + startMocks(); | ||
144 | + controller.addConnectedSwitch(0, h); | ||
145 | + controller.addActivatedMasterSwitch(0, sw); | ||
146 | + controller.transitionToEqualSwitch(0); | ||
147 | + assertNotNull(controller.getEqualSwitch(0)); | ||
148 | + } | ||
149 | + | ||
150 | + /** | ||
151 | + * Remove the switch from the controller instance. | ||
152 | + * @throws Exception an error occurred | ||
153 | + */ | ||
154 | + @Test | ||
155 | + public void testRemoveSwitch() throws Exception { | ||
156 | + sw.cancelAllStatisticsReplies(); | ||
157 | + EasyMock.expectLastCall().once(); | ||
158 | + sw.setConnected(false); | ||
159 | + EasyMock.expectLastCall().once(); | ||
160 | + startMocks(); | ||
161 | + controller.addConnectedSwitch(0, h); | ||
162 | + controller.addActivatedMasterSwitch(0, sw); | ||
163 | + controller.removeConnectedSwitch(0); | ||
164 | + assertNull(controller.getSwitch(0)); | ||
165 | + EasyMock.verify(sw, h); | ||
166 | + } | ||
167 | +} |
This diff is collapsed. Click to expand it.
-
Please register or login to post a comment