Metadata Update from @nhosoi: - Issue set to the milestone: CI test 1.0
Adding pytest test for bugzilla - https://bugzilla.redhat.com/show_bug.cgi?id=1465600
<img alt="0001-Password-accepts-sn-attr.patch" src="/389-ds-base/issue/raw/files/38ccf56b0efaa7f9f38bdaa532e036f7f3fd92acb6e3e7a34eb09f7ca07b870e-0001-Password-accepts-sn-attr.patch" />
Metadata Update from @sramling: - Custom field reviewstatus adjusted to None - Issue close_status updated to: None
Metadata Update from @sramling: - Custom field reviewstatus adjusted to review (was: None)
I think, it is a bit unclear. We have https://bugzilla.redhat.com/show_bug.cgi?id=1465600 and the bugzilla will have it's issue in Pagure. As I see, we need to create a ticket in pagure for the bug and upload the test ticket there. Sorry, that I didn't mention it at the previous ticket.
Ok, for the review. We can do it here and mention the commit later in the new issue.
I think it will be more efficient to create one data structure insted of this (btw, {} is for dict, not for list or tuple):
TEST_USER = 'UIDpwtest1' CN_ATTR = 'CNpwtest1' SN_ATTR = 'SNpwtest1' UID_ATTR = TEST_USER attrs = {CN_ATTR, SN_ATTR, UID_ATTR}
Like this:
user_data = {'cn': 'CNpwtest1', 'sn': 'SNpwtest1', 'uid': 'UIDpwtest1'}
And you have a lot of methods to work with dict:
user_data.items() user_data.keys() user_data.values() user_properties.update(user_data) # etc.
The next thing. I think it is better to parametrize the test case. Because it fails now and the failure stops the execution during the 'for loop'. So we don't see all results but only one for the first failure.
Thanks @spichugi for your valuable comments. Adding a new patch with suggested changes.
<img alt="0001-CI-tests-PasswordCheckSyntax.patch" src="/389-ds-base/issue/raw/files/f42429499528aa390d9bbaa02ed82eae90e693866a6456beaa04b4cd0eb9911c-0001-CI-tests-PasswordCheckSyntax.patch" />
Adding new patch
<img alt="0001-CI-tests-PasswordCheckSyntax.patch" src="/389-ds-base/issue/raw/files/2411b2d29d9ffe07a512347365b1085f8743634aa431a8fb663a3151a7429dae-0001-CI-tests-PasswordCheckSyntax.patch" />
{} is for dicts, not for lists or tuples. Also, please, use more explicit variable names. Like 'TEST_PASSWORDS' for lists and 'TEST_PASSWORD' for single password.
TEST_PASW = {'CN12pwtest31', 'SN3pwtest231', 'UID1pwtest123', 'MAIL2pwtest12@redhat.com'}
In the passw_policy() fixture, you enable subtreePwdPolicy but don't disable it in the finalizer.
topo.standalone.subtreePwdPolicy(subtree, {'passwordchange': 'on', 'passwordCheckSyntax': 'on'})
In the test_users() fixture, you can just use variable from outer scope. Also, it is only one 'test_user', not 'test_users'. And you can rerurn the 'tuser'. Like this:
log.info('Adding user-uid={},ou=people,{}'.format(user_data['uid'], SUFFIX)) users = UserAccounts(topo.standalone, SUFFIX) user_properties = { 'uidNumber': '1001', 'gidNumber': '2001', 'userpassword': USER_PASW, 'homeDirectory': '/home/pwtest1'} user_properties.update(user_data) tuser = users.create(properties=user_properties) def fin(): log.info('Deleting user-uid={},ou=people,{}'.format(user_data['uid'], SUFFIX)) tuser.delete() request.addfinalizer(fin) return tuser
And later, you dont need this:
userdn = 'uid={},ou=people,{}'.format(user_data['uid'], SUFFIX) users = UserAccounts(topo.standalone, SUFFIX) tuser = users.get(user_data['uid'])
Just work with 'test_user' fixture object.
conn = test_user.bind(USER_PASW)
Don't you need to change password back in the 'finally:' block in case if something will go wrong?
Review comments incorporated in the new patch.
<img alt="0001-CI-tests-PasswordCheckSyntax.patch" src="/389-ds-base/issue/raw/files/82dd750a42804f0d955c8f0bcb1c864c81f9d5f642ac955ea9b9b36c3fdc1d77-0001-CI-tests-PasswordCheckSyntax.patch" />
Review comments incorporated in the new patch. Okay, one thing more. After a discussion on IRC, we've decided to add markings with pytest. So please, add @pytest.mark.bz1465600 and @pytest.mark.bz1468284 like this:
Okay, one thing more. After a discussion on IRC, we've decided to add markings with pytest. So please, add @pytest.mark.bz1465600 and @pytest.mark.bz1468284 like this:
@pytest.mark.bz1468284 @pytest.mark.parametrize("user_pasw", TEST_PASSWORDS) def test_cn_sn_like_passw(topo, passw_policy, test_user, user_pasw):
Added pytest.mark
<img alt="0001-CI-tests-PasswordCheckSyntax.patch" src="/389-ds-base/issue/raw/files/c37a1430a6be2e4e060e047e9893b5cf054d1e7d12da6dc6a738e671e8d37c1f-0001-CI-tests-PasswordCheckSyntax.patch" />
Metadata Update from @spichugi: - Custom field reviewstatus adjusted to ack (was: review)
To ssh://pagure.io/389-ds-base.git e710054..05b00f2 master -> master commit 05b00f2 Author: Sankar Ramalingam sramling@redhat.com Date: Wed Aug 9 22:10:46 2017 +0530
Adding tests for OU and Givenname attribute.
<img alt="0001-CI-tests-PasswordCheckSyntax-ou-givenname.patch" src="/389-ds-base/issue/raw/files/4a3379af00a39ca1d2ed42411d983f77c3c61d027e48b91152786773949518b9-0001-CI-tests-PasswordCheckSyntax-ou-givenname.patch" />
Metadata Update from @sramling: - Custom field reviewstatus adjusted to review (was: ack)
@spichugi @firstyear , can you review the patch? I added couple of new inputs for testing OU and Givenname attributes.
There is a commented fixture you've missed:
40 +# @pytest.mark.parametrize("user_pasw", user_data.values())
Besides that, LGTM. Thanks. :)
Removed the comment, thanks @spichugi
<img alt="0001-CI-tests-PasswordCheckSyntax-ou-givenname.patch" src="/389-ds-base/issue/raw/files/cc8202c709e516d9a77ab41545ad947179cf75877b499f277ac5f26e785ba3a6-0001-CI-tests-PasswordCheckSyntax-ou-givenname.patch" />
commit da4ebf7 Author: Sankar Ramalingam sramling@redhat.com Date: Thu Aug 17 11:26:56 2017 +0530
Adding another test case for pwdPolicy_warning_test <img alt="0001-Issue-48081-CI-test-password.patch" src="/389-ds-base/issue/raw/files/fbee362e98a949d6366cafc66b26a198d024b2f798bf8b70cc7e4a8507bbf94f-0001-Issue-48081-CI-test-password.patch" />
Thanks, ack
To ssh://pagure.io/389-ds-base.git 5c2fa96..fe1cfca master -> master
Metadata Update from @mreynolds: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
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/1412
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: fixed)
Login to comment on this ticket.