Functional tests for auth. plugins #232
Conversation
This is preparation extension of functional tests. Signed-off-by: Petr Čech <pcech@redhat.com>
Codecov Report
@@ Coverage Diff @@
## master #232 +/- ##
=========================================
+ Coverage 72.74% 73.4% +0.65%
=========================================
Files 41 42 +1
Lines 4433 4614 +181
Branches 449 484 +35
=========================================
+ Hits 3225 3387 +162
- Misses 1052 1057 +5
- Partials 156 170 +14
Continue to review full report at Codecov.
|
This test suite tries entire configuration matrix of SimpleCredsAuth plugin. Signed-off-by: Petr Čech <pcech@redhat.com>
This test suite tests configuration options of SimpleHeaderAuth plugin. Signed-off-by: Petr Čech <pcech@redhat.com>
Signed-off-by: Petr Čech <pcech@redhat.com>
Signed-off-by: Petr Čech <pcech@redhat.com>
|
I just removed invalid comments from configuration templates. |
| @@ -17,8 +20,105 @@ | |||
| from custodia.server.config import parse_config | |||
|
|
|||
|
|
|||
| def wait_pid(process, wait): | |||
There was a problem hiding this comment.
Why did you move the functions out of the class? If the test class doesn't suite your needs, please talk to @raildo and modify the base class.
| current_uid = None | ||
|
|
||
| if meta_uid == "correct_id": | ||
| current_uid = pwd.getpwuid(os.geteuid()).pw_uid |
There was a problem hiding this comment.
This is kinda pointless, SOCK_PEERCRED always returns the effective uid and gid.
| current_gid = None | ||
|
|
||
| if meta_gid == "correct_id": | ||
| current_gid = grp.getgrgid(os.getegid()).gr_gid |
| raise OSError('Timeout error') | ||
|
|
||
|
|
||
| def translate_meta_uid(meta_uid): |
There was a problem hiding this comment.
Instead using translate_meta_uid(), I'd rather see some constants, e.g.:
USER_ID, USER_NAME, USER_WRONG_ID, USER_WRONG_NAME = get_test_user()
| class UniqueNumber(object): | ||
| unique_number = 0 | ||
|
|
||
| def get_unique_number(self): |
There was a problem hiding this comment.
The function is not necessary. If you add request to any test argument list, then you can access the test name as request.node.name. The name also contains the parameter values. This gives you an unique name that reflects the test case, too.
| self.custodia_conf = os.path.join(self.test_dir, 'custodia.conf') | ||
| self.params = conf_params | ||
|
|
||
| self.out_fd = os.open(os.devnull, os.O_RDWR) |
There was a problem hiding this comment.
This leaks a fd when enter fails. My original code used a global scoped fixture to have a single FD for all tests.
| self.env = os.environ.copy() | ||
| self.env['CUSTODIA_SOCKET'] = self.config['server_socket'] | ||
|
|
||
| def _get_conf_template(self): |
There was a problem hiding this comment.
I don't like this approach. You should rather create specialized subclasses in each test module:
class SimpleCredsCustodiaServer(CustodiaServer):
template = 'tests/functional/conf/template_simple_creds_auth.conf'
def extra_parameters(self):
return {
'UID': translate_meta_uid(self.params['meta_uid']),
'GID': translate_meta_gid(self.params['meta_gid'])
}
| 'meta_uid': meta_uid, | ||
| 'meta_gid': meta_gid} | ||
|
|
||
| with CustodiaServer(self.test_dir, params) as server: |
There was a problem hiding this comment.
This will start and stop a server for each parameter, which is rather slow. Do we really need integration tests for this case? You can archive the same test coverage with a unit test.
There was a problem hiding this comment.
I think, the speed is not our issue as long as you could run tests without those. But the point is that I look on Custodia as on black box. I really would like to test it in this way, because it is really essential use case. I want to see that entire Custodia work properly in this point.
If this is unacceptable to you, I can limit the set considerably. But I really don't prefer it.
There is a basic set of functional tests for auth. plugins.
For this set it is essential to use parameters, so we don't use fixtures but context manager which prepare right configuration and start Custodia server for us.
For more testing is good idea to prepare class which can prepare more difficult configuration automatically.