URL: https://github.com/SSSD/sssd/pull/25
Title: #25: TESTS: Add integration tests for the proxy provider of sssd-secrets

lslebodn commented:
"""
On (20/09/16 05:02), Jakub Hrozek wrote:
>On Tue, Sep 20, 2016 at 04:45:53AM -0700, lslebodn wrote:
>> On (20/09/16 04:09), Jakub Hrozek wrote:
>> >I was getting random failures where the socket was not yet ready without
>> >the sleep, like this:
>> >    ConnectionError: ('Connection aborted.', error(2, 'No such file or
>> >    directory'))
>> >
>> >I think the test was just too fast. I pushed a new version that instead
>> >tries to connect to the socket and waits 0.2 sec if it cannot. The test
>> >gives up after a second.
>> >
>> Active checking of socket is much better approach.
>> I think you forgot to close socket :-)
>
>No, I was relying on garbage collector:
>    socket.close()
>    Close the socket. All future operations on the socket object will
>    fail. The remote end will receive no more data (after queued data is
>    flushed). Sockets are automatically closed when they are
>    garbage-collected.
>
>But it's a good idea to add close explicitly, because at least we get an
>exception if the socket was not connected.
>
>> 
>> I would also appreciate smaller waits (0.1 or even 0.05)
>> it's up to you.
>
>changed.
>
Actually, your version did not work properly because
socket.SOCK_DGRAM is not supported with unix socket.
As a result of this we were always waiting 1 second.

There were lots of iteration therefore
I changed that before pushing.
I also removed semocolon after break (it's not a ansi-C :-)
and replaced "unused variable" i with "_"
replace xrange with range (due to python3 compatibility)
I also changed checking of errors in exception to by python3
compatible

e.g.
-    assert err409.value.message.startswith("409")
+    assert str(err409.value).startswith("409")

The rest of test are not very python3 friendly due to
lack of pthon3-ldb or issues ith ldap. But this
test does not depend on ldap or ldb :-)
Sorry that I did not catch python3 related issues earlier

I hope you don't mint these small changes.
If you do not like it I will not do that in future.

I could not see any failure in CI
http://sssd-ci.duckdns.org/logs-test/job/4/54/summary.html
http://sssd-ci.duckdns.org/logs-test/job/4/55/summary.html
http://sssd-ci.duckdns.org/logs-test/job/4/56/summary.html
http://sssd-ci.duckdns.org/logs-test/job/4/57/summary.html
http://sssd-ci.duckdns.org/logs-test/job/4/58/summary.html

master:
* db0982c52294ee5ea08ed242d27660783fde29cd

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/25#issuecomment-248340608
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to