Hello,
Here is a patch for https://fedorahosted.org/freeipa/ticket/4029 I
added test cases for valid and invalid advice.Thanks, Gabe On Wed, Jan 14, 2015 at 10:23 AM, Tomas Babej <tba...@redhat.com> wrote: > > On 01/14/2015 06:13 PM, Gabe Alford wrote: > > On Wed, Jan 14, 2015 at 10:05 AM, Tomas Babej <tba...@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 <redhatri...@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 >> listFreeipa-devel@redhat.comhttps://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 >> >> >> 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 >> >> >> -- >> Tomas Babej >> Associate Software Engineer | Red Hat | Identity Management >> RHCE | Brno Site | IRC: tbabej | freeipa.org >> >> > > -- > Tomas Babej > Associate Software Engineer | Red Hat | Identity Management > RHCE | Brno Site | IRC: tbabej | freeipa.org > >
freeipa-rga-0039-ipatests-Add-tests-for-valid-and-invalid-ipa-advise.patch
Description: Binary data
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
