Fails for me here. F30 with 389-ds-base built from your PR branch

py.test-3 --pylint reports a lot of warnings. Please, check it

I think it should be Accounts instead of DSLdapObjects

Either it should be transformed to the common case for the admin group (utilizing all attributes in a proper way).
Either it should be removed because creating this kind of account is a special case and I'd rather have it in the test suite code for the better visibility.

These three objectclasses are not related to the pure AdminGroup entry.

Please, avoid names like this: autoMembers_from_tet_test.py

The name should indicate the cause of why the test cases are in this test suite together

We have AutoMembershipPlugin object for that

Please, remove this function and use certain objects for creating proper entries. Like Groups, etc.

suff -> suffix (please, try to avoid word shortenning)
attr -> backend_name

It is created automatically during backend.create

You named it autouserGroups, etc. So why do you create generic OrganizationalUnits instead?

You create only one user here

We have DSLdapObject.preset() method. It is better and more transparent to use it

Only one group is added

I am not sure why you need this but you can use DSLdapObject.ensure_state() here

What is the second step? I think the function name is misleading

You name it the same everywhere automembers. It is confusing...

Could be done in a for loop with only two arguments

We usually name topology.standalone as instance. So it's a bit confusing

You already has marked it as bz834053. So the name should reflect the issue the test case checks

The same here

You reuse Config(instance1) a few times, why not assign it to a variable?

It is unclear what you test here... You can add some verb to the name, it'll help probably...

Why not just iterate here? Maybe even through a range() function.

In many test cases, you have only one step but the actual content has 3+ steps. I think the docstring should reflect the main actions at least (no need for overdoing it though)

Fails for me here. F30 with 389-ds-base built from your PR branch

None of the test case failed for me . Please discuss this with viktor as some test cases were failing for him also , but for me non of the test cases failed

Please, avoid names like this: autoMembers_from_tet_test.py
The name should indicate the cause of why the test cases are in this test suite together

I have renamed it as autoMembers_task_retro_chlog_test.py as this script will contain automemtask and retro changelog test cases , which i will add on the top of the same file , with my next PR

@spichugi all changes are done as per your suggestion

Please, go once again through the list of my concerns and check if everything is implemented... Or reply why you haven't implemented it...

These three objectclasses are not related to the pure AdminGroup entry.

I need to create entries as: http://git.app.eng.bos.redhat.com/git/dirsrv-tests.git/tree/Shared/DS/6.0/sh/appstates.sh

AddUsers()

objectClass: top
objectClass: person
objectClass: posixAccount
objectClass: nsAdminGroup
objectClass: inetuser

Thats why i have included those objectClass

1. Fails for me here. F30 with 389-ds-base built from your PR branch --- replied
2. py.test-3 --pylint reports a lot of warnings. Please, check it --- done(Ignored- Redefining, Invalid function name)
3. I think it should be Accounts instead of DSLdapObjects -- done
4. Either it should be transformed to the common case for the admin group (utilizing all attributes in a proper way).
   Either it should be removed because creating this kind of account is a special case and I'd rather have it in the test suite code for the better visibility. --- removed
5. These three objectclasses are not related to the pure AdminGroup entry. -- replied
6. Please, avoid names like this: autoMembers_from_tet_test.py --- renamed and replied
7. We have AutoMembershipPlugin object for that --- removed and implmented
8. Please, remove this function and use certain objects for creating proper entries. Like Groups, etc. -- removed and implmented
9. suff -> suffix (please, try to avoid word shortenning) -- implmented
10. It is created automatically during backend.create --- removed and implmented
11. You named it autouserGroups, etc. So why do you create generic OrganizationalUnits instead? --- As we have different backends we need thes generic OUs to create AutoMembershipRegexRules and for other stuff
12. You create only one user here -- corrected and implmented
13. We have DSLdapObject.preset() method. It is better and more transparent to use it -- implmented
14. Only one group is added -- corrected and implmented
15. I am not sure why you need this but you can use DSLdapObject.ensure_state() here --- implmented ,we need it , please check test_multi_valued_automemberdefaultgroup_with_uniquemember , test_invalid_automembergroupingattr_member etc
16. What is the second step? I think the function name is misleading -- corrected and renamed
17. You name it the same everywhere automembers. It is confusing... -- corrected and renamed
18. Could be done in a for loop with only two arguments -- corrected and implmented
19. We usually name topology.standalone as instance. So it's a bit confusing -- corrected and implmented
20. You already has marked it as bz834053. So the name should reflect the issue the test case checks -- corrected and implmented
21. You reuse Config(instance1) a few times, why not assign it to a variable? -- implmented
22. It is unclear what you test here... You can add some verb to the name, it'll help probably... --- corrected and implmented
23. Why not just iterate here? Maybe even through a range() function. --- corrected and implmented
24. In many test cases, you have only one step but the actual content has 3+ steps. I think the docstring should reflect the main actions at least (no need for overdoing it though) --- Done
25. Please, go once again through the list of my concerns and check if everything is implemented... Or reply why you haven't implemented it... --- Done

Fails for me here. F30 with 389-ds-base built from your PR branch --- replied

Try this Vagrant file - https://paste.fedoraproject.org/paste/xTJUfS10bNqF9QTQaNExgQ
It produces a pretty clean env.

py.test-3 --pylint reports a lot of warnings. Please, check it --- done(Ignored- Redefining, Invalid function name)

C:  1, 0: Module name "autoMembers_task_retro_chlog_test" doesn't conform to snake_case naming style (invalid-name)

Also, I think the naming is completely misleading:
autoMembers - the test suite name already has the word. task and retro_chlog are not the only things there. You test the basic functionality so I think it should be basic or acceptance.

W: 61,16: Duplicate string formatting argument 'suffix', consider passing as named argument (duplicate-string-formatting-argument)

Still present.

These three objectclasses are not related to the pure AdminGroup entry. -- replied

Then the objectclasses should be created additionally if they are needed for you test case.
In general, AdminGroup doesn't need it.

I am not sure why you need this but you can use DSLdapObject.ensure_state() here --- implmented ,we need it , please check
test_multi_valued_automemberdefaultgroup_with_uniquemember , test_invalid_automembergroupingattr_member etc

Could you please explain the logic why we need to add the object classes exactly at that point and not at the beginning while creating the group?
Probably, I miss something...

I think it is more natural to return a boolean here and then assert in the test case if group_is_present

What is the change here? It will confuse git history

Just noticed and I don't understand why you add nsAdminGroup under user.py.
Probably should be in group.py

Just noticed and I don't understand why you add nsAdminGroup under user.py.
Probably should be in group.py

Yes , i was also thinking like that , moved to group.y

What is the change here? It will confuse git history

Gone

I think it is more natural to return a boolean here and then assert in the test case if group_is_present

Done

Try this Vagrant file - https://paste.fedoraproject.org/paste/xTJUfS10bNqF9QTQaNExgQ
It produces a pretty clean env.

Now its should not fail

C: 1, 0: Module name "autoMembers_task_retro_chlog_test" doesn't conform to snake_case naming style (invalid-name)

Gone now

Also, I think the naming is completely misleading:
autoMembers - the test suite name already has the word. task and retro_chlog are not the only things there. You test the basic functionality so I think it should be basic or acceptance.

renamed to basic_test.py

W: 61,16: Duplicate string formatting argument 'suffix', consider passing as named argument (duplicate-string-formatting-argument)

Gone

Still present.

Then the objectclasses should be created additionally if they are needed for you test case.
In general, AdminGroup doesn't need it.

Done

I am not sure why you need this but you can use DSLdapObject.ensure_state() here --- implmented ,we need it , please check
test_multi_valued_automemberdefaultgroup_with_uniquemember , test_invalid_automembergroupingattr_member etc

Could you please explain the logic why we need to add the object classes exactly at that point and not at the beginning while creating the group?
Probably, I miss something...

we have created AutoMembershipDefinition with autoMemberGroupingAttr: groupOfNames (comes from Group type)
now in the test we are changing AutoMembershipDefinition(autoMemberGroupingAttr) with autoMemberGroupingAttr: uniquemember (comes from UniqueGroup type) with existing autoMemberGrouping. newly created user should be added to same host group as uniqueMember and verse versa . Its was just as sigle example.

Please check the main script
http://git.app.eng.bos.redhat.com/git/dirsrv-tests.git/tree/testcases/DS/6.0/autoMembers/autoMembers.sh

autoMembers_09, autoMembers_10 etc

Why it is commented out and the next line is there?..

py.test-3 --pylint reports some warnings. Please, check it

Still fails here. Have you checked with a clean install from my Vagrantfile?

Still fails here. Have you checked with a clean install from my Vagrantfile?

test_valid_and_invalid_automembergroupingattr fails after 7096094e0f26ad3f4acbbffeefc0a174a791686d
What test is doing is it changes the objectClass of the group to the incompatible with the members of the group. Previously automember plugin would deny adding a member with the incorrect OC, but since the change above it allows.

@mreynolds, is this expected?

Why it is commented out and the next line is there?..

Gone

py.test-3 --pylint reports some warnings. Please, check it

Done

Still fails here. Have you checked with a clean install from my Vagrantfile?

Fixed , as suggested by viktor .

Why do you have this line? Even if it's uncommented - it is not assigned anywhere

It is either regression or it is not... It should be clearly defined what it is and it should have an opened tracking issue.

Why do you have this line? Even if it's uncommented - it is not assigned anywhere

It was for troubleshooting purpose , now i have removed it .

It is either regression or it is not... It should be clearly defined what it is and it should have an opened tracking issue.

Viktor has already drop a mail to mark regarding this , but till now we did not get any confirmation/reply , so as per his suggestion we have made it xfail , we will create tracking issue after mark's confirmation . till then we have to merge it as xfail as we have to put more test cases on the top of the same module , which are from same TET test script and uses same entries and test functions .

Sure. Then let's wait for @mreynolds reply, put the info about the issue and then merge. :)

I get this error trying to run the script:

__________________________________________ ERROR collecting tests/suites/automember_plugin/autoMembers_from_tet_test.py ___________________________________________
/usr/lib/python2.7/site-packages/_pytest/python.py:448: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
/usr/lib/python2.7/site-packages/py/_path/local.py:668: in pyimport
    __import__(modname)
E     File "/home/mareynol/source/ds389/389-ds-base/dirsrvtests/tests/suites/automember_plugin/autoMembers_from_tet_test.py", line 46
E       'cn': f'{CN_CONT}'
E                        ^
E   SyntaxError: invalid syntax
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

python2.7

That's a very old version. f-strings are part of python 3.6.

I also had to switch around the imports to look like:

from lib389.idm.group import Groups, Group, UniqueGroups, UniqueGroup, nsAdminGroups
from lib389.idm.user import UserAccounts, UserAccount

nsAdminGroups was attempted to be imported from idm..user (not idm.group)

I don't see import from idm.user.
In the patch:

+ from lib389.idm.group import Groups, Group, UniqueGroup, nsAdminGroups, nsAdminGroup

I have a machine with the reproducer if you want to take a look.

Well I found part of the problem. The test is not updating the correct automember config entry:

AutoMembershipPlugin(topo.ms["master1"]).add("nsslapd-pluginConfigArea",
"cn=autoMembersPlugin,{}".format(BASE_REPL))

ThIs is:

nsslapd-pluginConfigArea: cn=autoMembersPlugin,dc=replAutoMembers,dc=com

But there is no automember config in this entry.

dn: cn=autoMembersPlugin,dc=replAutoMembers,dc=com
objectClass: top
objectClass: nscontainer
cn: autoMembersPlugin
creatorsName: cn=directory manager
modifiersName: cn=directory manager
createTimestamp: 20190830175354Z
modifyTimestamp: 20190830175354Z
nsUniqueId: 211e4f55-cb4f11e9-9e39a34b-b2b909d7

Looks like it created another config entry under a different suffix, so it's basically being ignored.

Sure. Then let's wait for @mreynolds reply, put the info about the issue and then merge. :)

we have got marks's comment here and after discussing with viktor , we have modified the failed test case as per the new behaviorism of DS auto member plugin .

As this test case was old so it was passing on older version of DS, after after modification done on DS by mark and as per the new behaviorism of DS auto member plugin we have changed the test case , and now its passing on newer version of DS.

LGTM, 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/3576

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.