attachment
0001-Ticket-49072-validate-memberof-fixup-task-args.patch

Looks good.

If it is a customer reported bug, do we have a corresponding bug #?

64b1ebf..a79ae70 master -> master
commit a79ae70
Author: Mark Reynolds mreynolds@redhat.com
Date: Tue Dec 20 14:59:02 2016 -0500

8ab4be5..b0020b7 389-ds-base-1.3.5 -> 389-ds-base-1.3.5
commit b0020b7

d2f46f5..f708c7f 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit f708c7f

7211e65..8efcc70 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 8efcc70

Fix log refactoring mistake:

b0020b7..f55caec 389-ds-base-1.3.5 -> 389-ds-base-1.3.5
commit f55caec
Author: Mark Reynolds mreynolds@redhat.com
Date: Thu Dec 22 13:37:25 2016 -0500

f708c7f..ffaf3aa 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit ffaf3aa

8efcc70..d16e6a6 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit d16e6a6

More refactoring misses:

f55caec..460c578 389-ds-base-1.3.5 -> 389-ds-base-1.3.5
commit 460c578

ffaf3aa..262f095 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 262f095

d16e6a6..f850128 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit f850128

fix 1.2.11 backport issue
0001-Ticket-49072-memberof-fixup-is-not-validating-base-d.patch

Looks good.

55aa091..a87ddab 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit a87ddab
Author: Mark Reynolds mreynolds@redhat.com
Date: Fri Jan 13 11:08:18 2017 -0500

Automated test cases in upstream. Please review.

Hi Sankar,
very good test case. Some of it we can make better though :)

+# Copyright (C) 2016 Red Hat, Inc. - it is 2017.

We need to put a feature name to :feature:, if I understand right. So it need to be something like 'memberOf'. And we have test suite for the feature, so, probably, you can move the test cases to the suites/plugins/memberof_test.py.
And "run fixup-memberof.pl" can't be part of the :setup:, it is from :steps:

:feature: To verify Bug #1406838, ticket #49072
:setup: Standalone instance, with memberOf plugin, run fixup-memberof.pl

This 'if' doesn't make sense, because 'True' is a constant. You can just remove it. If the test will fail on a previous line, it will cause the test to stop and other lines won't be executed.

assert expected in output
if True:
    log_entry = topo.standalone.ds_error_log.match('.*Failed to get be backend.*')
    log.info('Error log out: {}'.format(log_entry))
    assert topo.standalone.ds_error_log.match('.*Failed to get be backend.*')

Please, use 'os.path.join' instead of '{}/{}'.format.

memof_task = '{}/{}'.format(topo.standalone.ds_paths.inst_dir, FIXUP_MEMOF)

Thank you!

Thanks Simon for your comments. I have incorporated all the changes as detailed.

Looks good to me, really happy with how you are using the paths and log searching there. Makes the test really easy to follow.

I dont see the patch... It shows "Page not found (404)", I guess something happened with Pagure.
@sramling could you please reupload the patch? I think it is okay, if William says so, so I will just check and will push it to the master.

Attaching the patch

@spichugi , I uploaded the patch now

To ssh://pagure.io/389-ds-base.git
f39cc62..ca3fef5 master -> master
commit ca3fef5
Author: Sankar Ramalingam sramling@redhat.com
Date: Tue May 23 20:45:11 2017 +0530

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 issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2131

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.