Open Stack

On 02/24/2015 01:47 PM, Joe Gordon wrote:
> 
> 
> On Tue, Feb 24, 2015 at 9:47 AM, Sean Dague <sean at dague.net
> <mailto:sean at dague.net>> wrote:
> 
>     Towards the end of merging the regression test for the nova
>     volume-attach bug - https://review.openstack.org/#/c/157959/ there was a
>     discussion around what style the functional tests should take.
>     Especially as that had a mix of CLI and API calls in it.
> 
> 
> 
> Thanks for starting this thread.  Once we reach general agreement lets
> put this in a in tree README for record keeping.

Absolutely.

>     Here are my thoughts for why that test ended up that way:
> 
>     1) All resource setup that is table stakes for the test should be done
>     via the API, regardless if it's a CLI or API test.
> 
>     The reason for this is that structured data is returned, which removes
>     one possible error in the tests by parsing incorrectly. The API objects
>     returned also include things like .delete methods in most cases, which
>     means that addCleanup is a little more clean.
> 
> 
> IMHO the CLI should have an option to returned raw JSON back instead of
> pretty tabled results as well.

Um... isn't that just the API calls?

I'm not sure creating a 3rd functional surface is really the answer
here, because we still need to actually test the CLI / pretty table output.

>     2) Main logic should touch which ever interface you are trying to test.
>     This was demonstrating a CLI regression, so the volume-attach call was
>     important to be done over the CLI.
> 
> 
>     Now... here's where theory runs into issues.
> 
>     #1 - nova boot is table stakes. Under the above guidelines it should be
>     called via API. However --poll is a CLI construct and so saved a custom
>     wait loop here. We should implement that custom wait loop down the road
>     and make that an API call
> 
>     https://github.com/openstack/python-novaclient/blob/master/novaclient/tests/functional/test_instances.py#L116
> 
> 
> This issue is from a real short coming in the python client. So this
> isn't really an issue with your theory, just an issue with novaclient.

Sure, though, if we support the async form in the API we need to test
it. So while adding poll support is a good UX add, it doesn't actually
mean we get to not test the async versions. It just adds another feature
set we need to test.

>     #2 - the volume create command is table stakes. It should be an API
>     call. However, it can't be because the service catalog redirection only
>     works at the CLI layer. This is actually also the crux of bug #1423695.
>     The same reason the completion cache code failed is the reason we can't
>     use the API for that.
> 
>     https://github.com/openstack/python-novaclient/blob/master/novaclient/tests/functional/test_instances.py#L129
> 
> 
> Issues like this are why I wrote the read only nova CLI tests in the
> first place. Unit testing the python API is doable, but unit testing the
> CLI is a little bit more tricky. So I expect issues like this to come up
> over and over again.

So this is actually an issue with the API code. We have a chunk of code
that's directly callable by consumers, but if they do, it will error.

>     #3 - the cleanup of the volume should have been API call. See reason
>     for #2.
> 
>     https://github.com/openstack/python-novaclient/blob/master/novaclient/tests/functional/test_instances.py#L131
> 
>     #4 - the cleanup of the attachment should be an addCleanup via the API.
>     See reason for #2 why it's not.
> 
>     https://github.com/openstack/python-novaclient/blob/master/novaclient/tests/functional/test_instances.py#L155
> 
> 
>     I'm happy if there are other theories about how we do these things,
>     being the first functional test in the python-novaclient tree that
>     creates and destroys real resources, there isn't an established pattern
>     yet. But I think doing all CLI calls in CLI tests is actually really
>     cumbersome, especially in the amount of output parsing code needed if
>     you are going to setup any complicated resource structure.
> 
> 
> Here is an alternate theory:
> 
> We should have both python API and CLI functional tests. But they should
> be kept separate.
> 
> This is to help us make sure both the CLI and python API are actually
> usable interfaces. As the exercise above has shown, they both have
> really major short comings.  I think having in tree functional testing
> covering both the CLI and python API will make it easier for us to
> review new client features in terms of usability.
> 
> Here is a very rough proof of concept patch showing the same
> tests: https://review.openstack.org/#/c/157974/2/novaclient/tests/functional/test_volumes.py,cm
> 
> No matter how we define this functional testing model, I think its clear
> novaclient needs a decent amount of work before it can really be usable.

Agreed, but I think we should focus on testing the code we actually have
to prevent regressions. I think functional changes to put polling
throughout are nice UX, but they might be better left for
openstackclient, and assume we'll get to retire novaclient by then.

	-Sean

-- 
Sean Dague
http://dague.net