We need the tests to be discoverable... Could you please rename the test function to meaningful.

Also, please, move the test case to somewhere like - dirsrvtests/tests/suites/memberof_plugin/regression_test.py

We need the docstrings... I see you already have them as comments. Could you please put it here?
Also, for :expectedresults: you can put something simple like 1. Success.

It can be removed

Maybe @vashirov will have more comments regarding the test case. But it looks okay to me.

Also, C code looks good to me too. But I have a question because I am not very familiar with the part of the code base.
I see that you changed the behaviour a bit.

Before - in the memberof_moddn_attr_list - we go through all of the values and do memberof_modop_one_replace_r on each of them even though some of them can fail.

Now - in the memberof_moddn_attr_list - we go through the values and do memberof_modop_one_replace_r on each of them but we stop when there is a failure. So it is possible that some of the values were replaced succeffully and some of them not because we stopped in the middle of the list.
(and then we report that the whole operation is failed - https://pagure.io/fork/tbordaz/389-ds-base/blob/ticket_49985/f/ldap/servers/plugins/memberof/memberof.c#_923 )

Should be mention that a part of the list was successfully replaced or is it okay like this?
Wouldn't it affect something else in the plugin fuctionality?

Okay, so I don't really agree with the fix or the test because this function here should fail. The issue is that our schema check only checks that the auto_oc_memberof is a valid objectclass, not that it's a valid objectClass that can take memberOf as the attribute. So I would rather have the test attempt this change and fail here, than mask the failure with partial operation.

I think that this would be much better from an operator perspective, because we make the service "impossible" to misconfigure, rather than this which tolerates the misconfiguration.

To put another way: I would like to see this fix approached differently than it currently is. If this is urgent, then "ack" to this change, but I would rather have it fixed in the manner I have referred to above. Thanks,

I think there are two issues.

The first one (this ticket) is that, during a MODRDN, if memberof fails to update a member it does not trigger the failure of the MODRDN.
Here I trigger the failure of the update of a member with an invalid objectclass but it is just to create a reproducible test case. In production what can happen is a db_deadlock, db_out_of_lock,... that is much more difficult to recreate with a test case.

The second issue is that auto_oc_memberof can contain a useless objectclass. Should it be detected during the configuration, during the update or simply not detected (Note that the failure is logged in error log), I have no strong opinion.
Just I think the safest would be to detect it during update but I am fear of performance impact when we have many members to update.

@tbordaz We only need to validate the objectClass as valid when we change the memberof configuration, not on every operation though.

update: ack to the fix because it resolves the first case, and can we open a ticket for validation of the oc content on configuration change and load? That sounds like a good approach to resolve both, but it will mean we have to find a different way to test this scenario in the lib389 tests.

@firstyear I agree with your comment https://pagure.io/389-ds-base/issue/49967#comment-535339. It is precious to have a way to trigger plugin failure.
So I have a doubt about checking that configured OC allows 'memberof' as it will remove an easy way for us to trigger plugin failures.
An option would be to let admin configure invalid OC but when the failure occurs to log a more helpful message how to resolve it.

Thanks for the ACK. I will update the testcase following @spichugi recommendations

Thanks @firstyear for the ACK. merging the patch.
I move the testcase to the member regression suite

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

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.