PR - Issue: 50860 - Port Password Policy test cases from TET to python3 part1

[389-ds-bot]

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

• Created at 2020-01-28 07:55:13 by aborah (@aborah-sudo)
• Merged at 2020-05-28 09:55:03

---

Bug Description: Port Password Policy test cases from TET to python3 part1

Relates/Fixes: Resolves: #3914

Author: aborah-sudo

Reviewed by: ???

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-03-23 19:41:37

Why not set the properties directly here? Why do you set them later?

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-03-24 10:21:51

The test docstring should reflect the feature of some component it tests.
It is hard to understand which situation we test here...

Please, don't mention the placeholder names in the docstrings

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-03-24 10:24:18

It should raise INVALID_CREDENTIALS, it shouldn't be a success.
Please, check all your docstrings for the related issues (the same applies to other lines I report).

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-04-06 13:02:42

1 new commit added

• Fixing Simon's comments and test cases

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-04-06 13:04:06

Why not set the properties directly here? Why do you set them later?

Changed

The test docstring should reflect the feature of some component it tests.
It is hard to understand which situation we test here...
Please, don't mention the placeholder names in the docstrings

Changed

It should raise INVALID_CREDENTIALS, it shouldn't be a success.
Please, check all your docstrings for the related issues (the same applies to other lines I report).

Changed

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-04-06 13:08:22

2 new commits added

• Fixing Simon's comments and test cases
• Issue: 50860 - Port Password Policy test cases from TET to python3 part1

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-22 01:32:11

Could you please elaborate on why have you merged all of the test cases into one? (I didn't ask for that for sure...)

It is better to have separate test cases that test different features in isolated environments.

If your case, if one line in the test fails - the whole test will fail. So it may mask more issues and it will make the debugging harder.
Because of this, it is better to have separate test cases for different small features.

[389-ds-bot]

Comment from vashirov (@vashirov) at 2020-04-29 12:11:20

The original test in TET was split into multiple functions, but they were not isolated. Each of the ~100 functions was part of the same test. So it was not a lot of sense to port them as they were. I asked Anuj to have a single test around the tested feature.

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-29 12:37:54

The original test in TET was split into multiple functions, but they were not isolated. Each of the ~100 functions was part of the same test. So it was not a lot of sense to port them as they were. I asked Anuj to have a single test around the tested feature.

It is understandable and I am okay with it.

The main issue that I have is that it is very hard to follow what actually is tested.

The main feature is about 'passwordchange', okay.
And then in the test case, there are no explanations about the cases we test.
Instead, it has only things like changing the current password for joe and fed - success or fail.
So I need to spend much more time to figure out what this is about... Why would Joe fail? Why would orly2 success? etc.

So we either need descriptive comments or we can isolate the cases into smaller chunks of test cases and naming them properly so it's easier to follow the idea behind the test.

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-04-29 12:40:38

The original test in TET was split into multiple functions, but they were not isolated. Each of the ~100 functions was part of the same test. So it was not a lot of sense to port them as they were. I asked Anuj to have a single test around the tested feature.

It is understandable and I am okay with it.
The main issue that I have is that it is very hard to follow what actually is tested.
The main feature is about 'passwordchange', okay.
And then in the test case, there are no explanations about the cases we test.
Instead, it has only things like changing the current password for joe and fed - success or fail.
So I need to spend much more time to figure out what this is about... Why would Joe fail? Why would orly2 success? etc.
So we either need descriptive comments or we can isolate the cases into smaller chunks of test cases and naming them properly so it's easier to follow the idea behind the test.

I will take care of that . Will update the dock string

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-04-29 14:16:30

2 new commits added

• Fixing Simon's comments and test cases
• Issue: 50860 - Port Password Policy test cases from TET to python3 part1

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-04-29 14:17:59

The original test in TET was split into multiple functions, but they were not isolated. Each of the ~100 functions was part of the same test. So it was not a lot of sense to port them as they were. I asked Anuj to have a single test around the tested feature.

It is understandable and I am okay with it.
The main issue that I have is that it is very hard to follow what actually is tested.
The main feature is about 'passwordchange', okay.
And then in the test case, there are no explanations about the cases we test.
Instead, it has only things like changing the current password for joe and fed - success or fail.
So I need to spend much more time to figure out what this is about... Why would Joe fail? Why would orly2 success? etc.
So we either need descriptive comments or we can isolate the cases into smaller chunks of test cases and naming them properly so it's easier to follow the idea behind the test.

Doc string changed

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-29 15:55:31

Both IDs are equal to each other

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-29 15:56:11

The test case fails for me... ldap.INSUFFICIENT_ACCESS: {'desc': 'Insufficient access', 'info': "Insufficient 'write' privilege to the 'userPassword' attribute of entry 'uid=dbyers,ou=dirsec,dc=example,dc=com'.\n"}

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-04-29 16:24:06

The test case fails for me... ldap.INSUFFICIENT_ACCESS: {'desc': 'Insufficient access', 'info': "Insufficient 'write' privilege to the 'userPassword' attribute of entry 'uid=dbyers,ou=dirsec,dc=example,dc=com'.\n"}

both test case just passed for me . bellow are rps.

[root@localhost Documents]# pytest-3 password_policy_test.py
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.8.2, pytest-4.6.9, py-1.8.0, pluggy-0.13.0
rootdir: /home/aborah-sudo/Documents
collected 2 items

password_policy_test.py .. [100%]

================================================================================================ warnings summary =================================================================================================

[root@localhost Documents]# rpm -qa | grep 389
389-ds-base-snmp-1.4.3.5-1.fc32.x86_64
python3-lib389-1.4.3.5-1.fc32.noarch
389-ds-base-1.4.3.5-1.fc32.x86_64
389-ds-base-libs-1.4.3.5-1.fc32.x86_64
cockpit-389-ds-1.4.3.5-1.fc32.noarch
389-ds-base-legacy-tools-1.4.3.5-1.fc32.x86_64
[root@localhost Documents]#

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-05-11 14:53:40

"Both IDs are equal to each other" is still not fixed.

The rest looks good to me.

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-05-11 15:14:12

rebased onto 6235ee7e7010ade5545092ea6ac751181907184a

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-05-11 15:16:17

rebased onto 16a3da4bd981eee16b86b12d8179cd0aa565f6a6

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-05-11 15:21:24

rebased onto b87c13fd07e789627d525271058937f27a790071

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-05-22 15:05:41

rebased onto 675dc14304b8c3142557b5ce18383b67cae905bc

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2020-05-28 06:44:48

rebased onto 1befe92

[389-ds-bot]

Comment from vashirov (@vashirov) at 2020-05-28 09:55:03

Pull-Request has been merged by vashirov

[389-ds-bot]

Patch
50861.patch