[PATCH] ldb: Use deterministic order of dict for tests

[Lukas Slebodnik]

On (19/01/19 10:27), Douglas Bagnall via samba-technical wrote:
>hi Lukas
>
>On 19/01/19 8:48 AM, Lukas Slebodnik via samba-technical wrote:
>> ehlo,
>> 
>> Unit test failed for me in fedora on s390x
>> https://kojipkgs.fedoraproject.org//work/tasks/6615/32106615/build.log
>> https://kojipkgs.fedoraproject.org//work/tasks/7969/32107969/build.log
>> 
>> But there is also a reasonable reproducer on x86_64
>> 
>> sh$ cat > reproducer.py <<EOF
>> import ldb
>> msg = ldb.Message()
>> msg.dn = ldb.Dn(ldb.Ldb(), "dc=foo29")
>> msg["dc"] = b"foo"
>> print(repr(msg))
>> EOF
>> 
>> sh$ PYTHONHASHSEED=2 python2 reproducer.py
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> 
>> sh$ PYTHONHASHSEED=1 python2 reproducer.py
>> Message({'dn': Dn('dc=foo29'), 'dc': MessageElement(['foo'])})
>> 
>> sh$  for i in {1..10}; do PYTHONHASHSEED=random python2
>> reproducer.py; done
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> Message({'dn': Dn('dc=foo29'), 'dc': MessageElement(['foo'])})
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> Message({'dn': Dn('dc=foo29'), 'dc': MessageElement(['foo'])})
>> Message({'dc': MessageElement(['foo']), 'dn': Dn('dc=foo29')})
>> 
>> I was thinking about fixing it in pyldb.c but IMHO it does not worth
>> to spend time with fixing it in C in case of trivial workaround when
>> executing tests.
>
>yep. However in lib/ldb/Makefile (and other Makefiles) we have:
>
>WAF=PYTHONHASHSEED=1 WAF_MAKE=1 $(WAF_BINARY)
>
>[...]
>
>test:
>        $(WAF) test $(TEST_OPTIONS)
>
>
>Using, as you can see, "1" for the seed (it is there in the s390x
>build logs). Evidently that doesn't fall through to the underlying
>tests, at least not in all cases. But maybe sometimes it does.
>
>Which leads me to the two things I don't like so much about this patch:
>

I'll try to find out why it was not passed.
I had to be blind that I did not notice it.
I opened that file few times because of other issue :-)

https://lists.samba.org/archive/samba-technical/2019-January/132072.html

>> --- a/lib/ldb/wscript
>> +++ b/lib/ldb/wscript
>> @@ -559,7 +559,7 @@ def test(ctx):
>>      if env.HAVE_LMDB:
>>          pyret = samba_utils.RUN_PYTHON_TESTS(
>>              ['tests/python/api.py', 'tests/python/index.py'],
>> -            extra_env={'SELFTEST_PREFIX': test_prefix})
>> +            extra_env={'SELFTEST_PREFIX': test_prefix, 'PYTHONHASHSEED': '0'})
>>      else:
>>          pyret = 0
>>      print("Python testsuite returned %d" % pyret)
>
>1. It would be better to use "'PYTHONHASHSEED': '1'", for consistency.
>
>2. It would be better to have a more general solution that fixed it for
>ever for everyone, not just the lmdb ldb tests. Like maybe here:
>
>../buildtools/wafsamba/samba_utils.py:395:def LOAD_ENVIRONMENT()
>
>
>I don't know. There *might* be someone on the list who understands waf.
>

I'll try to spend more time with analysis.
Thank you for review.

LS