[Freeipa-devel] [PATCH] 801-806 webui-ci: otptoken tests

[Milan Kubik]

On 05/12/2015 01:57 PM, Petr Vobornik wrote:
> On 05/11/2015 01:25 PM, Milan Kubik wrote:
>> On 05/07/2015 01:38 PM, Petr Vobornik wrote:
>>> On 02/19/2015 03:51 PM, Petr Vobornik wrote:
>>>> https://fedorahosted.org/freeipa/ticket/4307
>>>>
>>>> For ipa-4-1 apply:
>>>> - patch 800 (different thread)
>>>> - patches 801-806
>>>>
>>>> For master apply:
>>>> - patch 800 (different thread)
>>>> - patch 807 (different thread)
>>>> - patch 801-master
>>>> - patches 802-806
>>>>
>>>> Patch 801 allows to use ipalib rpc client in Web UI test suite.
>>>> Patches 802-805 are various ui_driver fixes to allow stuff in patch 
>>>> 806.
>>>>
>>>> == [PATCH] 806 webui-ci: otptoken tests ==
>>>>
>>>> Basic otptoken Web UI CI coverage.
>>>>
>>>> tests:
>>>> * crud for otptokens as admin
>>>> * crud for normal users
>>>> * checks fields of adder dialog for both token types and user role
>>>> (admin/user)
>>>> * token actions as admin (enable, disable, delete)
>>>> * token actions as normal user (delete)
>>>> * login as normal user with hotp and totp token
>>>> * sync token hotp and totp token as normal user and then login
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4307
>>>>
>>>> == [PATCH] 805 webui-ci: allow custom names for disable/enable
>>>> actions ==
>>>>
>>>> Not all disable and enable actions are called 'disable' and 'enable'.
>>>>
>>>> == [PATCH] 804 webui-ci: allow to update pkey in post-add in 
>>>> basic-crud
>>>>   tests ==
>>>>
>>>> == [PATCH] 803 webui-ci: add post_add_action ==
>>>>
>>>> post add action allows to fill autogenerated values, e.g. a pkey of 
>>>> new
>>>> otptoken.
>>>>
>>>> This value can be then used in other subsequent test which would 
>>>> depend
>>>> on it - like crud tests.
>>>>
>>>> == [PATCH] 802 webui-ci: fix negative visibility check ==
>>>>
>>>> Allow to define, that element doesn't have to be present on a page for
>>>> negative visible checks.
>>>>
>>>> E.g. if element is added only if it's displayed and is removed
>>>> otherwise.
>>>>
>>>> == [PATCH] 801 webui-ci: support direct IPA API calls ==
>>>>
>>>> Add IPA API support to ui_driver. It leverages new ipalib RPC client's
>>>> forms based authentication. It then allows to call an IPA API while
>>>> the machine is not an IPA client nor is kerberized.
>>>>
>>>> api's environment values are taken from test configuration and
>>>> therefore duplication in ~/.ipa/default.conf is not required.
>>>>
>>>> Since the machine doesn't have to be IPA client, it then also doesn't
>>>> have nss database with IPA's CA certificate. Therefore on each API
>>>> initialization a new NSS database is created with a CA certificate
>>>> downloaded from IPA. This db is deleted in tearDown phase.
>>>>
>>>> Usage:
>>>>
>>>> 1. as admin one can immediately call rpc commands, api will be
>>>> initialized upon first request and is available under self.api
>>>> (assuming self is ui_driver):
>>>>    self.api.Command.user_del(USER_ID, **{'continue': True})
>>>>
>>>> 2. to reconnect as other user:
>>>>    self.reconnect_api(USER_ID, USER_PW)
>>>>
>>>> 3. reconnect back as admin:
>>>>    self.reconnect_api()
>>>>
>>>
>>> Patch #803 needed rebase.
>>>
>>>
>> Hi, thanks for the patches.
>>
>> Please, fix pep8 complaints in 803, 805 and 806.
>
>
> $ git diff HEAD~6 -U0 | pep8 --diff
>
> returns 20x E501 line too long
>
> IMO, it's better this way for better code readability.
>
>
>> Also, change the header in 806 to the shorter version, please.
>
> Fixed, patches were regenerated.
>
>>
>> #
>> # Copyright (C) 2015  FreeIPA Contributors see COPYING for license
>> #
>>
>> Patches 801, 802 and 804 look good to me.
>> The test cases in 806 look good to me as well.
>>
>> Milan
>>
I have reviewed the pep8 complaints closely and yes, readability would 
suffer a little.
<nicpick-alert>I don't like the line 317 after patch 806.</nicpick-alert>
Fix it at your discretion.
Otherwise ACK.

Thanks,
Milan