There is already create test_user as part of UserAccounts or nsUserAccounts. Why do you define your own?

There is already create test_user as part of UserAccounts or nsUserAccounts. Why do you define your own?

This is just to adjust with the acis , as if you use create test_user as part of UserAccounts it will create one with uid= and for some of the acis i need cn= , thats why in some cases i am using
create_test_user from working_constans.py in.

Why not change the aci's to use uid instead ...?

@firstyear , i have removed whole working_constans.py now only generic create_test functions are there .

This is a weird pattern, DEFAULT_SUFFIX is a "Domain" type, not a "UserAccount", so I don't see what you are trying to assert here either.

Rather than trying to make everything one-liners, just do:

ua = UserAccount(conn, "uid= ....".format(...))
ua.bind()
ua.add('roomnumber', '9999')

Not that it matters, but the homedirectory would probably be /home/jvedder'

Saying this, you have two static strings, why are you con-catinating them?

Don't use raw modrdn

No raw delete_s

Don't do long one-liners like this. Code needs to be readable.

Okay, STOP using UserAccount(inst, dn) on things that are not user accounts. There is an OrganisationalUnit type for this. Use the proper types and classes because this could break in ways you don't know yet.

Don't do raw rename.

Don't use raw modrdn

As this tests are about modrdn , how am i suppose to go ahead ?

The correct way to do this, is to add modrdn capability to _mapped_object.py, in DSLdapObject. Look at def rename in that file for an example of how it's done. It's basically the only place in the lib389 codebase I'll allow raw ldap calls because it is our abstraction layer.

I would also expect that the difference between "rename" and "modrdn" is clearly explained in the documentation of the DSLdapObject.

I know it seems annoying, but the issue here is we have TONS of legacy debt. I have spent literally years trying to un-bury us from this. This means that the hard way is sometimes the right way long term. Every time we allow a single bypass, you just make "William of the Future's" job harder. This is why I insist we take a little bit of extra time now to "do it right" because then we never have to fix it ever again.

PS: It looks like you may not need to because reading http://www.python-ldap.org/en/latest/reference/ldap.html?highlight=modrdn_s#ldap.LDAPObject.modrdn_s it looks like modrdn_s is actually emulated by rename anyway, so you can just use the existing DSLdapObject rename() function, with the appropriate flags because it does the same thing.

Don't do raw rename.

what about this one , instead modrdn you asked me to use rename , now i am asked not use raw rename ? how is it even possible .

Don't use raw rename_s on the dirsrv instance. Use rename on the mapped object type. For example code:

useraccount = UserAccounts(instance, DEFAULT_SUFFIX).get('william')
useraccount.rename('uid=claire')

That would rename "william" to "claire". To move the object in the tree you do:

useraccount.rename('uid=claire', 'ou=something else,%s' % DEFAULT_SUFFIX)

The raw renames on DirSrv are what I am saying not to use. I'm saying to use the abstractions in lib389's DSLdapObject which fix dumb ldap behaviour and annoying python issues.

@firstyear , all changes are done as per your suggestion

@firstyear , this one is ready , all changes are done as per your suggestion

Made this comment in another file, but you can probably inline this properties into the .create() because you never reuse the value.

you can just make userPassword a property rather than needing an extra .set operation.

Does this work without concat? You may be better to have:

ou.set('aci',
    'long one line aci'
    .format(things))

Space this out properly (each key on one line.

similar comment about reuse.

So you have this really large "per function fixture" right. You add and remove HEAPS of objects. But not all of them are used for every test.

Your fixture should delete everything, but each test should just create the limited set of aci and objects required. By setting up so much every single time, you are using more CPU time, you risk polluting the test environment (How can you be sure of what aci was in play?). You also may find you don'th need so many user account objects and aci definitions, because you are creating only what is needed per test.

So I think this fixture here should be broken down, and the required smaller pieces should be put into their relavent tests.

So you have this really large "per function fixture" right. You add and remove HEAPS of objects. But not all of them are used for every test.
Your fixture should delete everything, but each test should just create the limited set of aci and objects required. By setting up so much every single time, you are using more CPU time, you risk polluting the test environment (How can you be sure of what aci was in play?). You also may find you don'th need so many user account objects and aci definitions, because you are creating only what is needed per test.
So I think this fixture here should be broken down, and the required smaller pieces should be put into their relavent tests.

I have remove the fixture you are saying about , but i must say that if singly test case fails in that script , all rest test cases will fail automatically , it will give "User already exists error"

@firstyear all other changes also done as per your suggestion

I don't think you need the call to raw_entry here. You are trying to prove the entry exists for the assert, but .get() won't work if the entry doesn't exist. Additionally, there is a .exists method too.

@firstyear all changes also done as per your suggestion

Given the target is ldap:/// all, you don't need "name, ou=..." here. lib389 will do ldap escaping on values too, so it's pretty likely this will actually be getting escaped and the users are NOT created in the ou like you think they are .....

Format this properly please. One key:value per line. I think I said this before ....

Ignore the ldap:///all comment about ou and escaping, re-reading, I'm incorrect.

@firstyear all changes are done as per your suggestion

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3244

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.