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.child...@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 >
