You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Prasanna Santhanam <ts...@apache.org> on 2013/10/02 18:42:40 UTC

[MERGE] marvin-refactor to master

Once upon a time [1] I had propagated the idea of refactoring marvin to
make test case writing simpler. At the time, there weren't enough
people writing tests using marvin however. Now as focus on testing has
become much more important for the stability of our releases I would
like to bring back the discussion and to review the refactoring of
marvin which I've been doing in the marvin_refactor branch.

The key goal of this refactor was to simplify test case writing. In
doing so I've transformed the library from its brittle hand-written
nature to a completely auto-generated set of libraries. In that sense,
marvin is much closer to cloudmonkey now.

The two important changes in this refactor are:

1. data represented in an object-oriented fashion presented as factories
2. test case writing using entities and their operations rather than
a sequence of disconnected API calls.

To see the full nature of this proposal I've updated the spec I put up
on the wiki:
https://cwiki.apache.org/confluence/x/RI3lAQ

For a quick comparison I wrote a test for the VPC vm's lifecycle in
tools/marvin/marvin/test/test_vpc_life_cycle.py which one can compare
with the existing tests for vpc under
test/integration/component/test_vpc_vm_life_cycle.py

These changes being 'architectural' so to speak and in a way
disruptive even I would like to merge this at the beginning of the
upcoming cloudstack release.

This is only a small part of a larger change for marvin which will be
moving to a more BDD like implementation [2] where tests are written
using a gherkin-like language. But that will come later.

I've also tried to disconnect marvin from depending on CloudStack's
build and repo. This will help split marvin from CloudStack which I
will discuss in a seperate thread.

[1] http://markmail.org/message/4tsscn6zvtfsskuj
[2] http://pythonhosted.org/behave/

-- 
Prasanna.,

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


RE: [MERGE] marvin-refactor to master

Posted by Rayees Namathponnan <ra...@citrix.com>.
Currently we are running automation cloudstack repo ,  please don't remove marvin from cloudstack unless new repo is  working seamlessly.

In new marvin specific repo there will be same branch similar to cloudstack repo branches ?

Regards,
Rayees 

-----Original Message-----
From: Sebastien Goasguen [mailto:runseb@gmail.com] 
Sent: Wednesday, October 02, 2013 11:25 AM
To: dev@cloudstack.apache.org
Subject: Re: [MERGE] marvin-refactor to master


On Oct 2, 2013, at 12:56 PM, Prasanna Santhanam <ts...@apache.org> wrote:

> On Wed, Oct 02, 2013 at 12:51:18PM -0400, Chip Childers wrote:
>> On Wed, Oct 02, 2013 at 10:12:40PM +0530, Prasanna Santhanam wrote:
>>> I've also tried to disconnect marvin from depending on CloudStack's 
>>> build and repo. This will help split marvin from CloudStack which I 
>>> will discuss in a seperate thread.
>> 
>> Should we just use this branch as the source for a new repo, and move 
>> on from there?
> 
> I haven't fully disconnected it but will start a DISCUSS thread if 
> there are any concerns about splitting it from the existing repo. If 
> there is consensus we can break it apart from the branch.
> 
> --
> Prasanna.,
> 

I would probably break it and merge in the separate project, that would make a clean cut.
+1


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


Re: [MERGE] marvin-refactor to master

Posted by Sebastien Goasguen <ru...@gmail.com>.
On Oct 2, 2013, at 12:56 PM, Prasanna Santhanam <ts...@apache.org> wrote:

> On Wed, Oct 02, 2013 at 12:51:18PM -0400, Chip Childers wrote:
>> On Wed, Oct 02, 2013 at 10:12:40PM +0530, Prasanna Santhanam wrote:
>>> I've also tried to disconnect marvin from depending on CloudStack's
>>> build and repo. This will help split marvin from CloudStack which I
>>> will discuss in a seperate thread.
>> 
>> Should we just use this branch as the source for a new repo, and move on
>> from there?
> 
> I haven't fully disconnected it but will start a DISCUSS thread if
> there are any concerns about splitting it from the existing repo. If
> there is consensus we can break it apart from the branch.
> 
> -- 
> Prasanna.,
> 

I would probably break it and merge in the separate project, that would make a clean cut.
+1


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


Re: [MERGE] marvin-refactor to master

Posted by Prasanna Santhanam <ts...@apache.org>.
On Wed, Oct 02, 2013 at 12:51:18PM -0400, Chip Childers wrote:
> On Wed, Oct 02, 2013 at 10:12:40PM +0530, Prasanna Santhanam wrote:
> > I've also tried to disconnect marvin from depending on CloudStack's
> > build and repo. This will help split marvin from CloudStack which I
> > will discuss in a seperate thread.
> 
> Should we just use this branch as the source for a new repo, and move on
> from there?

I haven't fully disconnected it but will start a DISCUSS thread if
there are any concerns about splitting it from the existing repo. If
there is consensus we can break it apart from the branch.

-- 
Prasanna.,

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


Re: [MERGE] marvin-refactor to master

Posted by Chip Childers <ch...@sungard.com>.
On Wed, Oct 02, 2013 at 10:12:40PM +0530, Prasanna Santhanam wrote:
> I've also tried to disconnect marvin from depending on CloudStack's
> build and repo. This will help split marvin from CloudStack which I
> will discuss in a seperate thread.

Should we just use this branch as the source for a new repo, and move on
from there?

Re: [MERGE] marvin-refactor to master

Posted by Prasanna Santhanam <ts...@apache.org>.
Edison - thanks for the review! I've answered inline.

(I've brought the technical review to the right thread from the one about
marvin's repo separation)

> Few questions:
> 1. About the "more object-oriented" CloudStack API python binding: Is the
> proposed api  good enough?

As long as the cloudstack API retains its compatibility as it does now by not
altering required arguments. We are good to go. The current implementation of
VirtualMachine is bloated and does too many things, like SSH connections, NAT
creation, security group creation etc. The new method will provide such special
cases as factory hierarchies instead.

So: you'll have the regular VirtualMachine -> VpcVirtualMachine ->
VirtualMachineWithNAT -> VirtualMachineWithIngress etc

> For example, 
> The current hand written create virtual machine looks like:
> class VirtualMachine(object):
> ....
> @classmethod
> def create(cls, apiclient, services, templateid=None, accountid=None,
>     domainid=None, zoneid=None, networkids=None, serviceofferingid=None,
>     securitygroupids=None, projectid=None, startvm=None,
>     diskofferingid=None, affinitygroupnames=None, group=None,
>     hostid=None, keypair=None, mode='basic', method='GET'):
> 
> the proposed api may look like:
> 
> class VirtualMachine(object):
>    def create(self, apiclient, accountId, templateId, **kwargs)
> 
> The proposed api will look better than previous one, and it's automatically
> generated, so easy to maintain. But as a consumer of the api, how do people
> know what kind of parameters should be passed in? Will you have an online
> document for your api? Or you assume people will look at the api docs generated
> by CloudStack? 
> Or why not make the api itself as self-contained? For example, add docs
> before create method:

All **kwargs will be spelt out as docstrings in the entity's methods. This is
something I haven't got to yet. It's in the TODO list doc on the branch however. I
recognize the difficulty in understanding kwargs for someone looking at the
API. I will fix before merge.  

My concern however is of factories being appropriately documented since they
are user written. Those will need to be caught via review.

> 
> 2. Regarding to data factories. From the proposed factories, in each test
> case, does test writer still need to write the code to get data, such as
> writing code to get account during the setupclass?

No. this is not required anymore. All data is represented as a factory. So to
get account data you simply import the necessary factory. You don't have to
imagine the structure of this data and json anymore.

  from marvin.factory.data import UserAccount 
  ...
  def setUp()
      account = UserAccount(apiclient)

So those crufty json headers should altogether disappear.

> With the data factories, the code will look like the following?
> 
> Class TestFoo:
>      Def setupClass():
>               Account = UserAccount(apiclient)
>                VM = UserVM(apiClient)
> 
> And if I want to customize the default data factories, I should be able to
> use something like: UserAccount(apiclient, username='myfoo')?

Yes, this will create a new useraccount with an overridden username. You may
override any attribute of the data this way. This however, doesn't check for
duplicates.  So if a username 'myfoo' already exists, that account creation
will fail. If you use the factory, since it generates a random sequence you
won't have the problem of collisions

> And the data factories should be able to customized based on test
> environment, right? 
> For example, the current iso test cases are hardcoded to test against
> http://people.apache.org/~tsp/dummy.iso, but it won't work for devcloud, or
> in an internal network. The ISO data factory should be able to return an url
> based on different test environment, thus iso test cases can be reused.

Yes, we'll have to create a LocalIsoFactory which represents an ISO available
on the internal network. It is customizable. May be we can represent it to look
for a file within devcloud itself?

Thanks,

> On Wed, Oct 02, 2013 at 10:12:40PM +0530, Prasanna Santhanam wrote:
> > Once upon a time [1] I had propagated the idea of refactoring marvin to
> > make test case writing simpler. At the time, there weren't enough
> > people writing tests using marvin however. Now as focus on testing has
> > become much more important for the stability of our releases I would
> > like to bring back the discussion and to review the refactoring of
> > marvin which I've been doing in the marvin_refactor branch.
> > 
> > The key goal of this refactor was to simplify test case writing. In
> > doing so I've transformed the library from its brittle hand-written
> > nature to a completely auto-generated set of libraries. In that sense,
> > marvin is much closer to cloudmonkey now.
> > 
> > The two important changes in this refactor are:
> > 
> > 1. data represented in an object-oriented fashion presented as factories
> > 2. test case writing using entities and their operations rather than
> > a sequence of disconnected API calls.
> > 
> > To see the full nature of this proposal I've updated the spec I put up
> > on the wiki:
> > https://cwiki.apache.org/confluence/x/RI3lAQ
> > 
> > For a quick comparison I wrote a test for the VPC vm's lifecycle in
> > tools/marvin/marvin/test/test_vpc_life_cycle.py which one can compare
> > with the existing tests for vpc under
> > test/integration/component/test_vpc_vm_life_cycle.py
> > 
> > These changes being 'architectural' so to speak and in a way
> > disruptive even I would like to merge this at the beginning of the
> > upcoming cloudstack release.
> > 
> > This is only a small part of a larger change for marvin which will be
> > moving to a more BDD like implementation [2] where tests are written
> > using a gherkin-like language. But that will come later.
> > 
> > I've also tried to disconnect marvin from depending on CloudStack's
> > build and repo. This will help split marvin from CloudStack which I
> > will discuss in a seperate thread.
> > 
> > [1] http://markmail.org/message/4tsscn6zvtfsskuj
> > [2] http://pythonhosted.org/behave/
> > 
> > -- 
> > Prasanna.,
> > 
> > ------------------------
> > Powered by BigRock.com
> 
> -- 
> Prasanna.,

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


Re: [MERGE] marvin-refactor to master

Posted by Prasanna Santhanam <ts...@apache.org>.
After some conversations it was decided to not merge this branch to
master. The gist - since factories introduce complexity into the
library and a learning curve for test case writers it was felt this
would be a burden to manage and maintain this work.

I've left the branch as-is for anyone to pickup if they feel it is
worth improving upon and then merging with these fixed.

Thanks,

-- 
Prasanna.,

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


Re: [MERGE] marvin-refactor to master

Posted by Prasanna Santhanam <ts...@apache.org>.
On Sun, Oct 13, 2013 at 08:14:01AM +0530, Girish Shilamkar wrote:
> Hello Prasanna,
> 
> +1 to the new marvin architecture. 
> The proposed architecture will make marvin lot more stable, remove
> defining data in code and will be easier to add testcases.

Great! thanks for reviewing.

> 
> Can we have a run in jenkins.cloudstack.org to ensure that the
> existing tests won't break with new marvin ?

Absolutely. I will rebase the branch run the test suite on jenkins.
> 
> How much effort would it be to convert existing tests to use new
> marvin, sooner or later we might want to do it ?

I think it would be a reasonably big task to take up and I wasn't sure
if we were ever going to take that up. We would gradually have to
write tests using the new way and with enough tests under our belt
abandon the old tests. This is why I didn't break the compatibility or
I would've refactored it slightly differently getting rid of the
cloudstackAPI cmd objects altogether.

> Going forward it will be critical to maintain these associations via
> factory hierarchies else would beat the purpose. 
> And needs to be caught using reviews.

Yes - we need a lot of care in defining our factories to make them
reusable, well documented and carefully organized.

> 
> In the branch what is the feature/ dir for ?

I was experimenting with the behave framework for converting the new
representation of cloudstack entities into a BDD based test. But this
will be separate from the intermediary work of factories.

> 
> Could you also put up a Todo list on the wiki ? I did not find one
> in the branch.

I will paste the TODO list in the branch on to the wiki. They're in
errata.markdown in tools/marvin/doc

> 
> Regards,
> Girish
> 
> 
> On 02-Oct-2013, at 10:12 PM, Prasanna Santhanam <ts...@apache.org> wrote:
> 
> > Once upon a time [1] I had propagated the idea of refactoring marvin to
> > make test case writing simpler. At the time, there weren't enough
> > people writing tests using marvin however. Now as focus on testing has
> > become much more important for the stability of our releases I would
> > like to bring back the discussion and to review the refactoring of
> > marvin which I've been doing in the marvin_refactor branch.
> > 
> > The key goal of this refactor was to simplify test case writing. In
> > doing so I've transformed the library from its brittle hand-written
> > nature to a completely auto-generated set of libraries. In that sense,
> > marvin is much closer to cloudmonkey now.
> > 
> > The two important changes in this refactor are:
> > 
> > 1. data represented in an object-oriented fashion presented as factories
> > 2. test case writing using entities and their operations rather than
> > a sequence of disconnected API calls.
> > 
> > To see the full nature of this proposal I've updated the spec I put up
> > on the wiki:
> > https://cwiki.apache.org/confluence/x/RI3lAQ
> > 
> > For a quick comparison I wrote a test for the VPC vm's lifecycle in
> > tools/marvin/marvin/test/test_vpc_life_cycle.py which one can compare
> > with the existing tests for vpc under
> > test/integration/component/test_vpc_vm_life_cycle.py
> > 
> > These changes being 'architectural' so to speak and in a way
> > disruptive even I would like to merge this at the beginning of the
> > upcoming cloudstack release.
> > 
> > This is only a small part of a larger change for marvin which will be
> > moving to a more BDD like implementation [2] where tests are written
> > using a gherkin-like language. But that will come later.
> > 
> > I've also tried to disconnect marvin from depending on CloudStack's
> > build and repo. This will help split marvin from CloudStack which I
> > will discuss in a seperate thread.
> > 
> > [1] http://markmail.org/message/4tsscn6zvtfsskuj
> > [2] http://pythonhosted.org/behave/
> > 
> > -- 
> > Prasanna.,
> > 
> > ------------------------
> > Powered by BigRock.com
> > 
> 

-- 
Prasanna.,

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


Re: [MERGE] marvin-refactor to master

Posted by Girish Shilamkar <gi...@clogeny.com>.
Hello Prasanna,

+1 to the new marvin architecture. 
The proposed architecture will make marvin lot more stable, remove defining data in code and will be easier to add testcases.

Can we have a run in jenkins.cloudstack.org to ensure that the existing tests won't break with new marvin ?

How much effort would it be to convert existing tests to use new marvin, sooner or later we might want to do it ?

Going forward it will be critical to maintain these associations via factory hierarchies else would beat the purpose. 
And needs to be caught using reviews.

In the branch what is the feature/ dir for ?

Could you also put up a Todo list on the wiki ? I did not find one in the branch.

Regards,
Girish


On 02-Oct-2013, at 10:12 PM, Prasanna Santhanam <ts...@apache.org> wrote:

> Once upon a time [1] I had propagated the idea of refactoring marvin to
> make test case writing simpler. At the time, there weren't enough
> people writing tests using marvin however. Now as focus on testing has
> become much more important for the stability of our releases I would
> like to bring back the discussion and to review the refactoring of
> marvin which I've been doing in the marvin_refactor branch.
> 
> The key goal of this refactor was to simplify test case writing. In
> doing so I've transformed the library from its brittle hand-written
> nature to a completely auto-generated set of libraries. In that sense,
> marvin is much closer to cloudmonkey now.
> 
> The two important changes in this refactor are:
> 
> 1. data represented in an object-oriented fashion presented as factories
> 2. test case writing using entities and their operations rather than
> a sequence of disconnected API calls.
> 
> To see the full nature of this proposal I've updated the spec I put up
> on the wiki:
> https://cwiki.apache.org/confluence/x/RI3lAQ
> 
> For a quick comparison I wrote a test for the VPC vm's lifecycle in
> tools/marvin/marvin/test/test_vpc_life_cycle.py which one can compare
> with the existing tests for vpc under
> test/integration/component/test_vpc_vm_life_cycle.py
> 
> These changes being 'architectural' so to speak and in a way
> disruptive even I would like to merge this at the beginning of the
> upcoming cloudstack release.
> 
> This is only a small part of a larger change for marvin which will be
> moving to a more BDD like implementation [2] where tests are written
> using a gherkin-like language. But that will come later.
> 
> I've also tried to disconnect marvin from depending on CloudStack's
> build and repo. This will help split marvin from CloudStack which I
> will discuss in a seperate thread.
> 
> [1] http://markmail.org/message/4tsscn6zvtfsskuj
> [2] http://pythonhosted.org/behave/
> 
> -- 
> Prasanna.,
> 
> ------------------------
> Powered by BigRock.com
> 


Re: [MERGE] marvin-refactor to master

Posted by Prasanna Santhanam <ts...@apache.org>.
Copying folks that this change will affect for review.

On Wed, Oct 02, 2013 at 10:12:40PM +0530, Prasanna Santhanam wrote:
> Once upon a time [1] I had propagated the idea of refactoring marvin to
> make test case writing simpler. At the time, there weren't enough
> people writing tests using marvin however. Now as focus on testing has
> become much more important for the stability of our releases I would
> like to bring back the discussion and to review the refactoring of
> marvin which I've been doing in the marvin_refactor branch.
> 
> The key goal of this refactor was to simplify test case writing. In
> doing so I've transformed the library from its brittle hand-written
> nature to a completely auto-generated set of libraries. In that sense,
> marvin is much closer to cloudmonkey now.
> 
> The two important changes in this refactor are:
> 
> 1. data represented in an object-oriented fashion presented as factories
> 2. test case writing using entities and their operations rather than
> a sequence of disconnected API calls.
> 
> To see the full nature of this proposal I've updated the spec I put up
> on the wiki:
> https://cwiki.apache.org/confluence/x/RI3lAQ
> 
> For a quick comparison I wrote a test for the VPC vm's lifecycle in
> tools/marvin/marvin/test/test_vpc_life_cycle.py which one can compare
> with the existing tests for vpc under
> test/integration/component/test_vpc_vm_life_cycle.py
> 
> These changes being 'architectural' so to speak and in a way
> disruptive even I would like to merge this at the beginning of the
> upcoming cloudstack release.
> 
> This is only a small part of a larger change for marvin which will be
> moving to a more BDD like implementation [2] where tests are written
> using a gherkin-like language. But that will come later.
> 
> I've also tried to disconnect marvin from depending on CloudStack's
> build and repo. This will help split marvin from CloudStack which I
> will discuss in a seperate thread.
> 
> [1] http://markmail.org/message/4tsscn6zvtfsskuj
> [2] http://pythonhosted.org/behave/
> 
> -- 
> Prasanna.,
> 
> ------------------------
> Powered by BigRock.com

-- 
Prasanna.,

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