[Zope3-dev] opinion requested from people working towards therelease: doctest inaccuracy for zope.app.form.utility

[Garrett Smith]

Gary Poster wrote:
> I'm making some changes to zope.app.form.utility.  I hope to check
> them in in a few hours.  Meanwhile, I wanted to highlight a problem I
> found in the tests (zope.app.form.tests.test_utility) to see if I can
> check in a solution rather than an XXX.  In the
> test_applyWidgetsChanges, I have added the XXX comment you see below
> in my sandbox: 
> 
> ---8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<---
> 
>          When applyWidgetsChanges is called with multiple form
>          fields, some with valid data and some with invalid data, none
>          of the data is applied:
> 
>          XXX This description of what is going on does not appear to
> match
>          the facts.  applyWidgetsChanges does in fact change the foo
> attribute.
>          The current behavior seems acceptable to me, if not ideal;
>          I'm wrapping a subtransaction around the call and aborting
>          the subtransaction if it fails in my app code.
> 
>              >>> context = Content()
>              >>> view = BrowserView(context, request)
>              >>> setUpEditWidgets(view, IContent, names=('foo',
>              'bar')) ['foo', 'bar']
>              >>> view.foo_widget.input = 'a'
>              >>> view.bar_widget.input = 'b'
>              >>> view.bar_widget.valid = False
>              >>> context.foo
>              'Foo'
>              >>> getattr(context, 'bar', 'not really')
>              'not really'
>              >>> applyWidgetsChanges(view, IContent, names=('foo',
> 'bar'))
>              Traceback (most recent call last):
>              WidgetsError: ConversionError: ('invalid input', None)
>              >>> context.foo
>              'a'
>              >>> getattr(context, 'bar', 'not really')
>              'not really'
> 
> ---8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<---
> 
> In the next to last example, if the doctest description were correct,
> context.foo should be 'Foo', I believe.  Does anyone disagree?  If
> not, should I

Based on the docs, right.

> (a) check this in,
> (b) change the XXX to a TODO and check it in,
> (c) change the test description to actually describe the current
> behavior, or
> (d) try to change the code to match the description?
> 
> If (d) is your response, then please also give me a fallback option
> (a, b, or c) if I don't have time.  As I write in my XXX, I have a
> workaround already.
> 
> Opinions?  I particularly want to hear from the people working towards
> the next release, like Stephan.

I think the documentation can be changed. We can debate the behavior,
but it's benign as far as the current edit form -- if any error occurs,
the transaction is aborted. I don't think we need to make form utils
worry about transactional behavior.

> Gary
> 
> _______________________________________________
> Zope3-dev mailing list
> Zope3-dev at zope.org
> Unsub:
>
http://mail.zope.org/mailman/options/zope3-dev/garrett%40mojave-corp.com