#49728 Issue 49588 - Add py3 support for tickets : part-2
Closed by spichugi. Opened by aadhikari.
aadhikari/389-ds-base python3-support-ticket-2  into  master

Download 49728.patch

Description: Added py3 support by explicitly changing strings to bytes.
Ported tests from ticket to test suites, also added docstrings.

https://pagure.io/389-ds-base/issue/49588

Reviewed by: ??

When we initialize an environment for some particular test case, we should put it as a fixture,
because we should make sure it will be run before the test case and cleaned after the test case (it should not mess the environment)

You missed one step:
"# entry to add WITH memberS and WITH the ACI -> ldap.INSUFFICIENT_ACCESS
# member should contain only one value"

It should be two steps, I think

It is not descriptive enough. Your tests are about SelfDN aci.
First, you should rename the file to something like "selfdn_permissions_test.py".
And each first line should be rewritten to something related.
"It checks that, bound as bind_entry" -> "Check add entry operation with and without SelfDN aci"

Once again, it should be fixture

You can add the tests to the existing test suite. I don't think we should split it into different places.
https://pagure.io/389-ds-base/blob/master/f/dirsrvtests/tests/suites/replication/changelog_test.py

basicaly in this test case you check for valid and invalid values.
I think you can have just two steps. One will have all valid (mention each of them) and another invalid (negative and just chars)

Should be fixture

I think it is better to mention the attributes you set here.

It is for userpassword. nsslapd-pluginarg2: can have any attribute.

The same for the rest test cases here

Should be fixture

Add some verb here. Like 'check' or something

rebased onto 48373ff34f78005d9ca791453d7d954cc5ff23fa

@spichugi Hey mate! have made the changes mentioned above, have a look.

We should mention all of the setup fixtures here. So if you add some 'init' fixture, describe it shortly here.

rebased onto 60339ce804b74408fc2de0b4289a62fff5b79903

@spichugi it's done!

Pull-Request has been merged by spichugi

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

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

Metadata