[Freeipa-devel] [TEST][Patch-0027] Fixed test failure during in-tree session, ticket N 5736

[Oleg Fayans]

Hi Martin

On 03/16/2016 02:39 PM, Martin Basti wrote:
> 
> 
> On 16.03.2016 10:59, Oleg Fayans wrote:
>> With this patch applied integration tests pass and in-tree tests are
>> gracefully skipped.
>>
>> @mkubik, It is not possible to put the decorator to util.py as per our
>> discussion, because it uses tasks, so tasks must be imported. But tasks
>> already import util, which leads to circular imports. So I've put it to
>> tasks.py
>>
>>
>>
> NACK
> 
> 1)
> Use right ticket in commit message (#5723)

But (#5736) is exactly the issue that is being addressed. Probably note
both tickets in the commit message?

> 
> 2)
> Link to ticket should be last in the commit message
> 
> 3)
> dereplicafy
> 
> 3a)
> wrong doc string, it removes *only* replicas not clients

No, in fact it removes both:
uninstall_replica(args[0].master, host)
uninstall_client(host)

Both tasks have raiseonerr set to False, which means that even if
replica was not installed but the client was - it will also be removed

> 
> 3b)
> can we rename it to something different? (replicas_cleanup,
> replicas_uninstall, replicas_teardown)

replicas_cleanup, or even topo_cleanup sounds OK to me.

> 
> 4)
> Please fix commit message
> - Wile trated correctly
> - followiong
> - rewrote -> rewrite

Will do

> 
> 5)
> decorator
> +    def wrapped(*args):
> +        func(*args)
> +        for host in args[0].replicas:
> 
> Shouldn't be there try-finally around func() call, or something?
No, the wrapped function is a test_* method: if it fails we need to see
the original failure

> Are you sure that there is no need to return result of func()?
The same applies here: we never return results from test_* methods

> 
> *) Please create additional patch that will add licence there
> 
> 
Will do :)


-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.