Re: [389-devel] Please review lib389 ticket 47568: Rename DSAdmin class (2nd)

2013-10-25 Thread thierry bordaz

On 10/25/2013 11:38 AM, Roberto Polli wrote:

On Friday 25 October 2013 11:18:53 thierry bordaz wrote:

lib389/brooker.py:795: python variable naming convention: I would get
stick
with the "_" instead of camelCase  and change whenever possible.

If you prefer to use '_' also for local variable, I am fine.

Using camel just for classes is more explicative, and I find that "_"  are
easier to read and replace with sed ;)


tests/dsadmin_test.py: I renamed it lib389_test.py, you can merge my
changes tests/dsadmin_test.py:39: why remove the addbackend_harn?

Humm, to be honest... I do not know how to rename files :-)

git mv dsadmin_test.py lib389_test.py ;)

:-[ !! so easy :-D





tests/replica_test.py:119: you're using Backend.delete in a class that
should test just Replica. I would use harness and the standard
python-ldap methods in setup/teardown, so that we can change the Backend
and Replica class without at least breaking the tests.

I miss your point. It is calling in teardown conn.backend.delete, is
that the call that is not correct ?

That's just an IMHO: see those cases:
1- I change the Backend class and break the replica test: I'll look for errors
in Replica while the issue is in Backend
2- somebody works on the Backend class, I work on the Replica one: he can
break my tests.

Splitting the test stuff in an harness module will reduce the impact of all
that. As an example, I could even agree the setup process be done populating
entries via an LDIF. If I test Replica, Backend or Suffix I shouldn't have
other dependencies distracting me.


Is that related to Mock. For example in Replica, we need a suffix and a 
replica but do not want to rely on them.
If instead of creating a real suffix/backend, we have mock of them we 
could develop the replica tests without any concerns regarding further 
changes in suffix/backend.

Is that your concern ?

regards
thierry

Let me know + Peace,
R.
   



--
389-devel mailing list
389-de...@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel

Re: [389-devel] Please review lib389 ticket 47568: Rename DSAdmin class (2nd)

2013-10-25 Thread Roberto Polli
My comments (a github like platform for comment could be really useful)
> https://fedorahosted.org/389/attachment/ticket/47568/0002-Ticket-47568-Renam
> e-DSAdmin-class.patch

line: comment
lib389/__init__.py:1: the module is lib389, not dirsrv

lib389/brooker.py:795: python variable naming convention: I would get stick 
with the "_" instead of camelCase  and change whenever possible.

tests/dsadmin_test.py: I renamed it lib389_test.py, you can merge my changes
tests/dsadmin_test.py:39: why remove the addbackend_harn? 

tests/replica_test.py:119: you're using Backend.delete in a class that should 
test just Replica. I would use harness and the standard python-ldap methods in 
setup/teardown, so that we can change the Backend and Replica class without at 
least breaking the tests. 

Let me know + Peace,
R. 

-- 
Roberto Polli
Community Manager
Babel S.r.l. - http://www.babel.it
T: +39.06.9826.9651 M: +39.340.652.2736 F: +39.06.9826.9680
P.zza S.Benedetto da Norcia, 33 - 00040 Pomezia (Roma)

CONFIDENZIALE: Questo messaggio ed i suoi allegati sono di carattere 
confidenziale per i destinatari in indirizzo.
E' vietato l'inoltro non autorizzato a destinatari diversi da quelli indicati 
nel messaggio originale.
Se ricevuto per errore, l'uso del contenuto e' proibito; si prega di 
comunicarlo al mittente e cancellarlo immediatamente.
--
389-devel mailing list
389-de...@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel