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
