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

jhrozek commented:
"""
On Tue, Sep 20, 2016 at 08:40:08AM -0700, lslebodn wrote:
> 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

Great, thank you for doing this, I'm just glad we have tests in master
and can add regression tests for other stuff that Fabiano is fixing..

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/25#issuecomment-248359999
_______________________________________________
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