> 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
> 
>

Reply via email to