You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Meghna Kale <me...@sungard.com> on 2012/12/12 11:56:20 UTC
Review Request: Added Unit test cases for com.cloud.api.commands
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8545/
-----------------------------------------------------------
Review request for cloudstack, Chip Childers, Prasanna Santhanam, mice xia, Chiradeep Vittal, and Alex Huang.
Description
-------
Added Unit test cases for com.cloud.api.commands
Diffs
-----
api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java 937e9aa
api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java a688d13
api/src/com/cloud/api/commands/CreateLBStickinessPolicyCmd.java c2410c8
api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java 80a8684
api/src/com/cloud/api/commands/CreateNetworkACLCmd.java 033a496
api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java 3f3915a
api/src/com/cloud/api/commands/CreatePrivateGatewayCmd.java c56f9af
api/test/src/com/cloud/api/commands/test/CreateDiskOfferingCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreateFirewallRuleCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreateIpForwardingRuleCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreateLBStickinessPolicyCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreateLoadBalancerRuleCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreateNetworkACLCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreateNetworkCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreateNetworkOfferingCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreatePhysicalNetworkCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreatePodCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreatePortForwardingRuleCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/CreatePrivateGatewayCmdTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/8545/diff/
Testing
-------
Thanks,
Meghna Kale
Re: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by Chip Childers <ch...@sungard.com>.
Meghna,
If you don't have the time to re-base and re-work these test cases, just
let me know. I'd be happy to take the patches as-is, and modify them to
work in the new API architecture.
-chip
On Mon, Feb 11, 2013 at 11:02:31PM -0800, Animesh Chaturvedi wrote:
> Meghna
>
> While I was going through the review board I saw this was last updates over a month ago, Do you have any updates?
>
> Animesh
>
> > -----Original Message-----
> > From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
> > Sent: Wednesday, December 19, 2012 11:32 AM
> > To: David Nalley
> > Cc: cloudstack-dev@incubator.apache.org; Chip Childers; Prasanna Santhanam;
> > Alex Huang; mice xia; Chiradeep Vittal; Meghna Kale
> > Subject: Re: Review Request: Added Unit test cases for
> > com.cloud.api.commands
> >
> >
> > On 19-Dec-2012, at 8:36 AM, David Nalley <da...@gnsa.us> wrote:
> >
> > > On Tue, Dec 18, 2012 at 10:59 PM, Rohit Yadav <ro...@citrix.com>
> > wrote:
> > >>
> > >> -----------------------------------------------------------
> > >> This is an automatically generated e-mail. To reply, visit:
> > >> https://reviews.apache.org/r/8545/#review14711
> > >> -----------------------------------------------------------
> > >>
> > >>
> > >> Thanks for your patch, but your changes are not applying cleanly and will
> > break both api_refactoring and javelin. I will have to merge the
> > api_refactoring branch soon else there will be pile of review with a lot of
> > merge conflicts.
> > >>
> > >> My target was to merge api_refactoring yesterday but I thought I would fix
> > some more stuff before I merge it on master
> >
> > See below:
> >
> > >> so it won't break anything on master.
> > >> If you already working on some other unit tests pl. work on the
> > api_refactoring branch, I'm planning to send a merge request and progress
> > report on dev ML this weekend.
> > >>
> > >> - Rohit Yadav
> > >>
> > >
> > > Please discuss this on list before you attempt a merge.
> >
> > Yes, as I mentioned I'll send a merge request before doing it i.e. ask community
> > to checkout the code comment on merge etc. Meanwhile, any patch related to
> > api layer, anything like unit tests etc. should be done on api_refactoring to
> > avoid merge conflicts without waiting for the actual merge on master.
> >
> > > Something of
> > > this magnitude should not happen without some degree of notice.
> >
> > I understand your concern but I think it won't be any major issue as the merge
> > conflicts on master (as of now) are about 100-200 and the only significant
> > changes are package renames, and annotations.
> >
> > Regards.
> >
> > >
> > > --David
>
>
RE: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by Animesh Chaturvedi <an...@citrix.com>.
Meghna
While I was going through the review board I saw this was last updates over a month ago, Do you have any updates?
Animesh
> -----Original Message-----
> From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
> Sent: Wednesday, December 19, 2012 11:32 AM
> To: David Nalley
> Cc: cloudstack-dev@incubator.apache.org; Chip Childers; Prasanna Santhanam;
> Alex Huang; mice xia; Chiradeep Vittal; Meghna Kale
> Subject: Re: Review Request: Added Unit test cases for
> com.cloud.api.commands
>
>
> On 19-Dec-2012, at 8:36 AM, David Nalley <da...@gnsa.us> wrote:
>
> > On Tue, Dec 18, 2012 at 10:59 PM, Rohit Yadav <ro...@citrix.com>
> wrote:
> >>
> >> -----------------------------------------------------------
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/8545/#review14711
> >> -----------------------------------------------------------
> >>
> >>
> >> Thanks for your patch, but your changes are not applying cleanly and will
> break both api_refactoring and javelin. I will have to merge the
> api_refactoring branch soon else there will be pile of review with a lot of
> merge conflicts.
> >>
> >> My target was to merge api_refactoring yesterday but I thought I would fix
> some more stuff before I merge it on master
>
> See below:
>
> >> so it won't break anything on master.
> >> If you already working on some other unit tests pl. work on the
> api_refactoring branch, I'm planning to send a merge request and progress
> report on dev ML this weekend.
> >>
> >> - Rohit Yadav
> >>
> >
> > Please discuss this on list before you attempt a merge.
>
> Yes, as I mentioned I'll send a merge request before doing it i.e. ask community
> to checkout the code comment on merge etc. Meanwhile, any patch related to
> api layer, anything like unit tests etc. should be done on api_refactoring to
> avoid merge conflicts without waiting for the actual merge on master.
>
> > Something of
> > this magnitude should not happen without some degree of notice.
>
> I understand your concern but I think it won't be any major issue as the merge
> conflicts on master (as of now) are about 100-200 and the only significant
> changes are package renames, and annotations.
>
> Regards.
>
> >
> > --David
Re: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by Rohit Yadav <ro...@citrix.com>.
On 19-Dec-2012, at 8:36 AM, David Nalley <da...@gnsa.us> wrote:
> On Tue, Dec 18, 2012 at 10:59 PM, Rohit Yadav <ro...@citrix.com> wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/8545/#review14711
>> -----------------------------------------------------------
>>
>>
>> Thanks for your patch, but your changes are not applying cleanly and will break both api_refactoring and javelin. I will have to merge the api_refactoring branch soon else there will be pile of review with a lot of merge conflicts.
>>
>> My target was to merge api_refactoring yesterday but I thought I would fix some more stuff before I merge it on master
See below:
>> so it won't break anything on master.
>> If you already working on some other unit tests pl. work on the api_refactoring branch, I'm planning to send a merge request and progress report on dev ML this weekend.
>>
>> - Rohit Yadav
>>
>
> Please discuss this on list before you attempt a merge.
Yes, as I mentioned I'll send a merge request before doing it i.e. ask community to checkout the code comment on merge etc. Meanwhile, any patch related to api layer, anything like unit tests etc. should be done on api_refactoring to avoid merge conflicts without waiting for the actual merge on master.
> Something of
> this magnitude should not happen without some degree of notice.
I understand your concern but I think it won't be any major issue as the merge conflicts on master (as of now) are about 100-200 and the only significant changes are package renames, and annotations.
Regards.
>
> --David
Re: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by David Nalley <da...@gnsa.us>.
On Tue, Dec 18, 2012 at 10:59 PM, Rohit Yadav <ro...@citrix.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8545/#review14711
> -----------------------------------------------------------
>
>
> Thanks for your patch, but your changes are not applying cleanly and will break both api_refactoring and javelin. I will have to merge the api_refactoring branch soon else there will be pile of review with a lot of merge conflicts.
>
> My target was to merge api_refactoring yesterday but I thought I would fix some more stuff before I merge it on master so it won't break anything on master.
> If you already working on some other unit tests pl. work on the api_refactoring branch, I'm planning to send a merge request and progress report on dev ML this weekend.
>
> - Rohit Yadav
>
Please discuss this on list before you attempt a merge. Something of
this magnitude should not happen without some degree of notice.
--David
Re: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8545/#review14711
-----------------------------------------------------------
Thanks for your patch, but your changes are not applying cleanly and will break both api_refactoring and javelin. I will have to merge the api_refactoring branch soon else there will be pile of review with a lot of merge conflicts.
My target was to merge api_refactoring yesterday but I thought I would fix some more stuff before I merge it on master so it won't break anything on master.
If you already working on some other unit tests pl. work on the api_refactoring branch, I'm planning to send a merge request and progress report on dev ML this weekend.
- Rohit Yadav
On Dec. 12, 2012, 10:56 a.m., Meghna Kale wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8545/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2012, 10:56 a.m.)
>
>
> Review request for cloudstack, Chip Childers, Prasanna Santhanam, mice xia, Chiradeep Vittal, and Alex Huang.
>
>
> Description
> -------
>
> Added Unit test cases for com.cloud.api.commands
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java 937e9aa
> api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java a688d13
> api/src/com/cloud/api/commands/CreateLBStickinessPolicyCmd.java c2410c8
> api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java 80a8684
> api/src/com/cloud/api/commands/CreateNetworkACLCmd.java 033a496
> api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java 3f3915a
> api/src/com/cloud/api/commands/CreatePrivateGatewayCmd.java c56f9af
> api/test/src/com/cloud/api/commands/test/CreateDiskOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateFirewallRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateIpForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLBStickinessPolicyCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLoadBalancerRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkACLCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePhysicalNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePodCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePortForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePrivateGatewayCmdTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8545/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Meghna Kale
>
>
Re: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by Meghna Kale <me...@sungard.com>.
> On Dec. 13, 2012, 2:27 p.m., Chip Childers wrote:
> > Megnha,
> >
> > You are making a number of changes to the command classes. Can you please provide a description of why each change is needed before we apply this patch?
Chip,
The changes in the command classes are done to handle scenarios where the data returned is null or empty. I did it considering negative scenario for testing.
Please let me know , if you want me to add a comment for the change in the command files.
- Meghna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8545/#review14428
-----------------------------------------------------------
On Dec. 12, 2012, 10:56 a.m., Meghna Kale wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8545/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2012, 10:56 a.m.)
>
>
> Review request for cloudstack, Chip Childers, Prasanna Santhanam, mice xia, Chiradeep Vittal, and Alex Huang.
>
>
> Description
> -------
>
> Added Unit test cases for com.cloud.api.commands
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java 937e9aa
> api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java a688d13
> api/src/com/cloud/api/commands/CreateLBStickinessPolicyCmd.java c2410c8
> api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java 80a8684
> api/src/com/cloud/api/commands/CreateNetworkACLCmd.java 033a496
> api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java 3f3915a
> api/src/com/cloud/api/commands/CreatePrivateGatewayCmd.java c56f9af
> api/test/src/com/cloud/api/commands/test/CreateDiskOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateFirewallRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateIpForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLBStickinessPolicyCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLoadBalancerRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkACLCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePhysicalNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePodCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePortForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePrivateGatewayCmdTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8545/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Meghna Kale
>
>
Re: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by Chip Childers <ch...@sungard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8545/#review14428
-----------------------------------------------------------
Megnha,
You are making a number of changes to the command classes. Can you please provide a description of why each change is needed before we apply this patch?
- Chip Childers
On Dec. 12, 2012, 10:56 a.m., Meghna Kale wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8545/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2012, 10:56 a.m.)
>
>
> Review request for cloudstack, Chip Childers, Prasanna Santhanam, mice xia, Chiradeep Vittal, and Alex Huang.
>
>
> Description
> -------
>
> Added Unit test cases for com.cloud.api.commands
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java 937e9aa
> api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java a688d13
> api/src/com/cloud/api/commands/CreateLBStickinessPolicyCmd.java c2410c8
> api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java 80a8684
> api/src/com/cloud/api/commands/CreateNetworkACLCmd.java 033a496
> api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java 3f3915a
> api/src/com/cloud/api/commands/CreatePrivateGatewayCmd.java c56f9af
> api/test/src/com/cloud/api/commands/test/CreateDiskOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateFirewallRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateIpForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLBStickinessPolicyCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLoadBalancerRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkACLCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePhysicalNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePodCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePortForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePrivateGatewayCmdTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8545/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Meghna Kale
>
>
Re: Review Request: Added Unit test cases for com.cloud.api.commands
Posted by Chip Childers <ch...@sungard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8545/#review14635
-----------------------------------------------------------
I'd like someone more familiar with the command classes being modified to review prior to committing this submission.
- Chip Childers
On Dec. 12, 2012, 10:56 a.m., Meghna Kale wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8545/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2012, 10:56 a.m.)
>
>
> Review request for cloudstack, Chip Childers, Prasanna Santhanam, mice xia, Chiradeep Vittal, and Alex Huang.
>
>
> Description
> -------
>
> Added Unit test cases for com.cloud.api.commands
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java 937e9aa
> api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java a688d13
> api/src/com/cloud/api/commands/CreateLBStickinessPolicyCmd.java c2410c8
> api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java 80a8684
> api/src/com/cloud/api/commands/CreateNetworkACLCmd.java 033a496
> api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java 3f3915a
> api/src/com/cloud/api/commands/CreatePrivateGatewayCmd.java c56f9af
> api/test/src/com/cloud/api/commands/test/CreateDiskOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateFirewallRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateIpForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLBStickinessPolicyCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateLoadBalancerRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkACLCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreateNetworkOfferingCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePhysicalNetworkCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePodCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePortForwardingRuleCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/CreatePrivateGatewayCmdTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8545/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Meghna Kale
>
>