Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi Oleg! As we discussed this morning there're still some issues [1][2] but I think the patch set is in reasonably good state and I'm not sure that the issues are bugs in IPA or in tests. This can be investigated and fixed later. ACK. Pushed to master: * bbac233b5ee487ab0e035cf0b861144769a0b738 tests: Fixed method failures during second call for the method * 2f6ffa326adb4d4e9152463ffa733d559f7be2af tests: Added basic constraints extension to the CA certs * 0c635686ddc6dbba54285a9c7844e12342083b9b tests: Added generation of missing certs * 38ad864342bb3fcbf65397b763c240f034f3e2c7 tests: Updated ipa server installation stdin text * c0e16aa3b9c380fb7936dc18c4bfc04e7f8327b5 tests: Create a method that cleans all ipa certs * 48ca465a12a91977eb57bb791cf0b098e5e5a4b3 tests: Added teardown methods for server and replica installation * 725d8d0cac16f6a41ad54ae319e3822d44047031 tests: Removed call for install method from parent class * fad6ec8256a97fbf06b3e4509d93af1d159e6b81 tests: Adapted installation methods to utilize methods from tasks * 84db13f676771746f9ab7769073837a29f6de464 tests: Fixed incorrect assert in verify_installation * a81d8472042f4909020c073ecb00a37d8e05ec33 tests: Applied correct teardown methods * 759bbcdfcbeade91c77b201c439c939d6477cd08 tests: Removed outdated command options test * d17d13d77a7c09cbc99c8bb0a3f7af3b72da8aca tests: Added necessary getkeytabs calls to fixtures * 24f218f4ebe203213eebede59ff79b89c657ee76 tests: Added necessary xfails * e0b67dfa7e957cc134043b265b87ed71bb09a7d3 tests: Updated master and replica installation methods to enable negative testing * 9217bcc871468615110c85b1131b62735f9e5092 tests: Made unapply_fixes call optional at master uninstallation * bb4205b582038669888544786a5611b18e52bf42 tests: Enabled negative testing for cleaning replication agreements * dbf0d141c5a4d1ccd1b681ac49cb57e47234aaa4 tests: Replaced hardcoded certutil with imported from paths * b8cf212e8bbe301f6d44551ecac063d12a042520 tests: Replaced unused setUp method with install * 804aae81966f23e307ec86364489f808fd0c3357 tests: fixed expects of incorrect error messages * 43994e669743bb8f54e32b52f2410ebde3660f04 tests: Fixed Usage of improper certs in ca-less tests * b8968d923cedbbeb931c3ed33b81b299a55baf4a tests: Implemented check for domainlevel before installation verification * 106f37c26f64492f771214409d229feb5d70a113 tests: Standardized replica_preparation in test_no_certs * 8be0906b04bbf995af6326b560151d15901b544e tests: added verbose assert to test_service_disable_doesnt_revoke * f1f94a7b9fe354d93f31ce8cd606d985dd44703b tests: fixed super method invocation * 7412f0cb20801e1608f8cf388210e57ef7d27497 tests: fixed certinstall method * 9870c5804a65ae320ebbcb313f8facb21963f710 tests: Reverted erroneous asserts in 4 tests * 47c808afa35f0708ca00ac8e98851c9f8d75badc tests: Fixed code styling in caless tests to make pep8 happy [1] https://fedorahosted.org/freeipa/ticket/6346 [2] https://fedorahosted.org/freeipa/ticket/6348 On 22/09/16 12:55, Oleg Fayans wrote: Fixed patch N 41 On 09/21/2016 04:21 PM, Oleg Fayans wrote: Patch-0076 rebased to current master On 09/21/2016 02:41 PM, Oleg Fayans wrote: Hi David, As per your comments the patches were once again refactored. I am attaching the full set of them, please ignore any previous versions The patches apply cleanly on master and pylint swallows the resulting code silently On 09/12/2016 09:51 AM, David Kupka wrote: Hi Oleg, thank you, now it's completely different game. Please add prefix to commit message summaries. Simply prepending "tests: " should be OK. 0041 - -h is deprecated in favor of -H. 0062 - 0068 - LGTM 0069 - I see 2 unrelated changes in the patch, please split them: - 1 - certutil - > paths.CERTUTIL - 2 - assert 0070 - I see 2 unrelated changes in the patch, please split them: - 1 - teardown - 2 - TestReplicaInstall.setUp -> TestReplicaInstall.install 0071 - typos in commit message, I see 5 unrelated changes in that patch: - 1 - error messages in assert - 2 - certificates used - 3 - verify_installation called only in DOMAIN_LEVEL_0. - 4 - TestCertinstall.install - 5 - TestCertinstall.certinstall 0072 - 0077 - LGTM On 09/09/16 15:22, Oleg Fayans wrote: Hi David, team According to your suggestions I've splitted my commits so that each commit addresses some particular problem. One patch (0071) still contains several unrelated fixes, but they mostly reflect changes in error messages and really small but numerous bugfixes that I did not consider worthy of a separate commit each. Please, whenever you have a free time take a look at this new bunch of patches. Thanks! On 09/06/2016 04:41 PM, David Kupka wrote: Hi Oleg! 0013 - It looks like there are two unrelated changes, addition of CRL distribution extension and creating certificate signed by no longer existing CA. Please create separate patch for each of the changes, and describe the change and reason for it in commit messages. 0014 - Co
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Fixed patch N 41 On 09/21/2016 04:21 PM, Oleg Fayans wrote: Patch-0076 rebased to current master On 09/21/2016 02:41 PM, Oleg Fayans wrote: Hi David, As per your comments the patches were once again refactored. I am attaching the full set of them, please ignore any previous versions The patches apply cleanly on master and pylint swallows the resulting code silently On 09/12/2016 09:51 AM, David Kupka wrote: Hi Oleg, thank you, now it's completely different game. Please add prefix to commit message summaries. Simply prepending "tests: " should be OK. 0041 - -h is deprecated in favor of -H. 0062 - 0068 - LGTM 0069 - I see 2 unrelated changes in the patch, please split them: - 1 - certutil - > paths.CERTUTIL - 2 - assert 0070 - I see 2 unrelated changes in the patch, please split them: - 1 - teardown - 2 - TestReplicaInstall.setUp -> TestReplicaInstall.install 0071 - typos in commit message, I see 5 unrelated changes in that patch: - 1 - error messages in assert - 2 - certificates used - 3 - verify_installation called only in DOMAIN_LEVEL_0. - 4 - TestCertinstall.install - 5 - TestCertinstall.certinstall 0072 - 0077 - LGTM On 09/09/16 15:22, Oleg Fayans wrote: Hi David, team According to your suggestions I've splitted my commits so that each commit addresses some particular problem. One patch (0071) still contains several unrelated fixes, but they mostly reflect changes in error messages and really small but numerous bugfixes that I did not consider worthy of a separate commit each. Please, whenever you have a free time take a look at this new bunch of patches. Thanks! On 09/06/2016 04:41 PM, David Kupka wrote: Hi Oleg! 0013 - It looks like there are two unrelated changes, addition of CRL distribution extension and creating certificate signed by no longer existing CA. Please create separate patch for each of the changes, and describe the change and reason for it in commit messages. 0014 - Could you please split the patch to "numerous" commit each fixing one error? Please also describe each fix so everyone has at least vague idea about the patch without reading its code. Also why do you introduce global variable config, I don't see its used anywhere. 0039 - It looks like multiple different changes and commit message says nothing again. Please split and describe what did you change and why. 0041 - Looks like weird workaround to me. It would be better to investigate the root cause and fix it. Or at least describe the cause in commit message and code comment if it can't be fixed. Also "-h is deprecated in favor of -H" says man 1 ldapmodify. On 05/09/16 14:32, Oleg Fayans wrote: Hi guys, Finally the ca-less tests are stable. Here in the attachment is the full set of necessary patches. On 08/09/2016 10:57 AM, Oleg Fayans wrote: Hi all, Bump for the review of the 0013 patch. The script it addresses can be reused in some WebUI tests - one more reason to have it reviewed/merged The rest patches should be re-tested, since they were prepared a good while ago On 05/10/2016 05:08 PM, Oleg Fayans wrote: Hi David, After quite a while and some more struggles here comes the updated version of the patch together with other patches fixing things in ipatests/test_integration/tasks.py Server and replica installation was refactored in a way to utilize the code from tasks.py as much as it is possible The full set of necessary patches is attached On 04/20/2016 10:35 AM, David Kupka wrote: On 19/04/16 11:13, Oleg Fayans wrote: OK, that one, though passing lint, did not actually work. I gave up my attempts to define method decorators inside the class. Now it passes lint AND works:) Hi Oleg! 1) Current commit message is useless. Please use it to describe what is the point of the patch. 2) $ git show -U0 | pep8 --diff ./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:820:5: E303 too many blank lines (2) ./ipatests/test_integration/test_caless.py:825:80: E501 line too long (80 > 79 characters) ./ipatests/test_integration/test_caless.py:1035:44: E225 missing whitespace around operator 3) Isn't there a way to do this with pytest's fixtures? +def server_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +args[0].uninstall_server() +return wrapped + +def replica_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +# Uninstall replica +replica = args[0].replicas[0] +tasks.kinit_admin(args[0].master) +args[0].uninstall_server(replica) +args[0].master.run_command(['ipa-replica-manage', 'del', +replica.hostname, '--force'], + raiseonerr=False) +args[0].master.run_c
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Patch-0076 rebased to current master On 09/21/2016 02:41 PM, Oleg Fayans wrote: Hi David, As per your comments the patches were once again refactored. I am attaching the full set of them, please ignore any previous versions The patches apply cleanly on master and pylint swallows the resulting code silently On 09/12/2016 09:51 AM, David Kupka wrote: Hi Oleg, thank you, now it's completely different game. Please add prefix to commit message summaries. Simply prepending "tests: " should be OK. 0041 - -h is deprecated in favor of -H. 0062 - 0068 - LGTM 0069 - I see 2 unrelated changes in the patch, please split them: - 1 - certutil - > paths.CERTUTIL - 2 - assert 0070 - I see 2 unrelated changes in the patch, please split them: - 1 - teardown - 2 - TestReplicaInstall.setUp -> TestReplicaInstall.install 0071 - typos in commit message, I see 5 unrelated changes in that patch: - 1 - error messages in assert - 2 - certificates used - 3 - verify_installation called only in DOMAIN_LEVEL_0. - 4 - TestCertinstall.install - 5 - TestCertinstall.certinstall 0072 - 0077 - LGTM On 09/09/16 15:22, Oleg Fayans wrote: Hi David, team According to your suggestions I've splitted my commits so that each commit addresses some particular problem. One patch (0071) still contains several unrelated fixes, but they mostly reflect changes in error messages and really small but numerous bugfixes that I did not consider worthy of a separate commit each. Please, whenever you have a free time take a look at this new bunch of patches. Thanks! On 09/06/2016 04:41 PM, David Kupka wrote: Hi Oleg! 0013 - It looks like there are two unrelated changes, addition of CRL distribution extension and creating certificate signed by no longer existing CA. Please create separate patch for each of the changes, and describe the change and reason for it in commit messages. 0014 - Could you please split the patch to "numerous" commit each fixing one error? Please also describe each fix so everyone has at least vague idea about the patch without reading its code. Also why do you introduce global variable config, I don't see its used anywhere. 0039 - It looks like multiple different changes and commit message says nothing again. Please split and describe what did you change and why. 0041 - Looks like weird workaround to me. It would be better to investigate the root cause and fix it. Or at least describe the cause in commit message and code comment if it can't be fixed. Also "-h is deprecated in favor of -H" says man 1 ldapmodify. On 05/09/16 14:32, Oleg Fayans wrote: Hi guys, Finally the ca-less tests are stable. Here in the attachment is the full set of necessary patches. On 08/09/2016 10:57 AM, Oleg Fayans wrote: Hi all, Bump for the review of the 0013 patch. The script it addresses can be reused in some WebUI tests - one more reason to have it reviewed/merged The rest patches should be re-tested, since they were prepared a good while ago On 05/10/2016 05:08 PM, Oleg Fayans wrote: Hi David, After quite a while and some more struggles here comes the updated version of the patch together with other patches fixing things in ipatests/test_integration/tasks.py Server and replica installation was refactored in a way to utilize the code from tasks.py as much as it is possible The full set of necessary patches is attached On 04/20/2016 10:35 AM, David Kupka wrote: On 19/04/16 11:13, Oleg Fayans wrote: OK, that one, though passing lint, did not actually work. I gave up my attempts to define method decorators inside the class. Now it passes lint AND works:) Hi Oleg! 1) Current commit message is useless. Please use it to describe what is the point of the patch. 2) $ git show -U0 | pep8 --diff ./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:820:5: E303 too many blank lines (2) ./ipatests/test_integration/test_caless.py:825:80: E501 line too long (80 > 79 characters) ./ipatests/test_integration/test_caless.py:1035:44: E225 missing whitespace around operator 3) Isn't there a way to do this with pytest's fixtures? +def server_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +args[0].uninstall_server() +return wrapped + +def replica_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +# Uninstall replica +replica = args[0].replicas[0] +tasks.kinit_admin(args[0].master) +args[0].uninstall_server(replica) +args[0].master.run_command(['ipa-replica-manage', 'del', +replica.hostname, '--force'], + raiseonerr=False) +args[0].master.run_command(['ipa', 'host-del', +
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi David, As per your comments the patches were once again refactored. I am attaching the full set of them, please ignore any previous versions The patches apply cleanly on master and pylint swallows the resulting code silently On 09/12/2016 09:51 AM, David Kupka wrote: Hi Oleg, thank you, now it's completely different game. Please add prefix to commit message summaries. Simply prepending "tests: " should be OK. 0041 - -h is deprecated in favor of -H. 0062 - 0068 - LGTM 0069 - I see 2 unrelated changes in the patch, please split them: - 1 - certutil - > paths.CERTUTIL - 2 - assert 0070 - I see 2 unrelated changes in the patch, please split them: - 1 - teardown - 2 - TestReplicaInstall.setUp -> TestReplicaInstall.install 0071 - typos in commit message, I see 5 unrelated changes in that patch: - 1 - error messages in assert - 2 - certificates used - 3 - verify_installation called only in DOMAIN_LEVEL_0. - 4 - TestCertinstall.install - 5 - TestCertinstall.certinstall 0072 - 0077 - LGTM On 09/09/16 15:22, Oleg Fayans wrote: Hi David, team According to your suggestions I've splitted my commits so that each commit addresses some particular problem. One patch (0071) still contains several unrelated fixes, but they mostly reflect changes in error messages and really small but numerous bugfixes that I did not consider worthy of a separate commit each. Please, whenever you have a free time take a look at this new bunch of patches. Thanks! On 09/06/2016 04:41 PM, David Kupka wrote: Hi Oleg! 0013 - It looks like there are two unrelated changes, addition of CRL distribution extension and creating certificate signed by no longer existing CA. Please create separate patch for each of the changes, and describe the change and reason for it in commit messages. 0014 - Could you please split the patch to "numerous" commit each fixing one error? Please also describe each fix so everyone has at least vague idea about the patch without reading its code. Also why do you introduce global variable config, I don't see its used anywhere. 0039 - It looks like multiple different changes and commit message says nothing again. Please split and describe what did you change and why. 0041 - Looks like weird workaround to me. It would be better to investigate the root cause and fix it. Or at least describe the cause in commit message and code comment if it can't be fixed. Also "-h is deprecated in favor of -H" says man 1 ldapmodify. On 05/09/16 14:32, Oleg Fayans wrote: Hi guys, Finally the ca-less tests are stable. Here in the attachment is the full set of necessary patches. On 08/09/2016 10:57 AM, Oleg Fayans wrote: Hi all, Bump for the review of the 0013 patch. The script it addresses can be reused in some WebUI tests - one more reason to have it reviewed/merged The rest patches should be re-tested, since they were prepared a good while ago On 05/10/2016 05:08 PM, Oleg Fayans wrote: Hi David, After quite a while and some more struggles here comes the updated version of the patch together with other patches fixing things in ipatests/test_integration/tasks.py Server and replica installation was refactored in a way to utilize the code from tasks.py as much as it is possible The full set of necessary patches is attached On 04/20/2016 10:35 AM, David Kupka wrote: On 19/04/16 11:13, Oleg Fayans wrote: OK, that one, though passing lint, did not actually work. I gave up my attempts to define method decorators inside the class. Now it passes lint AND works:) Hi Oleg! 1) Current commit message is useless. Please use it to describe what is the point of the patch. 2) $ git show -U0 | pep8 --diff ./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:820:5: E303 too many blank lines (2) ./ipatests/test_integration/test_caless.py:825:80: E501 line too long (80 > 79 characters) ./ipatests/test_integration/test_caless.py:1035:44: E225 missing whitespace around operator 3) Isn't there a way to do this with pytest's fixtures? +def server_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +args[0].uninstall_server() +return wrapped + +def replica_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +# Uninstall replica +replica = args[0].replicas[0] +tasks.kinit_admin(args[0].master) +args[0].uninstall_server(replica) +args[0].master.run_command(['ipa-replica-manage', 'del', +replica.hostname, '--force'], + raiseonerr=False) +args[0].master.run_command(['ipa', 'host-del', +replica.hostname], + raiseonerr=Fal
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi Oleg, thank you, now it's completely different game. Please add prefix to commit message summaries. Simply prepending "tests: " should be OK. 0041 - -h is deprecated in favor of -H. 0062 - 0068 - LGTM 0069 - I see 2 unrelated changes in the patch, please split them: - 1 - certutil - > paths.CERTUTIL - 2 - assert 0070 - I see 2 unrelated changes in the patch, please split them: - 1 - teardown - 2 - TestReplicaInstall.setUp -> TestReplicaInstall.install 0071 - typos in commit message, I see 5 unrelated changes in that patch: - 1 - error messages in assert - 2 - certificates used - 3 - verify_installation called only in DOMAIN_LEVEL_0. - 4 - TestCertinstall.install - 5 - TestCertinstall.certinstall 0072 - 0077 - LGTM On 09/09/16 15:22, Oleg Fayans wrote: Hi David, team According to your suggestions I've splitted my commits so that each commit addresses some particular problem. One patch (0071) still contains several unrelated fixes, but they mostly reflect changes in error messages and really small but numerous bugfixes that I did not consider worthy of a separate commit each. Please, whenever you have a free time take a look at this new bunch of patches. Thanks! On 09/06/2016 04:41 PM, David Kupka wrote: Hi Oleg! 0013 - It looks like there are two unrelated changes, addition of CRL distribution extension and creating certificate signed by no longer existing CA. Please create separate patch for each of the changes, and describe the change and reason for it in commit messages. 0014 - Could you please split the patch to "numerous" commit each fixing one error? Please also describe each fix so everyone has at least vague idea about the patch without reading its code. Also why do you introduce global variable config, I don't see its used anywhere. 0039 - It looks like multiple different changes and commit message says nothing again. Please split and describe what did you change and why. 0041 - Looks like weird workaround to me. It would be better to investigate the root cause and fix it. Or at least describe the cause in commit message and code comment if it can't be fixed. Also "-h is deprecated in favor of -H" says man 1 ldapmodify. On 05/09/16 14:32, Oleg Fayans wrote: Hi guys, Finally the ca-less tests are stable. Here in the attachment is the full set of necessary patches. On 08/09/2016 10:57 AM, Oleg Fayans wrote: Hi all, Bump for the review of the 0013 patch. The script it addresses can be reused in some WebUI tests - one more reason to have it reviewed/merged The rest patches should be re-tested, since they were prepared a good while ago On 05/10/2016 05:08 PM, Oleg Fayans wrote: Hi David, After quite a while and some more struggles here comes the updated version of the patch together with other patches fixing things in ipatests/test_integration/tasks.py Server and replica installation was refactored in a way to utilize the code from tasks.py as much as it is possible The full set of necessary patches is attached On 04/20/2016 10:35 AM, David Kupka wrote: On 19/04/16 11:13, Oleg Fayans wrote: OK, that one, though passing lint, did not actually work. I gave up my attempts to define method decorators inside the class. Now it passes lint AND works:) Hi Oleg! 1) Current commit message is useless. Please use it to describe what is the point of the patch. 2) $ git show -U0 | pep8 --diff ./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank lines, found 1 ./ipatests/test_integration/test_caless.py:820:5: E303 too many blank lines (2) ./ipatests/test_integration/test_caless.py:825:80: E501 line too long (80 > 79 characters) ./ipatests/test_integration/test_caless.py:1035:44: E225 missing whitespace around operator 3) Isn't there a way to do this with pytest's fixtures? +def server_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +args[0].uninstall_server() +return wrapped + +def replica_install_teardown(func): +def wrapped(*args): +try: +func(*args) +finally: +# Uninstall replica +replica = args[0].replicas[0] +tasks.kinit_admin(args[0].master) +args[0].uninstall_server(replica) +args[0].master.run_command(['ipa-replica-manage', 'del', +replica.hostname, '--force'], + raiseonerr=False) +args[0].master.run_command(['ipa', 'host-del', +replica.hostname], + raiseonerr=False) +return wrapped + There is a standard pytest method called 'method_teardown', that is indent to be executed after each test method, but with our setup it does not work. 4) Is it necessary to create the $TEST_DIR in the test? Isn't it created by the framework?
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi David, team
According to your suggestions I've splitted my commits so that each
commit addresses some particular problem. One patch (0071) still
contains several unrelated fixes, but they mostly reflect changes in
error messages and really small but numerous bugfixes that I did not
consider worthy of a separate commit each. Please, whenever you have a
free time take a look at this new bunch of patches.
Thanks!
On 09/06/2016 04:41 PM, David Kupka wrote:
Hi Oleg!
0013 - It looks like there are two unrelated changes, addition of CRL
distribution extension and creating certificate signed by no longer
existing CA. Please create separate patch for each of the changes, and
describe the change and reason for it in commit messages.
0014 - Could you please split the patch to "numerous" commit each fixing
one error? Please also describe each fix so everyone has at least vague
idea about the patch without reading its code. Also why do you introduce
global variable config, I don't see its used anywhere.
0039 - It looks like multiple different changes and commit message says
nothing again. Please split and describe what did you change and why.
0041 - Looks like weird workaround to me. It would be better to
investigate the root cause and fix it. Or at least describe the cause in
commit message and code comment if it can't be fixed. Also "-h is
deprecated in favor of -H" says man 1 ldapmodify.
On 05/09/16 14:32, Oleg Fayans wrote:
Hi guys,
Finally the ca-less tests are stable. Here in the attachment is the full
set of necessary patches.
On 08/09/2016 10:57 AM, Oleg Fayans wrote:
Hi all,
Bump for the review of the 0013 patch. The script it addresses can be
reused in some WebUI tests - one more reason to have it reviewed/merged
The rest patches should be re-tested, since they were prepared a good
while ago
On 05/10/2016 05:08 PM, Oleg Fayans wrote:
Hi David,
After quite a while and some more struggles here comes the updated
version of the patch together with other patches fixing things in
ipatests/test_integration/tasks.py
Server and replica installation was refactored in a way to utilize the
code from tasks.py as much as it is possible
The full set of necessary patches is attached
On 04/20/2016 10:35 AM, David Kupka wrote:
On 19/04/16 11:13, Oleg Fayans wrote:
OK, that one, though passing lint, did not actually work. I gave
up my
attempts to define method decorators inside the class. Now it passes
lint AND works:)
Hi Oleg!
1) Current commit message is useless. Please use it to describe
what is
the point of the patch.
2) $ git show -U0 | pep8 --diff
./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
lines (2)
./ipatests/test_integration/test_caless.py:825:80: E501 line too long
(80 > 79 characters)
./ipatests/test_integration/test_caless.py:1035:44: E225 missing
whitespace around operator
3) Isn't there a way to do this with pytest's fixtures?
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname,
'--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
There is a standard pytest method called 'method_teardown', that is
indent to be executed after each test method, but with our setup it
does
not work.
4) Is it necessary to create the $TEST_DIR in the test? Isn't it
created
by the framework?
+host.transport.mkdir_recursive(host.config.test_dir)
Removed.
5) I don't think the comment match the code.
+# Remove CA cert in /etc/pki/nssdb, in case of failed
(un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
Not actual anymore
6) No! Create list with one element, iterate that list and append
every
item to the other list. Maybe there's better way (Hint: append).
I've seen this on multiple places.
if unattended:
args.extend(['-U'])
Agreed
7) Why don't you (extend and) use
ipatests.test_integaration.tasks.(un)install_{master,replica}?
This could be done pretty much all over the code.
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi Oleg!
0013 - It looks like there are two unrelated changes, addition of CRL
distribution extension and creating certificate signed by no longer
existing CA. Please create separate patch for each of the changes, and
describe the change and reason for it in commit messages.
0014 - Could you please split the patch to "numerous" commit each fixing
one error? Please also describe each fix so everyone has at least vague
idea about the patch without reading its code. Also why do you introduce
global variable config, I don't see its used anywhere.
0039 - It looks like multiple different changes and commit message says
nothing again. Please split and describe what did you change and why.
0041 - Looks like weird workaround to me. It would be better to
investigate the root cause and fix it. Or at least describe the cause in
commit message and code comment if it can't be fixed. Also "-h is
deprecated in favor of -H" says man 1 ldapmodify.
On 05/09/16 14:32, Oleg Fayans wrote:
Hi guys,
Finally the ca-less tests are stable. Here in the attachment is the full
set of necessary patches.
On 08/09/2016 10:57 AM, Oleg Fayans wrote:
Hi all,
Bump for the review of the 0013 patch. The script it addresses can be
reused in some WebUI tests - one more reason to have it reviewed/merged
The rest patches should be re-tested, since they were prepared a good
while ago
On 05/10/2016 05:08 PM, Oleg Fayans wrote:
Hi David,
After quite a while and some more struggles here comes the updated
version of the patch together with other patches fixing things in
ipatests/test_integration/tasks.py
Server and replica installation was refactored in a way to utilize the
code from tasks.py as much as it is possible
The full set of necessary patches is attached
On 04/20/2016 10:35 AM, David Kupka wrote:
On 19/04/16 11:13, Oleg Fayans wrote:
OK, that one, though passing lint, did not actually work. I gave up my
attempts to define method decorators inside the class. Now it passes
lint AND works:)
Hi Oleg!
1) Current commit message is useless. Please use it to describe what is
the point of the patch.
2) $ git show -U0 | pep8 --diff
./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
lines (2)
./ipatests/test_integration/test_caless.py:825:80: E501 line too long
(80 > 79 characters)
./ipatests/test_integration/test_caless.py:1035:44: E225 missing
whitespace around operator
3) Isn't there a way to do this with pytest's fixtures?
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname, '--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
There is a standard pytest method called 'method_teardown', that is
indent to be executed after each test method, but with our setup it does
not work.
4) Is it necessary to create the $TEST_DIR in the test? Isn't it
created
by the framework?
+host.transport.mkdir_recursive(host.config.test_dir)
Removed.
5) I don't think the comment match the code.
+# Remove CA cert in /etc/pki/nssdb, in case of failed
(un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
Not actual anymore
6) No! Create list with one element, iterate that list and append every
item to the other list. Maybe there's better way (Hint: append).
I've seen this on multiple places.
if unattended:
args.extend(['-U'])
Agreed
7) Why don't you (extend and) use
ipatests.test_integaration.tasks.(un)install_{master,replica}?
This could be done pretty much all over the code.
host.run_command(['ipa-server-install', '--uninstall',
'-U'])
8) Use ipaplatform.paths for certutil and other binaries. If the binary
is not there feel free to add it.
I've seen this on multiple places.
+host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
+ '-n', 'External CA cert'],
+ raiseonerr=False)
+# A workaround
forhttps://fedorahosted.org/freeipa/ticket/4639
+
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi guys,
Finally the ca-less tests are stable. Here in the attachment is the full
set of necessary patches.
On 08/09/2016 10:57 AM, Oleg Fayans wrote:
Hi all,
Bump for the review of the 0013 patch. The script it addresses can be
reused in some WebUI tests - one more reason to have it reviewed/merged
The rest patches should be re-tested, since they were prepared a good
while ago
On 05/10/2016 05:08 PM, Oleg Fayans wrote:
Hi David,
After quite a while and some more struggles here comes the updated
version of the patch together with other patches fixing things in
ipatests/test_integration/tasks.py
Server and replica installation was refactored in a way to utilize the
code from tasks.py as much as it is possible
The full set of necessary patches is attached
On 04/20/2016 10:35 AM, David Kupka wrote:
On 19/04/16 11:13, Oleg Fayans wrote:
OK, that one, though passing lint, did not actually work. I gave up my
attempts to define method decorators inside the class. Now it passes
lint AND works:)
Hi Oleg!
1) Current commit message is useless. Please use it to describe what is
the point of the patch.
2) $ git show -U0 | pep8 --diff
./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
lines (2)
./ipatests/test_integration/test_caless.py:825:80: E501 line too long
(80 > 79 characters)
./ipatests/test_integration/test_caless.py:1035:44: E225 missing
whitespace around operator
3) Isn't there a way to do this with pytest's fixtures?
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname, '--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
There is a standard pytest method called 'method_teardown', that is
indent to be executed after each test method, but with our setup it does
not work.
4) Is it necessary to create the $TEST_DIR in the test? Isn't it created
by the framework?
+host.transport.mkdir_recursive(host.config.test_dir)
Removed.
5) I don't think the comment match the code.
+# Remove CA cert in /etc/pki/nssdb, in case of failed
(un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
Not actual anymore
6) No! Create list with one element, iterate that list and append every
item to the other list. Maybe there's better way (Hint: append).
I've seen this on multiple places.
if unattended:
args.extend(['-U'])
Agreed
7) Why don't you (extend and) use
ipatests.test_integaration.tasks.(un)install_{master,replica}?
This could be done pretty much all over the code.
host.run_command(['ipa-server-install', '--uninstall',
'-U'])
8) Use ipaplatform.paths for certutil and other binaries. If the binary
is not there feel free to add it.
I've seen this on multiple places.
+host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
+ '-n', 'External CA cert'],
+ raiseonerr=False)
+# A workaround forhttps://fedorahosted.org/freeipa/ticket/4639
+result = host.run_command(['certutil', '-L', '-d',
+ paths.HTTPD_ALIAS_DIR])
+for rawcert in result.stdout_text.split('\n')[4: -1]:
+cert = rawcert.split('')[0]
+host.run_command(['certutil', '-D', '-d',
paths.HTTPD_ALIAS_DIR,
+ '-n', cert])
Done
9) certmonger is system service. You can check if is is .enabled() and
.running(). And IIUC the comment is negation of what the code does.
# Verify certmonger was not started
result = host.run_command(['getcert', 'list'],
raiseonerr=False)
-assert result > 0
-assert ('Please verify that the certmonger service has
been '
-'started.' in result.stdout_text),
result.stdout_text
+assert result.returncode == 0
10) What is the point of calling uninstall_server() when it will be
called in the finally block of server_install_teardown anyway?
+@server_inst
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi all,
Bump for the review of the 0013 patch. The script it addresses can be
reused in some WebUI tests - one more reason to have it reviewed/merged
The rest patches should be re-tested, since they were prepared a good
while ago
On 05/10/2016 05:08 PM, Oleg Fayans wrote:
Hi David,
After quite a while and some more struggles here comes the updated
version of the patch together with other patches fixing things in
ipatests/test_integration/tasks.py
Server and replica installation was refactored in a way to utilize the
code from tasks.py as much as it is possible
The full set of necessary patches is attached
On 04/20/2016 10:35 AM, David Kupka wrote:
On 19/04/16 11:13, Oleg Fayans wrote:
OK, that one, though passing lint, did not actually work. I gave up my
attempts to define method decorators inside the class. Now it passes
lint AND works:)
Hi Oleg!
1) Current commit message is useless. Please use it to describe what is
the point of the patch.
2) $ git show -U0 | pep8 --diff
./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
lines (2)
./ipatests/test_integration/test_caless.py:825:80: E501 line too long
(80 > 79 characters)
./ipatests/test_integration/test_caless.py:1035:44: E225 missing
whitespace around operator
3) Isn't there a way to do this with pytest's fixtures?
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname, '--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
There is a standard pytest method called 'method_teardown', that is
indent to be executed after each test method, but with our setup it does
not work.
4) Is it necessary to create the $TEST_DIR in the test? Isn't it created
by the framework?
+host.transport.mkdir_recursive(host.config.test_dir)
Removed.
5) I don't think the comment match the code.
+# Remove CA cert in /etc/pki/nssdb, in case of failed
(un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
Not actual anymore
6) No! Create list with one element, iterate that list and append every
item to the other list. Maybe there's better way (Hint: append).
I've seen this on multiple places.
if unattended:
args.extend(['-U'])
Agreed
7) Why don't you (extend and) use
ipatests.test_integaration.tasks.(un)install_{master,replica}?
This could be done pretty much all over the code.
host.run_command(['ipa-server-install', '--uninstall', '-U'])
8) Use ipaplatform.paths for certutil and other binaries. If the binary
is not there feel free to add it.
I've seen this on multiple places.
+host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
+ '-n', 'External CA cert'],
+ raiseonerr=False)
+# A workaround forhttps://fedorahosted.org/freeipa/ticket/4639
+result = host.run_command(['certutil', '-L', '-d',
+ paths.HTTPD_ALIAS_DIR])
+for rawcert in result.stdout_text.split('\n')[4: -1]:
+cert = rawcert.split('')[0]
+host.run_command(['certutil', '-D', '-d',
paths.HTTPD_ALIAS_DIR,
+ '-n', cert])
Done
9) certmonger is system service. You can check if is is .enabled() and
.running(). And IIUC the comment is negation of what the code does.
# Verify certmonger was not started
result = host.run_command(['getcert', 'list'],
raiseonerr=False)
-assert result > 0
-assert ('Please verify that the certmonger service has
been '
-'started.' in result.stdout_text),
result.stdout_text
+assert result.returncode == 0
10) What is the point of calling uninstall_server() when it will be
called in the finally block of server_install_teardown anyway?
+@server_install_teardown
def test_revoked_http(self):
"IPA server install with revoked HTTP certificate"
if result.returncode == 0:
+
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi David,
After quite a while and some more struggles here comes the updated
version of the patch together with other patches fixing things in
ipatests/test_integration/tasks.py
Server and replica installation was refactored in a way to utilize the
code from tasks.py as much as it is possible
The full set of necessary patches is attached
On 04/20/2016 10:35 AM, David Kupka wrote:
> On 19/04/16 11:13, Oleg Fayans wrote:
>> OK, that one, though passing lint, did not actually work. I gave up my
>> attempts to define method decorators inside the class. Now it passes
>> lint AND works:)
>>
>
> Hi Oleg!
>
> 1) Current commit message is useless. Please use it to describe what is
> the point of the patch.
>
> 2) $ git show -U0 | pep8 --diff
> ./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
> lines, found 1
> ./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
> lines, found 1
> ./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
> lines (2)
> ./ipatests/test_integration/test_caless.py:825:80: E501 line too long
> (80 > 79 characters)
> ./ipatests/test_integration/test_caless.py:1035:44: E225 missing
> whitespace around operator
>
>
> 3) Isn't there a way to do this with pytest's fixtures?
>
>> +def server_install_teardown(func):
>> +def wrapped(*args):
>> +try:
>> +func(*args)
>> +finally:
>> +args[0].uninstall_server()
>> +return wrapped
>> +
>> +def replica_install_teardown(func):
>> +def wrapped(*args):
>> +try:
>> +func(*args)
>> +finally:
>> +# Uninstall replica
>> +replica = args[0].replicas[0]
>> +tasks.kinit_admin(args[0].master)
>> +args[0].uninstall_server(replica)
>> +args[0].master.run_command(['ipa-replica-manage', 'del',
>> +replica.hostname, '--force'],
>> + raiseonerr=False)
>> +args[0].master.run_command(['ipa', 'host-del',
>> +replica.hostname],
>> + raiseonerr=False)
>> +return wrapped
>> +
There is a standard pytest method called 'method_teardown', that is
indent to be executed after each test method, but with our setup it does
not work.
>
> 4) Is it necessary to create the $TEST_DIR in the test? Isn't it created
> by the framework?
>
>> +host.transport.mkdir_recursive(host.config.test_dir)
>
Removed.
>
> 5) I don't think the comment match the code.
>
>>
>> +# Remove CA cert in /etc/pki/nssdb, in case of failed
>> (un)install
>> +for host in cls.get_all_hosts():
>> +cls.uninstall_server(host)
>> +
>> super(CALessBase, cls).uninstall(mh)
>
Not actual anymore
>
> 6) No! Create list with one element, iterate that list and append every
> item to the other list. Maybe there's better way (Hint: append).
> I've seen this on multiple places.
>
>> if unattended:
>> args.extend(['-U'])
Agreed
>
> 7) Why don't you (extend and) use
> ipatests.test_integaration.tasks.(un)install_{master,replica}?
> This could be done pretty much all over the code.
>
>> host.run_command(['ipa-server-install', '--uninstall', '-U'])
>
> 8) Use ipaplatform.paths for certutil and other binaries. If the binary
> is not there feel free to add it.
> I've seen this on multiple places.
>
>> +host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
>> + '-n', 'External CA cert'],
>> + raiseonerr=False)
>> +# A workaround forhttps://fedorahosted.org/freeipa/ticket/4639
>> +result = host.run_command(['certutil', '-L', '-d',
>> + paths.HTTPD_ALIAS_DIR])
>> +for rawcert in result.stdout_text.split('\n')[4: -1]:
>> +cert = rawcert.split('')[0]
>> +host.run_command(['certutil', '-D', '-d',
>> paths.HTTPD_ALIAS_DIR,
>> + '-n', cert])
>>
Done
>
> 9) certmonger is system service. You can check if is is .enabled() and
> .running(). And IIUC the comment is negation of what the code does.
>
>>
>> # Verify certmonger was not started
>> result = host.run_command(['getcert', 'list'],
>> raiseonerr=False)
>> -assert result > 0
>> -assert ('Please verify that the certmonger service has
>> been '
>> -'started.' in result.stdout_text),
>> result.stdout_text
>> +assert result.returncode == 0
>
> 10) What is the point of calling uninstall_server() when it will be
> called in the finally block of server_install_teardown anyway?
>
>> +@server_install_teardown
>> def test_revoked_http(self):
>> "IPA server install with revoked HTTP certificate"
>>
>> if result.returncode == 0:
>> +
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi David,
On 04/20/2016 11:55 AM, David Kupka wrote:
> On 20/04/16 10:35, David Kupka wrote:
>> On 19/04/16 11:13, Oleg Fayans wrote:
>>> OK, that one, though passing lint, did not actually work. I gave up my
>>> attempts to define method decorators inside the class. Now it passes
>>> lint AND works:)
>>>
>>
>> Hi Oleg!
>>
>> 1) Current commit message is useless. Please use it to describe what is
>> the point of the patch.
>>
>> 2) $ git show -U0 | pep8 --diff
>> ./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
>> lines, found 1
>> ./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
>> lines, found 1
>> ./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
>> lines (2)
>> ./ipatests/test_integration/test_caless.py:825:80: E501 line too long
>> (80 > 79 characters)
>> ./ipatests/test_integration/test_caless.py:1035:44: E225 missing
>> whitespace around operator
>>
>>
>> 3) Isn't there a way to do this with pytest's fixtures?
>>
>>> +def server_install_teardown(func):
>>> +def wrapped(*args):
>>> +try:
>>> +func(*args)
>>> +finally:
>>> +args[0].uninstall_server()
>>> +return wrapped
>>> +
>>> +def replica_install_teardown(func):
>>> +def wrapped(*args):
>>> +try:
>>> +func(*args)
>>> +finally:
>>> +# Uninstall replica
>>> +replica = args[0].replicas[0]
>>> +tasks.kinit_admin(args[0].master)
>>> +args[0].uninstall_server(replica)
>>> +args[0].master.run_command(['ipa-replica-manage', 'del',
>>> +replica.hostname, '--force'],
>>> + raiseonerr=False)
>>> +args[0].master.run_command(['ipa', 'host-del',
>>> +replica.hostname],
>>> + raiseonerr=False)
>>> +return wrapped
>>> +
>>
>> 4) Is it necessary to create the $TEST_DIR in the test? Isn't it created
>> by the framework?
>>
>>> +host.transport.mkdir_recursive(host.config.test_dir)
>>
>>
>> 5) I don't think the comment match the code.
>>
>>>
>>> +# Remove CA cert in /etc/pki/nssdb, in case of failed
>>> (un)install
>>> +for host in cls.get_all_hosts():
>>> +cls.uninstall_server(host)
>>> +
>>> super(CALessBase, cls).uninstall(mh)
>>
>>
>> 6) No! Create list with one element, iterate that list and append every
>> item to the other list. Maybe there's better way (Hint: append).
>> I've seen this on multiple places.
>>
>>> if unattended:
>>> args.extend(['-U'])
>>
>> 7) Why don't you (extend and) use
>> ipatests.test_integaration.tasks.(un)install_{master,replica}?
>> This could be done pretty much all over the code.
>>
>>> host.run_command(['ipa-server-install', '--uninstall', '-U'])
>>
>> 8) Use ipaplatform.paths for certutil and other binaries. If the binary
>> is not there feel free to add it.
>> I've seen this on multiple places.
>>
>>> +host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
>>> + '-n', 'External CA cert'],
>>> + raiseonerr=False)
>>> +# A workaround forhttps://fedorahosted.org/freeipa/ticket/4639
>>> +result = host.run_command(['certutil', '-L', '-d',
>>> + paths.HTTPD_ALIAS_DIR])
>>> +for rawcert in result.stdout_text.split('\n')[4: -1]:
>>> +cert = rawcert.split('')[0]
>>> +host.run_command(['certutil', '-D', '-d',
>>> paths.HTTPD_ALIAS_DIR,
>>> + '-n', cert])
>>>
>>
>> 9) certmonger is system service. You can check if is is .enabled() and
>> .running(). And IIUC the comment is negation of what the code does.
>>
>>>
>>> # Verify certmonger was not started
>>> result = host.run_command(['getcert', 'list'],
>>> raiseonerr=False)
>>> -assert result > 0
>>> -assert ('Please verify that the certmonger service has
>>> been '
>>> -'started.' in result.stdout_text),
>>> result.stdout_text
>>> +assert result.returncode == 0
>>
>> 10) What is the point of calling uninstall_server() when it will be
>> called in the finally block of server_install_teardown anyway?
>>
>>> +@server_install_teardown
>>> def test_revoked_http(self):
>>> "IPA server install with revoked HTTP certificate"
>>>
>>> if result.returncode == 0:
>>> +self.uninstall_server()
>>> raise nose.SkipTest(
>>> "Known CA-less installation defect, see "
>>> +"https://fedorahosted.org/freeipa/ticket/4270";)
>>>
>>> assert result.returncode > 0
>>>
>>
>> Nitpick) Do not mix fixing typos/grammar/spelling/style with functional
>> changes.
>>
>>> -def test_incorect_http_pin(self):
>>> +
Re: [Freeipa-devel] [PATCH] ca-less tests updated
On 20/04/16 10:35, David Kupka wrote:
On 19/04/16 11:13, Oleg Fayans wrote:
OK, that one, though passing lint, did not actually work. I gave up my
attempts to define method decorators inside the class. Now it passes
lint AND works:)
Hi Oleg!
1) Current commit message is useless. Please use it to describe what is
the point of the patch.
2) $ git show -U0 | pep8 --diff
./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
lines (2)
./ipatests/test_integration/test_caless.py:825:80: E501 line too long
(80 > 79 characters)
./ipatests/test_integration/test_caless.py:1035:44: E225 missing
whitespace around operator
3) Isn't there a way to do this with pytest's fixtures?
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname, '--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
4) Is it necessary to create the $TEST_DIR in the test? Isn't it created
by the framework?
+host.transport.mkdir_recursive(host.config.test_dir)
5) I don't think the comment match the code.
+# Remove CA cert in /etc/pki/nssdb, in case of failed
(un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
6) No! Create list with one element, iterate that list and append every
item to the other list. Maybe there's better way (Hint: append).
I've seen this on multiple places.
if unattended:
args.extend(['-U'])
7) Why don't you (extend and) use
ipatests.test_integaration.tasks.(un)install_{master,replica}?
This could be done pretty much all over the code.
host.run_command(['ipa-server-install', '--uninstall', '-U'])
8) Use ipaplatform.paths for certutil and other binaries. If the binary
is not there feel free to add it.
I've seen this on multiple places.
+host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
+ '-n', 'External CA cert'],
+ raiseonerr=False)
+# A workaround forhttps://fedorahosted.org/freeipa/ticket/4639
+result = host.run_command(['certutil', '-L', '-d',
+ paths.HTTPD_ALIAS_DIR])
+for rawcert in result.stdout_text.split('\n')[4: -1]:
+cert = rawcert.split('')[0]
+host.run_command(['certutil', '-D', '-d',
paths.HTTPD_ALIAS_DIR,
+ '-n', cert])
9) certmonger is system service. You can check if is is .enabled() and
.running(). And IIUC the comment is negation of what the code does.
# Verify certmonger was not started
result = host.run_command(['getcert', 'list'],
raiseonerr=False)
-assert result > 0
-assert ('Please verify that the certmonger service has
been '
-'started.' in result.stdout_text),
result.stdout_text
+assert result.returncode == 0
10) What is the point of calling uninstall_server() when it will be
called in the finally block of server_install_teardown anyway?
+@server_install_teardown
def test_revoked_http(self):
"IPA server install with revoked HTTP certificate"
if result.returncode == 0:
+self.uninstall_server()
raise nose.SkipTest(
"Known CA-less installation defect, see "
+"https://fedorahosted.org/freeipa/ticket/4270";)
assert result.returncode > 0
Nitpick) Do not mix fixing typos/grammar/spelling/style with functional
changes.
-def test_incorect_http_pin(self):
+@pytest.mark.xfail(reason='freeipa ticket 5378')
+def test_incorrect_http_pin(self):
"Install new HTTP certificate with incorrect PKCS#12 password"
I forget to mention the most important problem:
0) It does not work.
= test session starts
==
platform linux2 -- Python 2.7.11 -- py-1.4.30 -- pytest-2.7.3
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: multihost, sourceo
Re: [Freeipa-devel] [PATCH] ca-less tests updated
On 19/04/16 11:13, Oleg Fayans wrote:
OK, that one, though passing lint, did not actually work. I gave up my
attempts to define method decorators inside the class. Now it passes
lint AND works:)
Hi Oleg!
1) Current commit message is useless. Please use it to describe what is
the point of the patch.
2) $ git show -U0 | pep8 --diff
./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
lines (2)
./ipatests/test_integration/test_caless.py:825:80: E501 line too long
(80 > 79 characters)
./ipatests/test_integration/test_caless.py:1035:44: E225 missing
whitespace around operator
3) Isn't there a way to do this with pytest's fixtures?
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname, '--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
4) Is it necessary to create the $TEST_DIR in the test? Isn't it created
by the framework?
+host.transport.mkdir_recursive(host.config.test_dir)
5) I don't think the comment match the code.
+# Remove CA cert in /etc/pki/nssdb, in case of failed (un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
6) No! Create list with one element, iterate that list and append every
item to the other list. Maybe there's better way (Hint: append).
I've seen this on multiple places.
if unattended:
args.extend(['-U'])
7) Why don't you (extend and) use
ipatests.test_integaration.tasks.(un)install_{master,replica}?
This could be done pretty much all over the code.
host.run_command(['ipa-server-install', '--uninstall', '-U'])
8) Use ipaplatform.paths for certutil and other binaries. If the binary
is not there feel free to add it.
I've seen this on multiple places.
+host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
+ '-n', 'External CA cert'],
+ raiseonerr=False)
+# A workaround forhttps://fedorahosted.org/freeipa/ticket/4639
+result = host.run_command(['certutil', '-L', '-d',
+ paths.HTTPD_ALIAS_DIR])
+for rawcert in result.stdout_text.split('\n')[4: -1]:
+cert = rawcert.split('')[0]
+host.run_command(['certutil', '-D', '-d', paths.HTTPD_ALIAS_DIR,
+ '-n', cert])
9) certmonger is system service. You can check if is is .enabled() and
.running(). And IIUC the comment is negation of what the code does.
# Verify certmonger was not started
result = host.run_command(['getcert', 'list'], raiseonerr=False)
-assert result > 0
-assert ('Please verify that the certmonger service has been '
-'started.' in result.stdout_text), result.stdout_text
+assert result.returncode == 0
10) What is the point of calling uninstall_server() when it will be
called in the finally block of server_install_teardown anyway?
+@server_install_teardown
def test_revoked_http(self):
"IPA server install with revoked HTTP certificate"
if result.returncode == 0:
+self.uninstall_server()
raise nose.SkipTest(
"Known CA-less installation defect, see "
+"https://fedorahosted.org/freeipa/ticket/4270";)
assert result.returncode > 0
Nitpick) Do not mix fixing typos/grammar/spelling/style with functional
changes.
-def test_incorect_http_pin(self):
+@pytest.mark.xfail(reason='freeipa ticket 5378')
+def test_incorrect_http_pin(self):
"Install new HTTP certificate with incorrect PKCS#12 password"
--
David Kupka
--
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
Re: [Freeipa-devel] [PATCH] ca-less tests updated
OK, that one, though passing lint, did not actually work. I gave up my
attempts to define method decorators inside the class. Now it passes
lint AND works :)
On 04/13/2016 12:24 PM, Oleg Fayans wrote:
> Please. disregard my previous patch-0014: it doesn't pass pylint. The
> newer one does.
>
> On 04/08/2016 04:46 PM, Oleg Fayans wrote:
>> Hi all,
>>
>> It's been a while, and now the patch seems to be stable. It does hit one
>> known issue with replica installation occationally [1], but other than
>> that works fine on both domain levels.
>>
>> [1] https://fedorahosted.org/freeipa/ticket/5758
>>
>>
>>
>
>
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 911e3a6ff38c4f34fd43b1c1b1bd8d9053acfc54 Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Tue, 19 Apr 2016 11:04:31 +0200
Subject: [PATCH] Actualized ca-less tests
https://fedorahosted.org/freeipa/ticket/4589
---
ipatests/test_integration/test_caless.py | 453 +--
1 file changed, 248 insertions(+), 205 deletions(-)
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index fdc4fc8efe73631e9ab03f3b9019444f7d7e09ec..dd1d8194891f177a2055ee1cf66b7f82c5d6c245 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -32,20 +32,19 @@ from ipaplatform.paths import paths
from ipapython.dn import DN
from ipatests.test_integration.base import IntegrationTest
from ipatests.test_integration import tasks
+from env_config import get_global_config
+from ipalib.constants import DOMAIN_LEVEL_0, DOMAIN_LEVEL_1
_DEFAULT = object()
+config = get_global_config()
def get_install_stdin(cert_passwords=()):
lines = [
-'yes', # Existing BIND configuration detected, overwrite? [no]
'', # Server host name (has default)
-'', # Confirm domain name (has default)
]
lines.extend(cert_passwords) # Enter foo.p12 unlock password
lines += [
-'', # Do you want to configure the reverse zone? [yes]
-'', # Please specify the reverse zone name [47.34.10.in-addr.arpa.]
'yes', # Continue with these values?
]
return '\n'.join(lines + [''])
@@ -64,6 +63,31 @@ def assert_error(result, stderr_text, returncode=None):
else:
assert result.returncode > 0
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname, '--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
class CALessBase(IntegrationTest):
@classmethod
@@ -86,22 +110,23 @@ class CALessBase(IntegrationTest):
client_hostname = cls.clients[0].hostname
else:
client_hostname = 'unused-client.test'
-env = {
+cls.env = {
'domain': cls.master.domain.name,
'server1': cls.master.hostname,
'server2': replica_hostname,
'client': client_hostname,
'dbdir': 'nssdb',
-'dbpassword': cls.cert_password,
'crl_path': cls.crl_path,
+'dirman_password': cls.master.config.dirman_password,
}
-ipautil.run(['bash', '-ex', scriptfile], cwd=cls.cert_dir, env=env)
+ipautil.run(['bash', '-ex', scriptfile], cwd=cls.cert_dir, env=cls.env)
for host in cls.get_all_hosts():
tasks.apply_common_fixes(host)
# Copy CRLs over
base = os.path.join(cls.cert_dir, 'nssdb')
+host.transport.mkdir_recursive(host.config.test_dir)
host.transport.mkdir_recursive(cls.crl_path)
for source in glob.glob(os.path.join(base, '*.crl')):
dest = os.path.join(cls.crl_path, os.path.basename(source))
@@ -112,6 +137,10 @@ class CALessBase(IntegrationTest):
# Remove the NSS database
shutil.rmtree(cls.cert_dir)
+# Remove CA cert in /etc/pki/nssdb, in case of failed (un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
@classmethod
@@ -140,6 +169,11 @@ class CALessBase(IntegrationTest):
for filename in set(files_to_copy):
cls.copy_cert(host, filename)
+# Remove existing ca certs from defau
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Please. disregard my previous patch-0014: it doesn't pass pylint. The
newer one does.
On 04/08/2016 04:46 PM, Oleg Fayans wrote:
> Hi all,
>
> It's been a while, and now the patch seems to be stable. It does hit one
> known issue with replica installation occationally [1], but other than
> that works fine on both domain levels.
>
> [1] https://fedorahosted.org/freeipa/ticket/5758
>
>
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 1d750eb5097c8cffc7b6ceb7c3bfd719ca7078cb Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Wed, 13 Apr 2016 11:58:33 +0200
Subject: [PATCH] Actualized ca-less tests
https://fedorahosted.org/freeipa/ticket/4589
---
ipatests/test_integration/test_caless.py | 452 +--
1 file changed, 248 insertions(+), 204 deletions(-)
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index fdc4fc8efe73631e9ab03f3b9019444f7d7e09ec..6b05112af120cbba35b0fea593e68291665bfd52 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -32,20 +32,19 @@ from ipaplatform.paths import paths
from ipapython.dn import DN
from ipatests.test_integration.base import IntegrationTest
from ipatests.test_integration import tasks
+from env_config import get_global_config
+from ipalib.constants import DOMAIN_LEVEL_0, DOMAIN_LEVEL_1
_DEFAULT = object()
+config = get_global_config()
def get_install_stdin(cert_passwords=()):
lines = [
-'yes', # Existing BIND configuration detected, overwrite? [no]
'', # Server host name (has default)
-'', # Confirm domain name (has default)
]
lines.extend(cert_passwords) # Enter foo.p12 unlock password
lines += [
-'', # Do you want to configure the reverse zone? [yes]
-'', # Please specify the reverse zone name [47.34.10.in-addr.arpa.]
'yes', # Continue with these values?
]
return '\n'.join(lines + [''])
@@ -86,22 +85,23 @@ class CALessBase(IntegrationTest):
client_hostname = cls.clients[0].hostname
else:
client_hostname = 'unused-client.test'
-env = {
+cls.env = {
'domain': cls.master.domain.name,
'server1': cls.master.hostname,
'server2': replica_hostname,
'client': client_hostname,
'dbdir': 'nssdb',
-'dbpassword': cls.cert_password,
'crl_path': cls.crl_path,
+'dirman_password': cls.master.config.dirman_password,
}
-ipautil.run(['bash', '-ex', scriptfile], cwd=cls.cert_dir, env=env)
+ipautil.run(['bash', '-ex', scriptfile], cwd=cls.cert_dir, env=cls.env)
for host in cls.get_all_hosts():
tasks.apply_common_fixes(host)
# Copy CRLs over
base = os.path.join(cls.cert_dir, 'nssdb')
+host.transport.mkdir_recursive(host.config.test_dir)
host.transport.mkdir_recursive(cls.crl_path)
for source in glob.glob(os.path.join(base, '*.crl')):
dest = os.path.join(cls.crl_path, os.path.basename(source))
@@ -112,6 +112,10 @@ class CALessBase(IntegrationTest):
# Remove the NSS database
shutil.rmtree(cls.cert_dir)
+# Remove CA cert in /etc/pki/nssdb, in case of failed (un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
@classmethod
@@ -140,6 +144,11 @@ class CALessBase(IntegrationTest):
for filename in set(files_to_copy):
cls.copy_cert(host, filename)
+# Remove existing ca certs from default database to avoid conflicts
+args = ["certutil", "-D", "-d", "/etc/httpd/alias", "-n"]
+host.run_command(args + ["ca1"], raiseonerr=False)
+host.run_command(args + ["ca1/server"], raiseonerr=False)
+
host.collect_log(paths.IPASERVER_INSTALL_LOG)
host.collect_log(paths.IPACLIENT_INSTALL_LOG)
inst = host.domain.realm.replace('.', '-')
@@ -152,11 +161,14 @@ class CALessBase(IntegrationTest):
'--dirsrv-cert-file', dirsrv_pkcs12,
'--ca-cert-file', root_ca_file,
'--ip-address', host.ip,
-'-r', host.domain.name,
+'-n', host.domain.name,
+'-r', host.domain.realm,
'-p', host.config.dirman_password,
'-a', host.config.admin_password,
'--setup-dns',
'--forwarder', host.config.dns_forwarder,
+'--auto-reverse',
+'--domain-level', str(config.domain_level)
]
if http_pin is not None:
@@ -165,7 +177,6 @@ class CALessBase(IntegrationTest):
args.extend(['--dirsrv-pin', dirsrv_pin])
if unattended:
args.extend(['-U'])
-
return host.run_command(args, raiseonerr=False, stdin_text=stdin_text)
@classmethod
@@
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi all,
It's been a while, and now the patch seems to be stable. It does hit one
known issue with replica installation occationally [1], but other than
that works fine on both domain levels.
[1] https://fedorahosted.org/freeipa/ticket/5758
From a773c297f37340f36cf257a2b5b75eb8199bd47d Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Tue, 8 Dec 2015 10:49:18 +0100
Subject: [PATCH] Updated the script creating test certificate chains
https://fedorahosted.org/freeipa/ticket/4589
---
.../test_integration/scripts/caless-create-pki | 29 ++
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/ipatests/test_integration/scripts/caless-create-pki b/ipatests/test_integration/scripts/caless-create-pki
index f428ebae16e05644a875a35faf192f75eb149740..4c37077ffdecfb0c70663c7c4817f102154d3b26 100644
--- a/ipatests/test_integration/scripts/caless-create-pki
+++ b/ipatests/test_integration/scripts/caless-create-pki
@@ -3,7 +3,17 @@
profile_ca=(-t CT,C,C -v 120)
profile_server=(-t ,, -v 12)
-crl_path=${crl_path-$(readlink -f $dbdir)}
+# crl_path=${crl_path-$(readlink -f $dbdir)}
+profile_ca_request_options=(-1 -2 -4)
+profile_ca_request_input="\$'0\n1\n5\n6\n9\ny\ny\n\ny\n1\n7\nfile://'\$(readlink -f \$dbdir)/\$ca.crl\$'\n-1\n-1\n-1\nn\nn\n'"
+profile_ca_create_options=(-v 120)
+profile_ca_add_options=(-t ,,)
+
+profile_server_request_options=(-4)
+profile_server_request_input="\$'1\n7\nfile://'\$(readlink -f \$dbdir)/\$ca.crl\$'\n-1\n-1\n-1\nn\nn\n'"
+profile_server_create_options=(-v 12)
+profile_server_add_options=(-t ,,)
+
serial_number=0
@@ -18,7 +28,11 @@ gen_cert() {
ca="$nick"
fi
+echo $profile
eval "options=(\"\${profile_$profile[@]}\")"
+eval "request_options=(\"\${profile_${profile}_request_options[@]}\")"
+eval "eval request_input=(\"\${profile_${profile}_request_input[@]}\")"
+
if [ "$ca" = "$nick" ]; then
options=("${options[@]}" -x -m 1)
else
@@ -38,16 +52,7 @@ gen_cert() {
csr="$(mktemp)"
crt="$(mktemp)"
-certutil -R -d "$dbdir" -s "$subject" -f "$pwfile" -z "$noise" -o "$csr" -4 >/dev/null
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Hi David,
On 12/16/2015 03:35 PM, David Kupka wrote:
> On 06/11/15 14:04, Oleg Fayans wrote:
>> Hi Jan,
>>
>> On 11/06/2015 09:01 AM, Jan Cholasta wrote:
>>> Actually it might be better to keep them, but fix them to expect
>>> ipa-server-certinstall to success.
>>
>> Done. Updated patch attached.
>> Also in the patch 0013 I removed a trailing whitespace which caused lint
>> to complain
>>
>> Now with domain level 0 the test output looks like this:
>>
>> [11:40:51]ofayans@vm-076:~]$ ipa-run-tests
>> test_integration/test_caless.py
>>
>>
>> test session starts
>> =
>>
>>
>> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
>> plugins: multihost, sourceorder
>> collected 88 items
>>
>> test_integration/test_caless.py
>> ..xx..ss...xxssxx..ss...
>>
>>
>>
>> = 76
>> passed, 6 skipped, 6 xfailed in 7871.10 seconds
>> =
>>
>>
>>>
>>> On 6.11.2015 08:47, Jan Cholasta wrote:
Hi Oleg,
I think you can just remove
TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
imported correctly in this case and I didn't see anything break.
Honza
On 5.11.2015 20:20, Oleg Fayans wrote:
> Patch 0014 updated and passes lint
>
> On 11/05/2015 03:41 PM, Oleg Fayans wrote:
>> Wait a bit, the patch has problems with pylint: it does not build :)
>> The updated version (without the setupmaster nonsense) is being
>> tested
>> now.
>>
>> On 11/05/2015 08:45 AM, Oleg Fayans wrote:
>>> Hi Jan,
>>>
>>> Could you take a look at these, whenever you are free?
>>>
>>> On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to
the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to
ipa-replica-prepare
(unsupported under domain level 1)
>>>
>>
>
>
>
>>>
>>>
>>
>>
>>
>> This body part will be downloaded on demand.
>>
> Hello, thanks for updated patches. I'm really sorry it took so long
> before I got to them.
> There was change in ipapython.ipautil.run that happened after you sent
> the patches. Feel free to squash attached patch that fixes it.
Already noticed this and made the similar fix.
>
> Unfortunately I see a lot of test failing with domain-level 0:
> http://fpaste.org/301657/50275682/
>
> domain-level 1 (domain-level 1: http://fpaste.org/301658/02757191/)
> seems better. There are 2 failing test that you're probably mentioning
> in commit message plus one that I think is bug in code rather than bug
> in tests.
> Do you have any proposal for fixing the two failing tests?
I am working on it right now.
>
> One nitpick: Please use mail for notes like "need further consulting
> ..." rather that commit message. When the patch gets accepted it will
> still need modification before push just because inappropriate commit
> message.
Good to know, thank you!
>
> Thank you!
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
--
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
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
On 06/11/15 14:04, Oleg Fayans wrote:
Hi Jan,
On 11/06/2015 09:01 AM, Jan Cholasta wrote:
Actually it might be better to keep them, but fix them to expect
ipa-server-certinstall to success.
Done. Updated patch attached.
Also in the patch 0013 I removed a trailing whitespace which caused lint
to complain
Now with domain level 0 the test output looks like this:
[11:40:51]ofayans@vm-076:~]$ ipa-run-tests test_integration/test_caless.py
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..ss...xxssxx..ss...
= 76
passed, 6 skipped, 6 xfailed in 7871.10 seconds
=
On 6.11.2015 08:47, Jan Cholasta wrote:
Hi Oleg,
I think you can just remove
TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
imported correctly in this case and I didn't see anything break.
Honza
On 5.11.2015 20:20, Oleg Fayans wrote:
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
Wait a bit, the patch has problems with pylint: it does not build :)
The updated version (without the setupmaster nonsense) is being tested
now.
On 11/05/2015 08:45 AM, Oleg Fayans wrote:
Hi Jan,
Could you take a look at these, whenever you are free?
On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to
the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to
ipa-replica-prepare
(unsupported under domain level 1)
This body part will be downloaded on demand.
Hello, thanks for updated patches. I'm really sorry it took so long
before I got to them.
There was change in ipapython.ipautil.run that happened after you sent
the patches. Feel free to squash attached patch that fixes it.
Unfortunately I see a lot of test failing with domain-level 0:
http://fpaste.org/301657/50275682/
domain-level 1 (domain-level 1: http://fpaste.org/301658/02757191/)
seems better. There are 2 failing test that you're probably mentioning
in commit message plus one that I think is bug in code rather than bug
in tests.
Do you have any proposal for fixing the two failing tests?
One nitpick: Please use mail for notes like "need further consulting
..." rather that commit message. When the patch gets accepted it will
still need modification before push just because inappropriate commit
message.
Thank you!
--
David Kupka
From 2a6e8f02ecd00da2b86d2f3f9847a86caa35e74d Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Wed, 16 Dec 2015 09:12:56 +0100
Subject: [PATCH] Addapt CA less test to new ipapython.ipautil.run
---
ipatests/test_integration/test_caless.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 4b88ee9da1d5a476f13604f9a833e748a093..6cb55a708517062edb1bb950a72d6a66f717432e 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -300,10 +300,10 @@ class CALessBase(IntegrationTest):
@classmethod
def get_pem(cls, nickname):
-pem_cert, _stderr, _returncode = ipautil.run(
+result = ipautil.run(
['certutil', '-L', '-d', 'nssdb', '-n', nickname, '-a'],
-cwd=cls.cert_dir)
-return pem_cert
+cwd=cls.cert_dir, capture_output=True)
+return result.output
def verify_installation(self):
"""Verify CA cert PEM file and LDAP entry created by install
--
2.5.0
--
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
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Anyone to review it guys?
On 11/06/2015 02:04 PM, Oleg Fayans wrote:
> Hi Jan,
>
> On 11/06/2015 09:01 AM, Jan Cholasta wrote:
>> Actually it might be better to keep them, but fix them to expect
>> ipa-server-certinstall to success.
>
> Done. Updated patch attached.
> Also in the patch 0013 I removed a trailing whitespace which caused lint
> to complain
>
> Now with domain level 0 the test output looks like this:
>
> [11:40:51]ofayans@vm-076:~]$ ipa-run-tests test_integration/test_caless.py
>
> test session starts
> =
>
> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
> plugins: multihost, sourceorder
> collected 88 items
>
> test_integration/test_caless.py
> ..xx..ss...xxssxx..ss...
>
>
> = 76
> passed, 6 skipped, 6 xfailed in 7871.10 seconds
> =
>
>
>>
>> On 6.11.2015 08:47, Jan Cholasta wrote:
>>> Hi Oleg,
>>>
>>> I think you can just remove
>>> TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
>>> imported correctly in this case and I didn't see anything break.
>>>
>>> Honza
>>>
>>> On 5.11.2015 20:20, Oleg Fayans wrote:
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
> Wait a bit, the patch has problems with pylint: it does not build :)
> The updated version (without the setupmaster nonsense) is being tested
> now.
>
> On 11/05/2015 08:45 AM, Oleg Fayans wrote:
>> Hi Jan,
>>
>> Could you take a look at these, whenever you are free?
>>
>> On 10/30/2015 02:57 PM, Oleg Fayans wrote:
>>> Hi,
>>>
>>> The following patches contain updates to ca-less integration tests.
>>> It's still a proof of concept: 2 tests still fail seemingly due to
>>> the
>>> change in target system logic (marked as xfail with "ask jcholast
>>> comment")
>>>
>>> The test output looks like this:
>>>
>>> $ ipa-run-tests test_integration/test_caless.py --pdb
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> test session starts
>>> =
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
>>> plugins: multihost, sourceorder
>>> collected 88 items
>>>
>>> test_integration/test_caless.py
>>> ..xx..sssss.ss.xx..ssxx.
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> 53
>>>
>>> passed, 29 skipped, 6 xfailed in 5620.17 seconds
>>> =
>>>
>>>
>>> Numerous skips correspond to the tests related to
>>> ipa-replica-prepare
>>> (unsupported under domain level 1)
>>>
>>>
>>>
>>
>
>>>
>>>
>>
>>
>
>
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
--
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
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Hi guys,
Is there a chance these patches might be reviewed again this week?
On 11/06/2015 02:04 PM, Oleg Fayans wrote:
Hi Jan,
On 11/06/2015 09:01 AM, Jan Cholasta wrote:
Actually it might be better to keep them, but fix them to expect
ipa-server-certinstall to success.
Done. Updated patch attached.
Also in the patch 0013 I removed a trailing whitespace which caused lint
to complain
Now with domain level 0 the test output looks like this:
[11:40:51]ofayans@vm-076:~]$ ipa-run-tests test_integration/test_caless.py
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..ss...xxssxx..ss...
= 76
passed, 6 skipped, 6 xfailed in 7871.10 seconds
=
On 6.11.2015 08:47, Jan Cholasta wrote:
Hi Oleg,
I think you can just remove
TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
imported correctly in this case and I didn't see anything break.
Honza
On 5.11.2015 20:20, Oleg Fayans wrote:
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
Wait a bit, the patch has problems with pylint: it does not build :)
The updated version (without the setupmaster nonsense) is being tested
now.
On 11/05/2015 08:45 AM, Oleg Fayans wrote:
Hi Jan,
Could you take a look at these, whenever you are free?
On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to
the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to
ipa-replica-prepare
(unsupported under domain level 1)
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
--
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
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Hi Jan,
On 11/06/2015 09:01 AM, Jan Cholasta wrote:
Actually it might be better to keep them, but fix them to expect
ipa-server-certinstall to success.
Done. Updated patch attached.
Also in the patch 0013 I removed a trailing whitespace which caused lint
to complain
Now with domain level 0 the test output looks like this:
[11:40:51]ofayans@vm-076:~]$ ipa-run-tests test_integration/test_caless.py
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..ss...xxssxx..ss...
= 76
passed, 6 skipped, 6 xfailed in 7871.10 seconds
=
On 6.11.2015 08:47, Jan Cholasta wrote:
Hi Oleg,
I think you can just remove
TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
imported correctly in this case and I didn't see anything break.
Honza
On 5.11.2015 20:20, Oleg Fayans wrote:
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
Wait a bit, the patch has problems with pylint: it does not build :)
The updated version (without the setupmaster nonsense) is being tested
now.
On 11/05/2015 08:45 AM, Oleg Fayans wrote:
Hi Jan,
Could you take a look at these, whenever you are free?
On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to
the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to ipa-replica-prepare
(unsupported under domain level 1)
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 3142e9ab937b602a687639e7972422001e887211 Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Thu, 5 Nov 2015 16:25:29 +0100
Subject: [PATCH] Updated the script creating test certificate chains
https://fedorahosted.org/freeipa/ticket/4589
---
.../test_integration/scripts/caless-create-pki | 29 ++
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/ipatests/test_integration/scripts/caless-create-pki b/ipatests/test_integration/scripts/caless-create-pki
index f428ebae16e05644a875a35faf192f75eb149740..4c37077ffdecfb0c70663c7c4817f102154d3b26 100644
--- a/ipatests/test_integration/scripts/caless-create-pki
+++ b/ipatests/test_integration/scripts/caless-create-pki
@@ -3,7 +3,17 @@
profile_ca=(-t CT,C,C -v 120)
profile_server=(-t ,, -v 12)
-crl_path=${crl_path-$(readlink -f $dbdir)}
+# crl_path=${crl_path-$(readlink -f $dbdir)}
+profile_ca_request_options=(-1 -2 -4)
+profile_ca_request_input="\$'0\n1\n5\n6\n9\ny\ny\n\ny\n1\n7\nfile://'\$(readlink -f \$dbdir)/\$ca.crl\$'\n-1\n-1\n-1\nn\nn\n'"
+profile_ca_create_options=(-v 120)
+profile_ca_add_options=(-t ,,)
+
+profile_server_request_options=(-4)
+profile_server_request_input="\$'1\n7\nfile://'\$(readlink -f \$dbdir)/\$ca.crl\$'\n-1\n-1\n-1\nn\nn\n'"
+profile_server_create_options=(-v 12)
+profile_server_add_options=(-t ,,)
+
serial_number=0
@@ -18,7 +28,11 @@ gen_cert() {
ca="$nick"
fi
+echo $profile
eval "options=(\"\${profile_$profile[@]}\")"
+eval "request_options=(\"\${profile_${profile}_request_options[@]}\")"
+eval "eval request_input=(\"\${profile_${profile}_request_input[@]}\")"
+
if [ "$ca" = "$nick" ]; then
options=("${options[@]}" -x -m 1)
else
@@ -38,16 +52,7 @@ gen_cert() {
csr="$(mktemp)"
crt="$(mktemp)"
-certutil -R -d "$dbdir" -s "$subject" -f "$pwfile" -z "$noise" -o "$csr" -4 >/dev/null
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Actually it might be better to keep them, but fix them to expect
ipa-server-certinstall to success.
On 6.11.2015 08:47, Jan Cholasta wrote:
Hi Oleg,
I think you can just remove
TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
imported correctly in this case and I didn't see anything break.
Honza
On 5.11.2015 20:20, Oleg Fayans wrote:
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
Wait a bit, the patch has problems with pylint: it does not build :)
The updated version (without the setupmaster nonsense) is being tested
now.
On 11/05/2015 08:45 AM, Oleg Fayans wrote:
Hi Jan,
Could you take a look at these, whenever you are free?
On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to ipa-replica-prepare
(unsupported under domain level 1)
--
Jan Cholasta
--
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
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Hi Oleg,
I think you can just remove
TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
imported correctly in this case and I didn't see anything break.
Honza
On 5.11.2015 20:20, Oleg Fayans wrote:
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
Wait a bit, the patch has problems with pylint: it does not build :)
The updated version (without the setupmaster nonsense) is being tested
now.
On 11/05/2015 08:45 AM, Oleg Fayans wrote:
Hi Jan,
Could you take a look at these, whenever you are free?
On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to ipa-replica-prepare
(unsupported under domain level 1)
--
Jan Cholasta
--
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
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
Wait a bit, the patch has problems with pylint: it does not build :)
The updated version (without the setupmaster nonsense) is being tested now.
On 11/05/2015 08:45 AM, Oleg Fayans wrote:
Hi Jan,
Could you take a look at these, whenever you are free?
On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to ipa-replica-prepare
(unsupported under domain level 1)
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 4d9b4689ff08e3183fc0610f9dbc664f6e874290 Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Thu, 5 Nov 2015 19:32:37 +0100
Subject: [PATCH] Updated ca-less tests.
A preview. All tests except 2 pass. Those 2 failing ones need a consulting from
jcholast (so far marked as xfail).
https://fedorahosted.org/freeipa/ticket/4589
---
ipatests/test_integration/test_caless.py | 230 ---
1 file changed, 122 insertions(+), 108 deletions(-)
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 9cfba3ee29114badf5a703ccc1d47a1d3e0c41b7..4209f3942d22f7e2213ab52eeca45fb42b4405ef 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -32,13 +32,15 @@ from ipaplatform.paths import paths
from ipapython.dn import DN
from ipatests.test_integration.base import IntegrationTest
from ipatests.test_integration import tasks
+from env_config import get_global_config
_DEFAULT = object()
+config = get_global_config()
+reasoning = "ipa-replica-prepare disabled for domain levels > 0"
def get_install_stdin(cert_passwords=()):
lines = [
-'yes', # Existing BIND configuration detected, overwrite? [no]
'', # Server host name (has default)
'', # Confirm domain name (has default)
]
@@ -86,16 +88,16 @@ class CALessBase(IntegrationTest):
client_hostname = cls.clients[0].hostname
else:
client_hostname = 'unused-client.test'
-env = {
+cls.env = {
'domain': cls.master.domain.name,
'server1': cls.master.hostname,
'server2': replica_hostname,
'client': client_hostname,
'dbdir': 'nssdb',
-'dbpassword': cls.cert_password,
'crl_path': cls.crl_path,
+'dirman_password': cls.master.config.dirman_password,
}
-ipautil.run(['bash', '-ex', scriptfile], cwd=cls.cert_dir, env=env)
+ipautil.run(['bash', '-ex', scriptfile], cwd=cls.cert_dir, env=cls.env)
for host in cls.get_all_hosts():
tasks.apply_common_fixes(host)
@@ -118,7 +120,7 @@ class CALessBase(IntegrationTest):
'-n', 'External CA cert'],
raiseonerr=False)
-super(CALessBase, cls).uninstall()
+super(CALessBase, cls).uninstall(mh)
@classmethod
def install_server(cls, host=None,
@@ -146,6 +148,11 @@ class CALessBase(IntegrationTest):
for filename in set(files_to_copy):
cls.copy_cert(host, filename)
+# Remove existing ca certs from default database to avoid conflicts
+args = ["certutil", "-D", "-d", "/etc/httpd/alias", "-n"]
+host.run_command(args + ["ca1"], raiseonerr=False)
+host.run_command(args + ["ca1/server"], raiseonerr=False)
+
host.collect_log(paths.IPASERVER_INSTALL_LOG)
host.collect_log(paths.IPACLIENT_INSTALL_LOG)
inst = host.domain.realm.replace('.', '-')
@@ -163,6 +170,7 @@ class CALessBase(IntegrationTest):
'-a', host.config.admin_password,
'--setup-dns',
'--forwarder', host.config.dns_forwarder,
+'--domain-level', str(config.domain_level)
]
if http_pin is not None:
@@ -322,9 +330,7 @@ class CALessBase(IntegrationTest):
# Verify certmonger was not started
result = host.run_command(['ge
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Wait a bit, the patch has problems with pylint: it does not build :) The updated version (without the setupmaster nonsense) is being tested now. On 11/05/2015 08:45 AM, Oleg Fayans wrote: Hi Jan, Could you take a look at these, whenever you are free? On 10/30/2015 02:57 PM, Oleg Fayans wrote: Hi, The following patches contain updates to ca-less integration tests. It's still a proof of concept: 2 tests still fail seemingly due to the change in target system logic (marked as xfail with "ask jcholast comment") The test output looks like this: $ ipa-run-tests test_integration/test_caless.py --pdb test session starts = platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4 plugins: multihost, sourceorder collected 88 items test_integration/test_caless.py ..xx..sssss.ss.xx..ssxx. 53 passed, 29 skipped, 6 xfailed in 5620.17 seconds = Numerous skips correspond to the tests related to ipa-replica-prepare (unsupported under domain level 1) -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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
Re: [Freeipa-devel] [PATCH] ca-less tests updated - POC
Hi Jan, Could you take a look at these, whenever you are free? On 10/30/2015 02:57 PM, Oleg Fayans wrote: Hi, The following patches contain updates to ca-less integration tests. It's still a proof of concept: 2 tests still fail seemingly due to the change in target system logic (marked as xfail with "ask jcholast comment") The test output looks like this: $ ipa-run-tests test_integration/test_caless.py --pdb test session starts = platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4 plugins: multihost, sourceorder collected 88 items test_integration/test_caless.py ..xx..sssss.ss.xx..ssxx. 53 passed, 29 skipped, 6 xfailed in 5620.17 seconds = Numerous skips correspond to the tests related to ipa-replica-prepare (unsupported under domain level 1) -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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
