Update to persistent >= 4.2.3.

[icemac]

There seems to be regression in persistent 4.2.3+ breaking the test when using the C implementation.

The previously used version was pinned to 4.2.2 using Zope/master/versions.cfg.

Fixes #5 (currently it only demonstrates the problem)

[jamadden]

The tests are failing with this error:

  File "/home/travis/build/zopefoundation/Persistence/src/Persistence/tests/test_persistent.py", line 43, in add
    self.cache[obj._p_oid] = obj
  File "/home/travis/build/zopefoundation/Persistence/eggs/persistent-4.2.4.2-py3.4-linux-x86_64.egg/persistent/picklecache.py", line 111, in __setitem__
    raise TypeError("Cache values must be persistent objects.")
TypeError: Cache values must be persistent objects.

That was surprising because it indicates that the tests are using the pure-Python implementation of the PickleCache. But sure enough, that's what they're importing with from persistent.picklecache import PickleCache. That should be changed to from persistent import PickleCache to get the correct implementation.

This type checking and exception was added to the Python PickleCache way back in 2015 and persistent 4.1.0, as part of the efforts to make the Python implementations behave more like the C implementation. The C implementation has always enforced that type check AFAICS (well, or at least attempted to; the check is very poor for some reason).

So basically AFAICS the tests are broken not because of a regression in persistent but because they're legitimately wrong. They were getting away with bad behaviour before, but not any longer.

[hannosch]

I noticed the test failure before, but didn't follow up :( See #5.

[icemac]

@jamadden I think (but I could not prove it yet) that the following change in persistent causes this failure: zopefoundation/persistent@d03760b#diff-9349838838c54a85a15e0537529654b8R24

But maybe the failure here is only exists because it always uses the Python implementation of the pickle cache. And persistent is actually fine. I am going to investigate further by tomorrow.

[jamadden]

@jamadden I think (but I could not prove it yet) that the following change in persistent causes this failure: zopefoundation/persistent@d03760b#diff-9349838838c54a85a15e0537529654b8R24

Yes, it's absolutely possible that, by making the PickleCache even more strict and more like the cPickleCache (only accepting the kind of objects it knew about), the referenced change exposed the error here (said error being mixing and matching C and Python implementations of classes). The handling of PURE_PYTHON complicates things...

[icemac]

I switched the test to use the version of PickleCache matching the used persistent variant.

Now I get another error:

Error in test test_oid_jar_attrs (Persistence.tests.test_persistent.PersistenceTest)
Traceback (most recent call last):
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py", line 605, in run
    testMethod()
  File "/Users/mac/vcs/gocept/Persistence/src/Persistence/tests/test_persistent.py", line 113, in test_oid_jar_attrs
    del obj._p_oid
ValueError: can't delete _p_oid of cached object

@hannosch Could you please have a look into it because to were the last one who has changed the code.

[hannosch]

@icemac I think the whole test_oid_jar_attrs test is useless. Or rather I'm not quite sure why it tries to test deleting the _p_oid of an object in the first place. The behavior is already different between the C implementation and the Python implementation and now seems to have changed again.

I think I'd just delete the test and treat this as an internal detail of lowercase persistent.

[jamadden]

ValueError: can't delete _p_oid of cached object

That's expected in both cache implementations when used with their corresponding persistent type (C and Python) But when the C persistent object is put in the pure-Python PickleCache, as happened when the import was wrong, the C-level object didn't know that it was in the Python-level cache, and so the check didn't get done. Now that the imports and implementations match, the behaviour should be consistent, and the if side of that branch can probably be dropped.

[icemac]

@jamadden Thank you for your analysis of the problem, I fixed the tests according to your suggestions and modernised the code a bit.

[jamadden]

LGTM. I just had one comment about the version pin in setup.py and buildout.cfg not matching.

[jamadden]

setup.py does not match this. It seems to me like it probably should.

[icemac]

I was planning to remove this change before the merge because it was only necessary to force this new versions as it was not used by Zope because of the issue now fixed in this PR.

I'll have a look if this change is actually needed and has to be ported to setup.py.