PR - Issue:50112 - Port ACI test suit from TET to python3(Delete and Add)

[389-ds-bot]

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50319

• Created at 2019-04-03 13:08:50 by aborah (@aborah-sudo)
• Merged at 2019-04-16 10:47:22

---

Port ACI test suit from TET to python3(Delete and Add)

Resolves: #3171

Reviewed by: ???

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-04-04 02:45:42

use with pytest.raises instead of try except

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-04 04:30:06

rebased onto 4d4c74a687b87c75d5a0de1b1d73112dfc5b0537

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-04 04:35:41

@Firstyear , changes are done , Please check

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-04 15:33:26

Why do you add ou: People attribute for the users?

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-04 15:36:25

Why do you add ou: People attribute for the users?

There is a dynamic group which has :

group.add("memberURL", f'ldap:///{DEFAULT_SUFFIX}??sub?(&(ou=People)(cn=test_user_1000))')

as by default if you create a user with user = users.create_test_user(uid=i, gid=i)

the dn will be 'uid=test_user_1000,ou=People,dc=example,dc=com' but no ou: 'People'

but dynamic group need ou: 'People '

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-12 11:16:58

Why do you add ou: People attribute for the users?

There is a dynamic group which has :
group.add("memberURL", f'ldap:///{DEFAULT_SUFFIX}??sub?(&(ou=People)(cn=test_user_1000))')
as by default if you create a user with user = users.create_test_user(uid=i, gid=i)
the dn will be 'uid=test_user_1000,ou=People,dc=example,dc=com' but no ou: 'People'
but dynamic group need ou: 'People '

It doesn't make sense... We don't create users with ou attribute. If you want to setup memberURL with the specific filter - use something like &(objectclass=person)(cn=test_user_1000))

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-12 11:20:16

You can use get() method here. First, create UserAccounts instance (without this rdn). And then get the user by selector (test_user_1000)

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-12 16:24:02

rebased onto b3772c5580fe5c9ebaa120a4edd2af7bdc074b74

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-12 16:25:15

@droideck , changes are done , Please check

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-12 16:26:27

rebased onto 1cfd7584b2757af19eadbb3ed5e40537abbd6c43

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-12 16:30:03

It shouldn't be a docstring. It should be a commented text.

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-12 16:33:07

Please, go through --pylint output. There are a few issues left

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-12 16:35:22

You already have users object. Why not to use it here?

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-12 16:58:19

I think it will make more sense if you create a user (and assign it to user for delete operation). And then you can assert if the user is present (user.exists())

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-12 17:25:29

I think it will make more sense if you create a user (and assign it to user for delete operation). And then you can assert if the user is present (user.exists())

You cant do it as you only have add privilege to parents .

And UserAccounts(conn, DEFAULT_SUFFIX, rdn='uid=test_user_1000, ou=people') and UserAccounts(topo.standalone, DEFAULT_SUFFIX).get('test_user_1').delete() are different

see conn and topo.standalone

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-12 17:26:45

rebased onto d826d53052f08e9dc81f193eda05b703717048f0

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-12 17:27:31

@droideck , changes are done , Please check

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-12 21:29:01

You cant do it as you only have add privilege to parents .
And UserAccounts(conn, DEFAULT_SUFFIX, rdn='uid=test_user_1000, ou=people') and UserAccounts(topo.standalone, DEFAULT_SUFFIX).get('test_user_1').delete() are different

Right. We can't use the same UserAccount instance for the delete operation.
But the rest in my comment is true. assert doesn't make sense here because if the user creation will fail it will through the exception anyway.
We should assert that the user exists().

see conn and topo.standalone !!

The imperative mood plus these two exclamation marks (after a space) can be misunderstood as a rude gesture (it sounds a bit like you are yelling at me).
Please, avoid such expressions in the project.

P.S. I understand that it wasn't, probably, your intention. But to be sure, please, use polite language forms. :)

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-13 04:47:58

rebased onto b673a77540f5218f5c402258c0e5ca8d39d6c4ba

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-13 04:48:42

rebased onto a6a53483afa7e421a693cab73299bbaf05b8b303

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-13 04:50:17

You cant do it as you only have add privilege to parents .
And UserAccounts(conn, DEFAULT_SUFFIX, rdn='uid=test_user_1000, ou=people') and UserAccounts(topo.standalone, DEFAULT_SUFFIX).get('test_user_1').delete() are different

Right. We can't use the same UserAccount instance for the delete operation.
But the rest in my comment is true. assert doesn't make sense here because if the user creation will fail it will through the exception anyway.
We should assert that the user exists().

see conn and topo.standalone !!

The imperative mood plus these two exclamation marks (after a space) can be misunderstood as a rude gesture (it sounds a bit like you are yelling at me).
Please, avoid such expressions in the project.
P.S. I understand that it wasn't, probably, your intention. But to be sure, please, use polite language forms. :)

Obviously it was not my intention , will keep in mind in future

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-13 04:50:22

@droideck , changes are done , Please check

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-15 17:35:15

LGTM! Thanks!
Ack

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-04-16 10:30:48

rebased onto af97382

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-04-16 10:47:23

Pull-Request has been merged by droideck

[389-ds-bot]

Patch
50319.patch