Shouldn't it be "nsManagedRole" (note the 'd' in Managed) to match the objectClass name?

So I never understand why you do this pattern. Why not do:

nsroles = nsManagedRoles(topo.standalone, DEFAULT_SUFFIX)
nsrole1 = nsroles.create(properties={'cn': 'role1'})
...
uas = UserAccounts(topo.standalone, DEFAULT_SUFFIX)
user = uas.create(properties = {
    'nsRoleDN': nsrole1.dn
})

So keep the nsrole1 around then you can use the .dn, and give that to the user?

Don't do a replace on this aci, do "add" to add the ACI to the set (replace destroys all existing acis and can break tests in amazing ways like I found out recently. ....)

Why not do "for i in uas.list(): i.delete()".

Okay, I don't understand, but you keep refusing to use the pluralised DSLdapObjects types? They are there to proxy creates, they help setup things, and they can get by name and list etc. I think there are ways to make this test much more legible, robust and better. Right now it looks really fragile.

@firstyear all changes are done as per your suggestion , please check

Just a general naming remark. Many 389-ds objects come from nescape days and start with 'ns'. lib389 is new and intend to be the unique interface to configure/reach 389-ds. It could be a opportunity to drop 'ns' in the defined lib389 classes even if internally the function use 'ns..' attributes/oc.

Align the comment with the class , dropping the 'ns'

idem, please remove the 'ns'

Local variable could be also 'roles' and 'role'

idem , drop of 'ns'. in the comment also below

As nsFilterRoles is not widely used so far. Could you change it into FilterRoles or is it too late ?
Also the name of the file 'nsrole.py' could be renamed 'role.py'

As nsFilterRoles is not widely used so far. Could you change it into FilterRoles or is it too late ?
Also the name of the file 'nsrole.py' could be renamed 'role.py'

It has been already used in acivattr_test.py, basic_test.py, cos_test.py (already maerged).So we cant make these change right now .

All other changes are done as per your suggestion

@aborah I think we can make those changes as part of this patch, so can we do the rename like @tbordaz suggests? He generally has good points, and test cases are not "fixed in stone". We change them a lot.

Minor change test_nsmangerole --> test_managedrole (without 'ns' and with 'd')

test_managedrole

@tbordaz all changes are done as per your suggestion . kindly check

@aborah thanks for your continuous effort moving away from 'ns' ;)
The code looks good to me so you have my ACK but wait for @firstyear ACK before merging (he is the true python expert)

Seems fine to me at this point,

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/3313

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.