Re: [PATCH] modlist.py: Compare old and new values

[Bhuvaneswaran A. <bh...@co...>]

Thanks for your response. Please find my comments and a patch for
test_modlist.py script.

On Wed, 2006-09-13 at 10:20 +0200, Michael Ströder wrote:
> Bhuvaneswaran Arumugam wrote:
> > 
> > BUG:
> > 
> > Currently you compare the length of old and new values.
> 
> Judging from your patch you mean the attribute value lists.

Yep. After i went through test_modlist.py cases, i observed it was
because i don't use list as the value of ldap key/variable. Please find
attached a patch to test_modlist.py script. I've included two test cases
viz. 'case 2' (list) and 'case 3' (string). 'case 2' is ok but 'case 3'
reports following error:

<error>

Testing function addModlist():

Testing function modifyModlist():
result_modlist is empty '[]'
modifyModlist({'nisMapEntry':
'-intr,hard,tcp,rsize=32768,wsize=32768,nfsvers=3
mgr:/u1/chroot//test/grue'},{'nisMapEntry':
'-intr,hard,tcp,rsize=32768,wsize=32768,nfsvers=3
mgr:/u1/chroot//test/mrue'}) returns
[]
instead of
[(0, 'nisMapEntry', '-intr,hard,tcp,rsize=32768,wsize=32768,nfsvers=3
mgr:/u1/chroot//test/mrue'), (1, 'nisMapEntry', None)].
</error>

But i'm unsure whether it's the intended behavior.

> > So, it does not
> > recognize the diff when there is no change in length but the actual
> > string itself is changed (ex: s/usr/var/g).
> 
> It's not that easy. replace_attr_value can also be set to 1 after line
> 87. Could you please post an example of the error?
> 
> Best thing would be to add your errornous case as test-case to
> Tests/Lib/ldap/test_modlist.py.
> 
> > Due to this bug, it returns
> > empty ldif so subsequent call to modify_s (in my application) fails.
> 
> Hmm, are you using module ldif afterwards to turn the modlist into LDIF?
> How does the modlist look like. Is it empty?
> 
> > <       replace_attr_value = len(old_value)!=len(new_value)
> > ---
> > 
> >>      replace_attr_value = old_value!=new_value
> 
> Please note that old_value and new_value are actually lists of attribute
> values. This line above sets only a first indicator that an attribute
> has changed to avoid performance loss due to calling the function
> list_dict().
> 
> Ciao, Michael.
> 
-- 
Regards,
Bhuvaneswaran