search

zopefoundation / Products.ZSQLMethods

SQL method support for Zope.
Other
3 stars 14 forks source link

Bogus test? #3

Closed dataflake closed 7 years ago

dataflake commented 7 years ago

https://github.com/zopefoundation/Products.ZSQLMethods/blob/master/src/Shared/DC/ZRDB/tests/test_results.py#L142

This test wants to use the built-in hash function to assert that the r class is hashable. This works under Python 2. Under Python 3 this happens:

Error in test test_record_hash (Shared.DC.ZRDB.tests.test_results.TestResults)
Traceback (most recent call last):
  File "/usr/local/Cellar/python3/3.6.1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/local/Cellar/python3/3.6.1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py", line 601, in run
    testMethod()
  File "/Users/jens/src/Products.ZSQLMethods/src/Shared/DC/ZRDB/tests/test_results.py", line 145, in test_record_hash
    self.assert_(isinstance(hash(row), int))
TypeError: unhashable type: 'r'

I'm not sure what that test is trying to prove. Is it bogus?

(This is the last failing test on my local Python 3 compatibility branch)

tseaver commented 7 years ago

The use case would be to use a brain record as a dictionary key, or to add one or more to a set. I would guess we could look at ZCatalog to see how it might use them.

If needed:

    def __hash__(self):
        return hash(self.__data__)
dataflake commented 7 years ago

I couldn't find any explicitly defined __hash__ method defined in ExtensionClass.Base, so my guess is that some built-in fallback hashing mechanism is used which is different between Python2 and Python3.

Experimenting at the command line showed that simply hashing __data__ won't be enough. Currently under Python 2, even Record objects with the same __data__ and identical __record_schema__ hash differently.

A __hash__ method that will show similar behavior and hash any record object uniquely:

def __hash__(self):
    return hash(str(self))

What do you think?