[SSSD] [PATCH] TESTS: extend PAM responder unit test

Lukas Slebodnik lslebodn at redhat.com
Thu Nov 5 10:06:06 UTC 2015


On (05/11/15 10:52), Sumit Bose wrote:
>On Thu, Nov 05, 2015 at 10:29:41AM +0100, Lukas Slebodnik wrote:
>> On (05/11/15 09:50), Pavel Reichl wrote:
>> >
>> >
>> >On 11/05/2015 08:59 AM, Lukas Slebodnik wrote:
>> >>On (04/11/15 18:58), Pavel Reichl wrote:
>> >>>
>> >>>
>> >>>On 11/04/2015 06:41 PM, Sumit Bose wrote:
>> >>>>On Wed, Nov 04, 2015 at 06:00:34PM +0100, Pavel Reichl wrote:
>> >>>>>
>> >>>>>
>> >>>>>On 11/04/2015 03:02 PM, Lukas Slebodnik wrote:
>> >>>>>>On (04/11/15 14:40), Pavel Reichl wrote:
>> >>>>>
>> >>>>>>>
>> >>>>>>>Also one of few not yet tested corners of pam_reply() - 'Printing account
>> >>>>>>>expiration warning for sshd' requires pam_service to be sshd, but I can
>> >>>>>>>change when I will write another test (it might happen soon as pcech is
>> >>>>>>>working on similar change in adjacent code).
>> >>>>>>>
>> >>>>>>That's exactly the reason why the default value should not be sshd.
>> >>>>>
>> >>>>>I don't understand why this would be the reason can you elaborate?
>> >>>>
>> >>>>Because a default value should not lead to a code path which handles a
>> >>>>special case. The general PAM responder test should not run through the
>> >>>>'sshd' case in pam_reply() only if the service is set explicitly to
>> >>>>'sshd' this features should be tests.
>> >>>
>> >>>Sure, but just setting sshd would not lead to execution of this branch AFAIK
>> >>>at least pam verbosity would be needed to be set. As said before service
>> >>>value doesn't matter in current test so IMO this patch would only set service
>> >>>to more sane default however I agree with you that 'generic service name' is
>> >>>not a bad idea at all.
>> >>>
>> >>We spent enormous time with such unimportant patch.
>> >>
>> >>I pushed patch with Sumit's propsal "pam_test_service"
>> >>using one-liner rule.
>> >
>> >So this rule really allows you to:
>> >
>> >1) amend patch yourself,
>> >2) change authorship of the patch,
>> Sumit proposed "pam_test_service" so he is author.
>
>Pavel proposed to change the service name first.
>
>> 
>> >3) ack the patch yourself,
>> yes
>> 
>> >4) push the patch yourself,
>> yes
>> 
>> But only for oneliners
>
>yes, this rule exists but imo it should be only used to un-break build
There was a disagreement in the content of patch.
IMHO "ssh" was good enough and nothing needed to be changed.

But Pavel insisted on a change but there was disagreement about using "sshd".
So the oneliner was used for UNBLOCKING review and cut off unimportant
discussion. Because we could spent many hours about proper name.

LS


More information about the sssd-devel mailing list