> On Dec. 5, 2012, 1:57 p.m., mice xia wrote: > > Suppose they should follow the same flavor, so I only took a look at > > AddTrafficTypeCmdTest.java > > 1) indent: should be 4 spaces > > 2) naming: method testCreateSuccess actually tests a failure scenario, and > > testCreateFailure actually tests a successful case. > > 3) test coverage: do you think both *create* and *execute* should be > > unit-tested in AddTrafficTypeCmd? The former method is called when a DB > > entry is persisted, the later is called when the async job is executed. > >
I have made the two changes mentioned in the comments 1 & 2 and uploaded the new patch. 1) indent : corrected the indentation. 2) naming : corrected and verified the test cases method names. 3) test coverage : For unit tests, you would mock dependency and see whether the mock object is invoked. For Example in AddTrafficTypeCmdTest execute method , the _responseGenerator will be mocked and we will check whether createTrafficTypeResponse is invoked once by calling mockito for verification. Same applies for create, if it is integration testing then there will be a check whether the values passed are getting persisted to db. - Meghna ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8355/#review14047 ----------------------------------------------------------- On Dec. 6, 2012, 10:25 a.m., Meghna Kale wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8355/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2012, 10:25 a.m.) > > > Review request for cloudstack, Chip Childers, Prasanna Santhanam, Chiradeep > Vittal, and Alex Huang. > > > Description > ------- > > Added unit test cases for api/commands > > > Diffs > ----- > > api/src/com/cloud/api/commands/AssignToLoadBalancerRuleCmd.java 2a88e87 > api/src/com/cloud/api/commands/AssignVMCmd.java be28cc0 > api/src/com/cloud/api/commands/AssociateIPAddrCmd.java 7aaa5b5 > api/src/com/cloud/api/commands/AuthorizeSecurityGroupEgressCmd.java a6088d0 > api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java > e8f8b98 > api/src/com/cloud/api/commands/CreateAutoScalePolicyCmd.java 4d93747 > api/src/com/cloud/api/commands/CreateAutoScaleVmGroupCmd.java 83d7607 > api/src/com/cloud/api/commands/CreateAutoScaleVmProfileCmd.java 68c85d0 > api/src/com/cloud/api/commands/CreateConditionCmd.java eafd8a0 > api/test/src/com/cloud/api/commands/test/AddTrafficTypeCmdTest.java > PRE-CREATION > > api/test/src/com/cloud/api/commands/test/AssignToLoadBalancerRuleCmdTest.java > PRE-CREATION > api/test/src/com/cloud/api/commands/test/AssignVMCmdTest.java PRE-CREATION > api/test/src/com/cloud/api/commands/test/AttachIsoCmdTest.java PRE-CREATION > api/test/src/com/cloud/api/commands/test/AttachVolumeCmdTest.java > PRE-CREATION > > api/test/src/com/cloud/api/commands/test/AuthorizeSecurityGroupEgressCmdTest.java > PRE-CREATION > > api/test/src/com/cloud/api/commands/test/AuthorizeSecurityGroupIngressCmdTest.java > PRE-CREATION > api/test/src/com/cloud/api/commands/test/CancelMaintenanceCmdTest.java > PRE-CREATION > > api/test/src/com/cloud/api/commands/test/CancelPrimaryStorageMaintenanceCmdTest.java > PRE-CREATION > api/test/src/com/cloud/api/commands/test/CreateAutoScalePolicyCmdTest.java > PRE-CREATION > api/test/src/com/cloud/api/commands/test/CreateAutoScaleVmGroupCmdTest.java > PRE-CREATION > > api/test/src/com/cloud/api/commands/test/CreateAutoScaleVmProfileCmdTest.java > PRE-CREATION > api/test/src/com/cloud/api/commands/test/CreateConditionCmdTest.java > PRE-CREATION > api/test/src/com/cloud/api/commands/test/CreateCounterCmdTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/8355/diff/ > > > Testing > ------- > > > Thanks, > > Meghna Kale > >
