tests: Added basic tests for certs in idoverrides #225
Conversation
| # Create an idview | ||
| api.Command.idview_add(cls.idview) | ||
| # Create a user | ||
| api.Command.user_add(cls.testuser, givenname=u'Bob', sn=u'Dylan') |
There was a problem hiding this comment.
Use the User tracker instance to manipulate with test resources.
| ) | ||
|
|
||
| @classmethod | ||
| def teardown_class(cls): |
There was a problem hiding this comment.
It is preferable not to use nose styled tests. Using pytest's fixtures for the resource management is the way how other tests are implemented.
| super(TestCertManipIdOverride, cls).teardown_class() | ||
|
|
||
| @classmethod | ||
| def setup_class(cls): |
There was a problem hiding this comment.
The same reasoning as in teardown. Fixtures are a better place for resources needed by the test (and are reusable)
|
Please address the inline comments. |
| @@ -0,0 +1,95 @@ | |||
| # | |||
| # Copyright (C) 2015 FreeIPA Contributors see COPYING for license | |||
| @@ -11,6 +11,30 @@ | |||
| from ipatests.util import assert_deepequal, raises | |||
| from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test | |||
| from ipatests.test_xmlrpc.testcert import get_testcert | |||
| from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker | |||
| from ipatests.test_xmlrpc.tracker.idview_plugin import IdviewTracker | |||
There was a problem hiding this comment.
This commit precludes the implementation of IdviewTracker. Please rebase them in the right order.
|
|
||
| def test_03_add_the_same_cert_to_idoverride(self, testuser, | ||
| idview, cert1, cert2): | ||
| raises(errors.ExecutionError, |
There was a problem hiding this comment.
I'm aware that the rest of the test uses this utility function, however I think using pytest.raises context manager adds readability here. That way the add/del cert methods for certificate could be moved to the tracker itself.
| def __init__(self, cn, **kwargs): | ||
| super(IdviewTracker, self).__init__(default_version=None) | ||
| self.cn = cn | ||
| self.dn = DN(('cn', cn), 'cn=views', 'cn=accounts', api.env.basedn) |
There was a problem hiding this comment.
You can use api.env.container_views here.
| cn=(self.cn,), | ||
| dn=unicode(self.dn), | ||
| idoverrideusers=[], | ||
| objectclass=(u'ipaIDView', u'top', u'nsContainer') |
There was a problem hiding this comment.
please use the objectclasses module for this.
|
Please address the inline comments |
|
@apophys done, thank you for review! |
| @@ -227,3 +227,9 @@ | |||
| u'top', | |||
| u'ipaca', | |||
| ] | |||
|
|
|||
| idview = [ | |||
There was a problem hiding this comment.
idview is already in the module on line 185.
| cert_add_cmd = api.Command.idoverrideuser_add_cert | ||
| cert_del_cmd = api.Command.idoverrideuser_remove_cert | ||
|
|
||
| def del_cert_from_idoverride(self, username, view_name, cert): |
There was a problem hiding this comment.
Since the method operates on the idview entry, it makes sense to move it to the tracker to be available for other tests as well. Please move it to tracker.
| summary=result.get('summary') | ||
| ) | ||
|
|
||
| def add_cert_to_idoverride(self, username, view_name, cert): |
|
Thank you for the change of the order and using the objectclasses module. There are still things I'd like to be changed, though. |
Needed for basic certs in idoverrides tests https://fedorahosted.org/freeipa/ticket/6412
|
Thank you for addressing the issues. The implementation is somehow minimal, however in the future it can be extended as needed. |
https://fedorahosted.org/freeipa/ticket/6412