Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (athena.apache.org: domain of williamstevens@gmail.com
 designates 209.85.192.180 as permitted sender)
MIME-Version: 1.0
Sender: williamstevens@gmail.com
In-Reply-To: <20130528180725.GZ11891@USLT-205755.sungardas.corp>
References: 
 <CAC2panoHUE38AW-bS3gVqJhO5rpR5WEa7=RyF0Q57DEFRjSxsg@mail.gmail.com>
	<20130528180725.GZ11891@USLT-205755.sungardas.corp>
Date: Tue, 28 May 2013 16:31:27 -0400
Message-ID: 
 <CAC2panpvUQ7XkMA0paOj7ghBFwAypY5xytzO0zCn04RSChnUTg@mail.gmail.com>
Subject: Re: Palo Alto Firewall Integration - Review Process
From: Will Stevens <wstevens@cloudops.com>
To: dev@cloudstack.apache.org
Content-Type: multipart/alternative; boundary=047d7b0721ce47f40904ddcd267c

--047d7b0721ce47f40904ddcd267c
Content-Type: text/plain; charset=ISO-8859-1

Thanks for the reply Chip.  Are there any docs / guides for the test
functionality?


On Tue, May 28, 2013 at 2:07 PM, Chip Childers <chip.childers@sungard.com>wrote:

> On Tue, May 28, 2013 at 01:32:48PM -0400, Will Stevens wrote:
> > Hey All,
> > I am getting close to finishing up this integration, so I want to make
> sure
> > I understand the process and what is required for submitting my code for
> > review.
> >
> > I have read this and am comfortable with its content:
> > http://cloudstack.apache.org/develop/non-committer.html
> >
> > You can check out more details regarding this integration here:
> >
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Palo+Alto+Firewall+Integration
> >
> > Please let me know if you feel I am missing anything on that page.  That
> is
> > still a work in progress, but it does cover the functionality being added
> > pretty well.  The screenshots are not complete yet, but they are at about
> > 90% right now.
> >
> > On that page I have linked a public repo which has a recent working
> version
> > of the code (not feature complete yet and still needs some clean up).
>
> Thanks for doing that!
>
> >
> > Here are the questions that I have about the process:
> > - Do I need to include tests for this code?  If so, is this documented
> > somewhere?  Since this is an integration with an external device, how
> would
> > tests be written to pass without actually connecting to a device?
>
> 2 test types:
>
> 1 - unit tests using a mocking framework are needed for non-trivial
> logic (complex methods)
>
> 2 - integration tests using the marvin framework are the best method of
> providing automated testing of a specific integration.  However, you
> might want to see if your functionality is *already* covered in the test
> suite.  If you are only implementing a driver for a specific technology,
> it might be easy to just play a set of tests against an environment with
> that device enabled.
>
> > - There is a small limitation in core which did not have any dependancies
> > which I have fixed (Sheng and I have discussed this briefly).
>  Basically, I
> > added support for multiple networks per account when the source nat type
> is
> > 'per account' with an external device.  Question: Should I be submitting
> > two patches; one which only addresses this core fix (about 5 lines of
> code)
> > and one which addresses the addition of the palo_alto network plugin?
>  Or,
> > should I submit it all as one patch?
>
> Best to do it as 2.  Note in the new feature patch that it relies on the
> "core" patch.
>
> > - Since this is an integration with a 3rd party product; should I setup
> > a publicly accessible system where the functionality can be reviewed, or
> > should I work with Palo Alto to get demo licenses for their VM firewall
> > appliances and provide the reviewers licences to test the functionality?
>  I
> > am not sure how this aspect should work, so let me know what the best
> > approach would be.
>
> We don't have a good model for this.  Your demo license proposal sounds
> interesting though.  Perhaps that's the model we *should* be using
> whenever possible?
>
> >
> > I think thats it.  Please let me know if something is not clear or if you
> > feel I need to flush out some of the details somewhere.
> >
> > Thanks,
> >
> > Will
>

--047d7b0721ce47f40904ddcd267c--