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. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code