[Freeipa-devel] [PATCH 0039] Add test case for unsupported arg for ipa-advise

[Tomas Babej]

On 01/14/2015 06:13 PM, Gabe Alford wrote:
> On Wed, Jan 14, 2015 at 10:05 AM, Tomas Babej <tbabej at redhat.com
> <mailto:tbabej at redhat.com>> wrote:
>
>
>     On 01/14/2015 06:00 PM, Tomas Babej wrote:
>>
>>     On 01/14/2015 05:37 PM, Tomas Babej wrote:
>>>
>>>     On 01/14/2015 02:55 PM, Gabe Alford wrote:
>>>>     Hello,
>>>>
>>>>            In looking into
>>>>     https://fedorahosted.org/freeipa/ticket/4029 I am wondering if
>>>>     there should be separate ipa-advise test, Yes/No? Could be
>>>>     handy in the future to test more ipa-advise output? Or should
>>>>     this test be added to the test_legacy_clients.py?
>>>>
>>>>     Thanks,
>>>>
>>>>     Gabe  
>>>>
>>>>     On Tue, Dec 2, 2014 at 9:21 PM, Gabe Alford
>>>>     <redhatrises at gmail.com <mailto:redhatrises at gmail.com>> wrote:
>>>>
>>>>         Hello,
>>>>
>>>>         I was going to try my hand at attempting a patch for
>>>>         ipa-tests. However in wanting to test my patch, I am not
>>>>         sure how to run ipa-tests to check if it works or not.
>>>>         Documentation is not really clear on what needs to be done
>>>>         to start a test and run a test. This is for
>>>>         https://fedorahosted.org/freeipa/ticket/4029
>>>>
>>>>         I have attached the patch that I have yet to really test
>>>>         with ipa-test. Any help on how to test the patch running
>>>>         ipa-tests would be great. Of course, if one of the
>>>>         reviewers looks at the patch and looks good, then I would
>>>>         be happy with that as well.
>>>>
>>>>         Thanks,
>>>>
>>>>         Gabe
>>>>
>>>>
>>>>
>>>>
>>>>     _______________________________________________
>>>>     Freeipa-devel mailing list
>>>>     Freeipa-devel at redhat.com <mailto:Freeipa-devel at redhat.com>
>>>>     https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>
>>>     Hello,
>>>
>>>     TL;DR: feel free to create a separate ipa-advise test file. Test
>>>     requested in this ticket really does not belong to the legacy
>>>     clients feature test.
>>>
>>>     As for the any new tests that might come: I think tests for
>>>     ipa-advise that are specific to that particular feature should
>>>     be tested with that feature, more so, if they contain parts that
>>>     are supposed to work copy-pasted. If a tests, however, tests a
>>>     general behaviour of ipa-advise, it should live in the
>>>     ipa-advise namespace, hence separate test file.
>>>
>>>     HTH,
>>>
>>>     -- 
>>>     Tomas Babej
>>>     Associate Software Engineer | Red Hat | Identity Management
>>>     RHCE | Brno Site | IRC: tbabej | freeipa.org <http://freeipa.org> 
>>
>>     The attached patch looks fine, although, please also test for a
>>     non-zero return code number.
>>
>
>     Upon hitting send I noticed you did not include raiseonerr=False
>     into the run_command call. You need to do that, otherwise a
>     exception will be raised, since ipa-advise exited with non-zero
>     return code.
>
> Thanks Tomas.
>
> Which do you prefer: a test_advise.py or an update to the existing patch?

A new test file, as I pointed out in the second email :) sorry for
splitting.

However, it would be the best if you could spin up a positive test as
well (maybe listing out available advices), not just this negative one,
to justify the overhead reinstalling IPA for testing this feature.


>>
>>     -- 
>>     Tomas Babej
>>     Associate Software Engineer | Red Hat | Identity Management
>>     RHCE | Brno Site | IRC: tbabej | freeipa.org <http://freeipa.org> 
>
>     -- 
>     Tomas Babej
>     Associate Software Engineer | Red Hat | Identity Management
>     RHCE | Brno Site | IRC: tbabej | freeipa.org <http://freeipa.org> 
>
>

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150114/2bf074ba/attachment.htm>