Tests: Fix cert revocation tests #140
Conversation
A bunch of certplugin tests were testing number of revoked certificates with various revocation reasons. Since existence of revoked certificates often depends on other parts of IdM than IPA, it is not really valid to check their presence unless creation of revoked certificate is intentionally tested. https://fedorahosted.org/freeipa/ticket/6349
|
I think that it is not good idea to remove tests, because we are lowering coverage. Therefore NACK. Could we rather rewrite these tests? For example issue certain certificates, revoke them and then test whether there are revoked certs with correct revocation reason. I think that our Tracker could help with it. |
|
Hi, I discussed this with Rob who authored the tests and he said that these tests were there just as a kind of checking that no extra revoked certificates get in. Tests are cca 4 years old, revoked certificates do get in e.g. due to changes in Dogtag (they can be created by other tests and can't be deleted) and cert tests fail. Creating new tests as you described (create cert, revoke it and check it's in the database with correct info) could be separate task, since these tests didn't do such think, they just checked what's already in regardless of how it got there. |
|
Yes, that's true and I understand that these tests depend on previous actions. What I actually wanted to say is that I think that we should rather rewrite these tests right now instead of just removing them. |
|
Ok, I would vote for the new tests and when we have them merged we can safely delete this part of code that is not relevant anymore. The reason we would leave the current (not relevant tests) here is to not loose the information that we need to cover that part. |
|
Ok, I will do it like Ales proposed and will sync this PR when new tests are ready. |
|
Hi alichbox, |
Providing tests for certificate revocation to replace deleted tests from test_cert_find. https://fedorahosted.org/freeipa/ticket/6349
MartinBasti
changed the title
|
Works correctly. ACK |
|
|
A bunch of certplugin tests were testing number of revoked certificates with
various revocation reasons. Since existence of revoked certificates often
depends on other parts of IdM than IPA, it is not really valid to check their
presence unless creation of revoked certificate is intentionally tested.
https://fedorahosted.org/freeipa/ticket/6349