GenericSetup tests fail under Zope 5.3.

[mauritsvanrees]

I first saw this in a Plone 6 test run when I switch from Zope 5.2.1 to 5.3.

Sample failure locally with GenericSetup tox:

Failure in test test__initProperties_nopurge_base (Products.GenericSetup.tests.test_utils.PropertyManagerHelpersTests)
Traceback (most recent call last):
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 593, in run
    self._callTestMethod(testMethod)
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 550, in _callTestMethod
    method()
  File "/Users/maurits/community/plone-coredev/6.0/src/Products.GenericSetup/src/Products/GenericSetup/tests/test_utils.py", line 669, in test__initProperties_nopurge_base
    self.assertEqual(obj.getProperty('lines3'), (b'Gee', b'Foo', b'Bar'))
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 831, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 1048, in assertTupleEqual
    self.assertSequenceEqual(tuple1, tuple2, msg, seq_type=tuple)
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 1019, in assertSequenceEqual
    self.fail(msg)
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 670, in fail
    raise self.failureException(msg)
AssertionError: Tuples differ: ('Foo', 'Gee', b'Foo', b'Bar') != (b'Gee', b'Foo', b'Bar')

First differing element 0:
'Foo'
b'Gee'

First tuple contains 1 additional elements.
First extra element 3:
b'Bar'

- ('Foo', 'Gee', b'Foo', b'Bar')
?  ^^^^^^^

+ (b'Gee', b'Foo', b'Bar')
?  ^

So there is a mixup between bytes and strings.

With tox, the Python 2.7 and 3.5 tests pass, because they use Zope 4.x.
On the newer Pythons, 6 tests fail. They pass when I change buildout.cfg to extend the Zope 5.2.1 versions.cfg.
When I edit tox.ini to let Python 3.6, 3.7 and 3.8 use Zope4 (buildout4.cfg) those tests pass.

The failure above is for this line. The test imports _NOPURGE_IMPORT

Ignoring the test failure for lines3 for a moment, let's look at what happens with the lines1 field. I think the following is a bit suspicious:

• The test creates a lines field with (in Python 3 terms) strings.
• When you call getProperty, you indeed get a tuple of strings.
• Then the import is done, which purges these strings and puts back two new elements.
• And then getProperty gives a tuple of bytes.

I don't know yet if this is a problem in practice, or if the tests just need to be fixed.

I wonder what changed in Zope 5.3 for this. Maybe zopefoundation/Zope#962 is involved.

[mauritsvanrees]

I wonder what changed in Zope 5.3 for this. Maybe zopefoundation/Zope#962 is involved.

Confirmed. When I use a Zope checkout and revert Zope commit f30212ef52d3211bd305eafc058f4bda0f9b4af8, the GenericSetup tests pass again.

cc @jugmac00 who added this commit.
I don't know if this commit is bad, or if GenericSetup should catch up.

[jugmac00]

I wonder what changed in Zope 5.3 for this.

Yes, this is my change, see also announced at https://zope.readthedocs.io/en/latest/changes.html#id1

It was agreed upon that the old behavior (field2lines returned a list of bytes) is a bug in zopefoundation/Zope#962 as it both did not work as the other converters and also did not work as documented.

Then, during the last sprint we discussed this issue again at the standup and it was decided to align the code.

I certainly grepped the zopefoundation repositories beforehands, but there are no callers of field2line - but obviously it is used via its alias/mapping lines.

From reading the documentation, I was not aware that those converters were also used outside of e.g.web forms.

The new behavior of the converters is correct, and aligned to the others, and thus here to stay I guess.

Unfortunately, I have no knowledge about Products.GenericSetup and thus I cannot say whether the tests need to be updated or the code.

@mauritsvanrees, on PyPI I saw you are a maintainer of Products.GenericSetup. So you should have more insight whether the test or the code needs to be updated.

Certainly another problem surfaced here... there was a breaking change four weeks ago, and we just noticed now. I am not sure why the tests were green until a couple of days ago (https://github.com/zopefoundation/Products.GenericSetup/actions).

I am sorry, @mauritsvanrees, I cannot help here.

[jugmac00]

I think this is where we need changes in Products.GenericSetup:

Products.GenericSetup/src/Products/GenericSetup/utils.py

Lines 803 to 813 in a557e16

	if prop_map.get('type') in ('lines', 'tokens', 'ulines',
	'multiple selection'):
	prop_value = tuple(new_elements) or ()
	elif prop_map.get('type') == 'boolean':
	prop_value = self._convertToBoolean(self._getNodeText(child))
	else:
	# if we pass a *string* to _updateProperty, all other values
	# are converted to the right type
	prop_value = self._getNodeText(child)
	if six.PY2 and isinstance(prop_value, six.text_type):
	prop_value = prop_value.encode(self._encoding)

This is also the place which had been updated 3 years ago, when it was decided to let field2lines return bytes ( zopefoundation/Zope#206 ).

[jensens]

Slowly, times are coming to drop Python 2.7 support? At least, I get the impression if I read the bloated code in the linked PR.

[icemac]

Slowly, times are coming to drop Python 2.7 support? At least, I get the impression if I read the bloated code in the linked PR.

After the end of 2021 Zope 4 will no longer receive bug fixes. So probably Python 2 support can be dropped in 2022.

[mauritsvanrees]

I have released this in 2.1.4.