[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain martbab commented: """ master: * 10494b1bb34b6ff9c1b810cc0739c761b017202c Tests: Basic coverage with tree root domain """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-283316659 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain gkaihorodova commented: """ Thanks you for review. Let's hope for the best . """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-283057505 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain martbab commented: """ The patch looks ok, let's hope that our CI will play nice with it. """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-283054583 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain gkaihorodova commented: """ Bump for review """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-282664683 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain gkaihorodova commented: """ Yes, sure I'll work on these issues """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-278975336 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain martbab commented: """ Well you still have some issues to fix, notably the failing Travis CI and the not-so nice multiline-string literal. """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-278973164 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain gkaihorodova commented: """ Thank you for explanation and tips. I noticed it as well and I agree that it (and not only that) worth refactoring. Yes, my PR is more or less copy-paste, because I was following existing pattern in the code. Also I think PR can be pushed and at the same time feel free to open ticket/request or whatever is more suitable for refactoring task and signed it to me. Please, don t be grieve, it makes me sad """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-278961376 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain martbab commented: """ If you look at the test cases (e.g. test_login_ipa-user, test_login_ad_user, test_login_subdomain_user are the 'best: examples) you can see that the function body is the same code copy-pasted with slight alterations so that it works for the new case. Your patch adds a *fourth* level of copy-pasta to the code, which is something that grieves me greatly. Clearly, you can group the common code into a private method that can be only called with the use-case specific parameters for each test case. Or you can expand the existing mixing hierarchy to achieve this. Then it would also be simpler to extend the test cases for tree-root domains. """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-278939703 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain gkaihorodova commented: """ Can you be a little bit more specific about "triplication of the test cases ", please. Because, to be honest, I'm having hard time trying to navigate myself there. """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-278606153 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#448][comment] Tests: Basic coverage with tree root domain
URL: https://github.com/freeipa/freeipa/pull/448 Title: #448: Tests: Basic coverage with tree root domain martbab commented: """ I have quickly skimmed through code and have one comment. Also, I have noticed the extreme code triplication of the test cases. I think that this warrants some refactoring first before adding tree-root domain tests. """ See the full comment at https://github.com/freeipa/freeipa/pull/448#issuecomment-278381632 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
