> On Nov. 29, 2014, 11: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?
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 ------------ - tian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28535/#review63285 ----------------------------------------------------------- On Nov. 29, 2014, 5:22 p.m., tian chunfeng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28535/ > ----------------------------------------------------------- > > (Updated Nov. 29, 2014, 5:22 p.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, > > tian chunfeng > >