New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CI test - password #1412
Comments
Comment from nhosoi (@nhosoi) at 2017-02-11 23:09:56 Metadata Update from @nhosoi:
|
Comment from sramling at 2017-08-04 15:23:20 Adding pytest test for bugzilla - https://bugzilla.redhat.com/show_bug.cgi?id=1465600 |
Comment from sramling at 2017-08-04 15:23:20 Metadata Update from @sramling:
|
Comment from sramling at 2017-08-04 15:23:30 Metadata Update from @sramling:
|
Comment from spichugi (@droideck) at 2017-08-04 15:42:37 I think, it is a bit unclear. |
Comment from spichugi (@droideck) at 2017-08-08 10:19:54 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):
Like this:
And you have a lot of methods to work with dict:
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. |
Comment from sramling at 2017-08-08 12:29:39 Thanks @droideck for your valuable comments. Adding a new patch with suggested changes. |
Comment from sramling at 2017-08-08 19:08:05 Adding new patch |
Comment from spichugi (@droideck) at 2017-08-09 10:46:51 {} 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.
In the passw_policy() fixture, you enable subtreePwdPolicy but don't disable it in the finalizer.
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:
And later, you dont need this:
Just work with 'test_user' fixture object.
Don't you need to change password back in the 'finally:' block in case if something will go wrong? |
Comment from sramling at 2017-08-09 15:12:33 Review comments incorporated in the new patch. |
Comment from spichugi (@droideck) at 2017-08-09 16:51:22
Okay, one thing more. After a discussion on IRC, we've decided to add markings with pytest.
|
Comment from sramling at 2017-08-09 18:46:14 Added pytest.mark |
Comment from spichugi (@droideck) at 2017-08-09 19:17:50 Metadata Update from @droideck:
|
Comment from spichugi (@droideck) at 2017-08-09 19:19:02 To ssh://pagure.io/389-ds-base.git |
Comment from sramling at 2017-08-14 13:23:10 Adding tests for OU and Givenname attribute. |
Comment from sramling at 2017-08-14 13:26:04 Metadata Update from @sramling:
|
Comment from sramling at 2017-08-16 16:33:46 @droideck @Firstyear , can you review the patch? I added couple of new inputs for testing OU and Givenname attributes. |
Comment from spichugi (@droideck) at 2017-08-16 18:33:33 There is a commented fixture you've missed:
Besides that, LGTM. Thanks. :) |
Comment from sramling at 2017-08-17 08:01:12 Removed the comment, thanks @droideck |
Comment from spichugi (@droideck) at 2017-08-17 09:36:10 commit da4ebf7 |
Comment from spichugi (@droideck) at 2017-08-17 09:36:25 Metadata Update from @droideck:
|
Comment from vashirov (@vashirov) at 2017-09-12 15:35:47 |
Comment from mreynolds (@mreynolds389) at 2017-09-12 16:03:32 Thanks, ack |
Comment from vashirov (@vashirov) at 2017-09-12 17:25:57 To ssh://pagure.io/389-ds-base.git |
Comment from mreynolds (@mreynolds389) at 2019-08-23 21:43:35 Metadata Update from @mreynolds389:
|
Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/48081
#No Description Provided
The text was updated successfully, but these errors were encountered: