[SSSD] [PATCHSET] intg: sss_override

Nikolai Kondrashov Nikolai.Kondrashov at redhat.com
Fri Nov 13 19:42:15 UTC 2015


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


More information about the sssd-devel mailing list