[Freeipa-devel] [PATCH] jderose 027 Extensible return values

[Jason Gerard DeRose]

On Wed, 2009-12-09 at 23:08 -0500, Rob Crittenden wrote:
> Jason Gerard DeRose wrote:
> > Okay, here's a revised patch.
> > 
> > Significant additions/changes from the previous version are:
> > 
> > 1. The return value dict now includes a 'summary' value, something like
> > 'Added user "jdoe"'.  This summary is used by the CLI and webUI.
> > Previously I was generating the summary in the CLI and webUI separately.
> > This removes the duplication and allows the commands to easily produce
> > arbitrary summaries (before they were limited a single summary format
> > like 'Added user "%(primary_key)s"'.  This also makes it easier for
> > 3rd-party tools to provide UIs without having to introspect the Python
> > API (because they happen to be written in PHP, whatever).
> > 
> > 
> > 2. I renamed the 'primary_key' member in the return value dict to
> > 'value'.  This is simpler and will be will be easier on translators
> > ('Added user "%(primary_key)s"' vs 'Added user "%(value)s"').  I'm also
> > thinking of returning the name of the primary_key (e.g., 'uid') when
> > returning an entry or a list of entries, so this opens the door for me
> > to use 'key' in the future without confusion.  Note this change is only
> > relative to my previous proposed patch.  The use of the return value
> > dict hasn't yet hit master.
> > 
> > 
> > 3. XMLRPC_test.setUp() no longer tests for server availability with
> > `user-show notfound` prior to each test running.  Instead, I try to
> > connect to the server just once when the `xmlrcp_test` module loads,
> > which sets the `server_available` module attribute.  XMLRPC_test.setUp()
> > will still raise nose.SkipTest for each test as before.  This change
> > helps the XMLRPC tests run much faster and also makes problems easier to
> > debug server-side as there isn't all the `user-show notfound` background
> > noise.
> > 
> > 
> > 4. This adds my new `Declarative` base class for the XMLRPC tests which
> > allows you to define the XMLRPC tests using simple data structures,
> > letting the base class do the tedious stuff.  IHMO, the tests are
> > considerably faster and easier to write this way, but just as important
> > is the fact that Declarative takes care of reporting the errors when a
> > command's return value doesn't match what we expected.  We have pretty
> > good coverage in the XMLRCP tests, but we don't have very good reporting
> > when something goes wrong.  I've put a lot of effort into making sure
> > typical error reports contain the information needed to quickly focus in
> > on the problem.  The most important part of the error reporting is in
> > the new tests.util.assert_deepequal() function, which can be used by any
> > test to compare two nested data structures.  Currently only the
> > test_user_plugin and test_group_plugin tests are using `Declarative`,
> > but the rest will follow.
> > 
> > 
> > 5. I rewrote the make-test script in Python and added a feature John
> > asked for and one I wanted.  John wanted the ability to easily run only
> > the tests in one or more modules.  You can now be specifying the module
> > in Python notation or the module file.  For example:
> > 
> >     ./make-test tests.test_xmlrpc.test_user_plugin
> > 
> > Or equivalently:
> > 
> >     ./make-test tests/test_xmlrpc/test_user_plugin.py
> > 
> > I wanted an easy way to use the nosetests --stop option, which causes
> > the testing to abort upon reaching the first error, which I have found
> > useful when updating plugins to one of my incompatible API changes.  Use
> > it like this:
> > 
> >     ./make-test --stop
> > 
> > 
> > Yup, big!  May my patch reviewers one day forgive me.
> > 
> > -Jason
> 
> Ack. There are a couple of things we need to address such as porting the 
> rest of the plugins to work with this new return value scheme but we can 
> do that post-push. IMHO it is better to get this in now and clean up the 
> few remaining items than to delay any further.
> 
> We also need to try to avoid hardcoding domains in the tests. A couple 
> of user tests look for dc=example,dc=com instead of api.env.basedn.
> 
> rob


Thanks.  Pushed to master.  I'll get on porting the few remaining
plugins.