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

[Gary Poster]

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

(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.

Gary