You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Santhosh Edukulla <sa...@citrix.com> on 2013/11/04 18:22:17 UTC

Re: Review Request 15208: Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15208/
-----------------------------------------------------------

(Updated Nov. 4, 2013, 5:22 p.m.)


Review request for cloudstack and Prasanna Santhanam.


Summary (updated)
-----------------

Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase


Bugs: CLOUDSTACK-5032
    https://issues.apache.org/jira/browse/CLOUDSTACK-5032


Repository: cloudstack-git


Description (updated)
-------

Added custom assert by name "assertElementInList" to cloudstackTestCase, this will help users to simplify their asserts. 
Allows users to dump custom message for these asserts.
Changed the Interface for "verifyElementInList" to take additional argument.


Diffs
-----

  tools/marvin/marvin/cloudstackTestCase.py 85ef542 
  tools/marvin/marvin/integration/lib/utils.py d53c1ae 

Diff: https://reviews.apache.org/r/15208/diff/


Testing
-------


Thanks,

Santhosh Edukulla


Re: Review Request 15208: Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase

Posted by Girish Shilamkar <gi...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15208/#review28527
-----------------------------------------------------------


4.2: c76da53123f4130ac59bdf112171bc2e39af91d4

- Girish Shilamkar


On Nov. 4, 2013, 5:22 p.m., Santhosh Edukulla wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15208/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2013, 5:22 p.m.)
> 
> 
> Review request for cloudstack and Prasanna Santhanam.
> 
> 
> Bugs: CLOUDSTACK-5032
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5032
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Added custom assert by name "assertElementInList" to cloudstackTestCase, this will help users to simplify their asserts. 
> Allows users to dump custom message for these asserts.
> Changed the Interface for "verifyElementInList" to take additional argument.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/cloudstackTestCase.py 85ef542 
>   tools/marvin/marvin/integration/lib/utils.py d53c1ae 
> 
> Diff: https://reviews.apache.org/r/15208/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Santhosh Edukulla
> 
>


Re: Review Request 15208: Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase

Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15208/#review28181
-----------------------------------------------------------

Ship it!


1058e3049a2141cad8bfba4cf8afca6916396d77

- daan Hoogland


On Nov. 4, 2013, 5:22 p.m., Santhosh Edukulla wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15208/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2013, 5:22 p.m.)
> 
> 
> Review request for cloudstack and Prasanna Santhanam.
> 
> 
> Bugs: CLOUDSTACK-5032
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5032
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Added custom assert by name "assertElementInList" to cloudstackTestCase, this will help users to simplify their asserts. 
> Allows users to dump custom message for these asserts.
> Changed the Interface for "verifyElementInList" to take additional argument.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/cloudstackTestCase.py 85ef542 
>   tools/marvin/marvin/integration/lib/utils.py d53c1ae 
> 
> Diff: https://reviews.apache.org/r/15208/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Santhosh Edukulla
> 
>


Re: Review Request 15208: Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase

Posted by Prasanna Santhanam <ts...@apache.org>.
hi Daan, thanks for helping review this. This was a follow-up patch to
r15175 [1] where the utility to verify if an element is of type
non-empty-list was submitted by Santhosh. Typically in test cases you
will see that the test has to create a resource (X) and verify by
calling list resources to see if the resource X achieved desired state.
Often these are followed with asserts where we check if list returned
is a non-empty-list. A utility (overlaoded assert) here will help make
that code cleaner.

Our listXXX Apis can throw an exception when the entity being listed
does not exist/deleted or an invalid uuid is provided that never
existed in the first place. If it returned an empty list in these
situations we could get rid of these asserts altogether. I actually do
not see the need for the user to know /differentiate between the two
situations (entity deleted and entity non-existent)

[1] https://reviews.apache.org/r/15175/

Ship it!

On Tue, Nov 05, 2013 at 10:03:59AM +0100, Daan Hoogland wrote:
> Prasanna,
> 
> This looks alright to me. As I am not an expert on bvt; Can you give some
> clues as what to look for in reviews of this kind of code?
> 
> otherwise ship it,
> Daan
> 
> 
> On Mon, Nov 4, 2013 at 6:22 PM, Santhosh Edukulla <
> santhosh.edukulla@citrix.com> wrote:
> 
> >    This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/15208/
> >   Review request for cloudstack and Prasanna Santhanam.
> > By Santhosh Edukulla.
> >
> > *Updated Nov. 4, 2013, 5:22 p.m.*
> > Summary (updated)
> >
> > Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase
> >
> >   *Bugs: * CLOUDSTACK-5032<https://issues.apache.org/jira/browse/CLOUDSTACK-5032>
> >  *Repository: * cloudstack-git
> > Description (updated)
> >
> > Added custom assert by name "assertElementInList" to cloudstackTestCase, this will help users to simplify their asserts.
> > Allows users to dump custom message for these asserts.
> > Changed the Interface for "verifyElementInList" to take additional argument.
> >
> >
> >
> >   Diffs
> >
> >    - tools/marvin/marvin/cloudstackTestCase.py (85ef542)
> >    - tools/marvin/marvin/integration/lib/utils.py (d53c1ae)
> >
> > View Diff <https://reviews.apache.org/r/15208/diff/>
> >

-- 
Prasanna.,

------------------------
Powered by BigRock.com


Re: Review Request 15208: Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase

Posted by Daan Hoogland <da...@gmail.com>.
Prasanna,

This looks alright to me. As I am not an expert on bvt; Can you give some
clues as what to look for in reviews of this kind of code?

otherwise ship it,
Daan


On Mon, Nov 4, 2013 at 6:22 PM, Santhosh Edukulla <
santhosh.edukulla@citrix.com> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15208/
>   Review request for cloudstack and Prasanna Santhanam.
> By Santhosh Edukulla.
>
> *Updated Nov. 4, 2013, 5:22 p.m.*
> Summary (updated)
>
> Added fix for bug 5032. Provides custom assert facility to test features. Added the new assert to cloudstackTestCase
>
>   *Bugs: * CLOUDSTACK-5032<https://issues.apache.org/jira/browse/CLOUDSTACK-5032>
>  *Repository: * cloudstack-git
> Description (updated)
>
> Added custom assert by name "assertElementInList" to cloudstackTestCase, this will help users to simplify their asserts.
> Allows users to dump custom message for these asserts.
> Changed the Interface for "verifyElementInList" to take additional argument.
>
>
>
>   Diffs
>
>    - tools/marvin/marvin/cloudstackTestCase.py (85ef542)
>    - tools/marvin/marvin/integration/lib/utils.py (d53c1ae)
>
> View Diff <https://reviews.apache.org/r/15208/diff/>
>