UserAccounts can be used here

You can use ReplicationManager's 'test_replication_topology()' method for this (wait for everything to be insync).
Or you can create a loop with timeout time.sleep(1) and check for your assert there.

Anyway, avoid any big sleeps (bigger than time.sleep(1)

You reuse the assert a lot. It can be encapsulated

You assume that the users are created. If the test case will be run stand alone - the users won't be there.
Please, put the users to the 'test_users' fixture

I think it should be three steps

Why do you use such a format?
I think usual topo_m4.ms["master{}".format(i))] will work without any issue

You can use Agreement(DSLDapObject) here

UserAccounts can be used here

@spichugi all chages has been done , i am using Entry in one test case(test_bob_acceptance_tests) as if i use UserAccounts , after doing modrdn_s , it complaints about abject class.

Please merge if all ok.

@mreynolds please merge this one also if , all ok

As I've mentioned in another PR. You can use users.create_test_user for standard users.

Actually, if you create the same users in all test cases, you can add it to the fixture (with def fin())

It is better to name the variable with actual things it represents.

Like 'for user in users.list():' # yes, there is no need for the list comprehensions feature.

repl.test_replication doesn't check for an attribute values order
you can check the particular entry searches

Please, never use os.system() - use subprocess module instead

But in this case you can use os.chown()

it can be just:

for inst in topo_m4:
inst.stop()

topo_m4.all_insts.get("master1") can be written as topo_m4.ms["master1"]. It is shorter

please, don't use legacy methods. Use DSLDapObject approach instead

Please, don't use such sleeps. If you need to wait for the replication to happen. You can use ReplicationManager.wait_for_replication() method

if you don't need 'i', you can use '_' as a placeholder there.

for _ in range(100)

Please, use a fixture or just a function. Don't use 'Class' because it is inappropriate use (you dont have any methods)

You can use 'for' loop here

Don't use str(instance.search_s) approoach at all. Use the programming language and lib389 API semantics

@spichugi , all chages has been done as per your suggestion

Test case description is missing in all test cases

Please, wrap the user's creation in the fixture. So it is cleaned up even if test case failed

Why do you need time.sleep(5) here?

please, use search_s or DSLdapObjects functionality. 'getEntry' will be deprecated sooner or later

please, replace all topo_m4.all_insts.get("master{}".format(i)) with topo_m4.ms["master{}".format(i)] - there is no sense in using all_insts.get

All the time.sleep bigger than (1) should be removed. It is a bad pattern that unnecessarily prolongs the test case execution

Please, replace all of the patterns 'assert "anything" in str(' with something that uses lib389 and Python semantics/ As I gave you an example before

Do we really need add_s here? I think UserAccounts will work too

The same here. You can use UserAccounts

You can use UserAccounts here. And you can return the UserAccount instances in _bulkaddusers and use it here (I think it makes sense to transform _bulkaddusers into a fixture)

@spichugi , all chages has been done as per your suggestion

Ack

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

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.