> On Nov. 29, 2014, 3:32 p.m., Rohit Yadav wrote: > > core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java, line 50 > > <https://reviews.apache.org/r/28535/diff/1/?file=778385#file778385line50> > > > > If you want insertion ordered iteration or array (toArrary output) why > > not use LinkedHashSet? Even if TreeSet will return sorted/ordered traversal > > or array, it will add log(n) complexity to already O(n) loop. Should we use > > that and fix any tests accordingly? Can you also share the test and log > > which broke for you? > > ChunFeng Tian wrote: > Rohit Yadav: > > Actually , LinkedHashSet is my first choice too , but can not pass the > junit test . > In method "generateFwRules()" , the toAdd Set only used generate toArray > result which is expected sequenced as input in test code. > > > I can pass test on windows with HashSet but failed on Mac OSX. > > > failed test log (HashSet): > ----------------------------- > > Results : > > Failed tests: > testFirewallRulesCommand(com.cloud.agent.resource.virtualnetwork.VirtualRoutingResourceTest): > > expected:<...1.2/24:,64.10.10.10:[reverted:0:0:0:,64.10.10.10:TCP:22:80:10.10.1.1/24-10.10.1.2/24]:,> > but > was:<...1.2/24:,64.10.10.10:[TCP:22:80:10.10.1.1/24-10.10.1.2/24:,64.10.10.10:reverted:0:0:0]:,> > > testAggregationCommands(com.cloud.agent.resource.virtualnetwork.VirtualRoutingResourceTest): > > expected:<...1.2/24:,64.10.10.10:[reverted:0:0:0:,64.10.10.10:TCP:22:80:10.10.1.1/24-10.10.1.2/24]:,(..) > > Tests run: 139, Failures: 2, Errors: 0, Skipped: 0 > > end > ------------
Hi, I played the patch using LinkedHashSet, it builds fine and the tests passes for me on OSX using JDK 1.7. In fact the tests have been passing for most of us in OSX, since this test has been in core module for several months. Are you using JDK 7? If you're using JDK 8, some tests and build failures may happen. - Rohit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28535/#review63285 ----------------------------------------------------------- On Nov. 29, 2014, 9:22 a.m., ChunFeng Tian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28535/ > ----------------------------------------------------------- > > (Updated Nov. 29, 2014, 9:22 a.m.) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > run test on core failed : Change firewall rule set from HashSet to TreeSet > because HashSet elements are not ordered , which will cause test failed when > elements toArray are not same between windows and mac . > > > Diffs > ----- > > core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java be85887 > > Diff: https://reviews.apache.org/r/28535/diff/ > > > Testing > ------- > > after ensure the firewall rule set in TreeSet , toArray will the same order > accross windows and mac platform, and passed test. > > > Thanks, > > ChunFeng Tian > >
