[PATCH] ldb: Use deterministic order of dict for tests
Lukas Slebodnik
lslebodn at fedoraproject.org
Mon Jan 21 21:33:37 UTC 2019
On (22/01/19 09:19), Andrew Bartlett via samba-technical wrote:
>On Mon, 2019-01-21 at 19:50 +0100, Lukas Slebodnik via samba-technical
>wrote:
>>
>> I spent more time debugging it on s390x and I can say that there is
>> not any
>> issue with passing env PYTHONHASHSEED to the test.
>>
>> The problem is with the value of env PYTHONHASHSEED.
>>
>> It deterministically fails with the value 1.
>>
>> So is there any reason why you insist on the value 1.
>> IMHO value 0 for PYTHONHASHSEED is perfectly valid and works on all
>> architectures in fedora
>> https://koji.fedoraproject.org/koji/buildinfo?buildID=1180702
>>
>> I can send a patch to change all values from 1 -> 0 if there are not
>> any
>> objection.
>
>I would really prefer to understand where the issue really comes from
>if at all possible, this is just getting even more strange and changing
>magic numbers. Even specifying PYTHONHASHSEED was meant to be a very
>short term hack, not a long-term solution, but so far a proper solution
>has been elusive...
>
I have no idea why PYTHONHASHSEED was added in commit 19a4d3ca692e
But following test failed for me when packaging libldb
def test_repr(self):
self.msg.dn = ldb.Dn(ldb.Ldb(), "dc=foo29")
self.msg["dc"] = b"foo"
if PY3:
self.assertIn(repr(self.msg), [
"Message({'dn': Dn('dc=foo29'), 'dc': MessageElement([b'foo'])})",
"Message({'dc': MessageElement([b'foo']), 'dn': Dn('dc=foo29')})",
])
self.assertIn(repr(self.msg.text), [
"Message({'dn': Dn('dc=foo29'), 'dc': MessageElement([b'foo'])}).text",
"Message({'dc': MessageElement([b'foo']), 'dn': Dn('dc=foo29')}).text",
])
else:
self.assertEqual(
repr(self.msg),
"Message({'dn': Dn('dc=foo29'), 'dc': MessageElement(['foo'])})")
self.assertEqual(
repr(self.msg.text),
"Message({'dn': Dn('dc=foo29'), 'dc': MessageElement(['foo'])}).text")
======================================================================
FAIL: test_repr (__main__.LdbMsgTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "tests/python/api.py", line 2322, in test_repr
"Message({'dn': Dn('dc=foo29'), 'dc': MessageElement(['foo'])})")
AssertionError: "Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})" != "Message({'dn': Dn('dc=foo29'), 'dc': MessageElement(['foo'])})"
So the issue is that implementation of py_ldb_msg_repr is not deterministic
and depends on the ordering of values in dictionary.
static PyObject *py_ldb_msg_repr(PyLdbMessageObject *self)
{
PyObject *dict = PyDict_New(), *ret, *repr;
if (PyDict_Update(dict, (PyObject *)self) != 0)
return NULL;
repr = PyObject_Repr(dict);
if (repr == NULL) {
Py_DECREF(dict);
return NULL;
}
ret = PyStr_FromFormat("Message(%s)", PyStr_AsUTF8(repr));
Py_DECREF(repr);
Py_DECREF(dict);
return ret;
}
And order of values in dictionary depends on hash randomization which can be
influenced by env PYTHONHASHSEED.
I briefly checked C-api for dictionary and I was not able to find
simple solution to have a deterministic order for dictionary.
https://docs.python.org/2.7/c-api/dict.html
But you might know better why the env variable was added with such value.
LS
More information about the samba-technical
mailing list