[SSSD] [PATCHSET] intg: sss_override

Jakub Hrozek jhrozek at redhat.com
Sat Nov 14 12:56:34 UTC 2015


On Fri, Nov 13, 2015 at 09:42:15PM +0200, Nikolai Kondrashov wrote:
> On 11/13/2015 07:06 PM, Pavel Reichl wrote:
> >On 11/13/2015 05:44 PM, Nikolai Kondrashov wrote:
> >>On 11/13/2015 06:19 PM, Pavel Reichl wrote:
> >>>Thanks Nick, updated patch attached.
> >>
> >>Thanks, Pavel, this looks really neat now! I have only two comments.
> >>
> >>>+def test_simple_user_override(ldap_conn, env_simple_user_override):
> >>>+    """Test entries are overriden"""
> >>>+
> >>>+
> >>
> >>Shouldn't there be an actual test here?
> >Sorry :-)
> 
> No problem :)
> 
> >>
> >>>+# Regression test for bug #2802
> >>>+# sss_override segfaults when accidentally adding --help flag to some commands
> >>>+
> >>>+
> >>>+def test_regr_2802_override(request, ldap_conn):
> >>>+    prepare_sssd(request, ldap_conn)
> >>
> >>I had no idea this could actually work, I mean getting "request" in a test.
> >>Can you give a link to documentation where this is explained?
> >
> >I don't have any, I was in a hurry and I did a mistake.
> 
> Ah, fine.
> 
> >>Otherwise, could you please move the setup to a proper fixture? Tests are not
> >>supposed to have setup/teardown inside, AFAIK, unless this is really official.
> >>
> >>>+
> >>>+    subprocess.check_call(["sss_override", "user-del", "--help"])
> >
> >Sure.
> 
> Thanks a lot!
> 
> Looks fine to me now. Tests passed on my machine both all together, and each
> separately.  Great job!
> 
> ACK
> 
> Nick

Pushed to master so far: 8d1dcb6af723f2968410c4b088d06d63d02b4fea

We can push all these to sssd-1-13 later, when CI ressurects.


More information about the sssd-devel mailing list