PR#50192: Issue:48851 - investigate and port TET matching rules filter tests - 389-ds-base

389-ds-base

#50192 Issue:48851 - investigate and port TET matching rules filter tests

Closed 3 years ago by spichugi. Opened 5 years ago by aborah.

aborah/389-ds-base filter_part1 into master

Issue:48851 - investigate and port TET matching rules filter tests

Anuj Borah • 4 years ago

d0da028

dirsrvtests/tests/suites/filter/bitw_filter_test.py

file added

+396

		`@@ -0,0 +1,396 @@`
		`+ # --- BEGIN COPYRIGHT BLOCK ---`
		`+ # Copyright (C) 2019 Red Hat, Inc.`
		`+ # All rights reserved.`
		`+ #`
		`+ # License: GPL (version 3 or any later version).`
		`+ # See LICENSE for details.`
		`+ # --- END COPYRIGHT BLOCK ---`
		`+`
		`+ """`
		`+ This script will test different type of Filers.`
		`+ """`
		`+`
		`+ import os`
		`+ import pytest`
		`+`
		`+ from lib389.topologies import topology_st as topo`
		`+ from lib389._constants import PW_DM`
		`+ from lib389.idm.user import UserAccounts`
		`+ from lib389.idm.account import Accounts`
		`+ from lib389.plugins import BitwisePlugin`
		`+ from lib389.schema import Schema`
		`+ from lib389.backend import Backends`
		`+ from lib389.idm.domain import Domain`
		`+`
		`+ import ldap`
		`+`
		`+ FILTER_TESTPERSON = "objectclass=testperson"`
		`+ FILTER_TESTERPERSON = "objectclass=testerperson"`
		`+ FILTER_CONTROL = f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=514))"`
		`+ SUFFIX = 'dc=anuj,dc=com'`
		`+`
		`+`
		`+ class CreateUsers():`
		`+ """`
		`+ Will create users with different testUserAccountControl, testUserStatus`
		`+ """`
		`+ def __init__(self, *args):`
		`+ self.args = args`
		`+`
		`+ def user_create(self):`
		`+ """`
		`+ Will create users with different testUserAccountControl, testUserStatus`
		`+ """`
		`+ self.args[0].create(properties={`
		`+ 'sn': self.args[1],`
		`+ 'uid': self.args[1],`
		`+ 'cn': self.args[1],`
		`+ 'userpassword': PW_DM,`
		`+ 'givenName': 'bit',`
		`+ 'mail': '{}@redhat.com'.format(self.args[1]),`
		`+ 'objectclass': 'top account posixaccount organizationalPerson '`
		`+ 'inetOrgPerson testperson'.split(),`
		`+ 'testUserAccountControl': [i for i in self.args[2]],`
		`+ 'testUserStatus': [i for i in self.args[3]],`
		`+ 'uidNumber': str(self.args[4]),`
		`+ 'gidNumber': str(self.args[4]),`
		`+ 'homeDirectory': self.args[1]`
		`+ })`
		`+`
		`+ def create_users_other(self):`
		`+ """`
		`+ Will create users with different testUserAccountControl(8388608)`
		`+ """`
		`+ self.args[0].create(properties={`
		`+ 'telephoneNumber': '98989819{}'.format(self.args[1]),`
		`+ 'uid': 'anuj_{}'.format(self.args[1]),`
		`+ 'sn': 'testwise_{}'.format(self.args[1]),`
		`+ 'cn': 'bit testwise{}'.format(self.args[1]),`
		`+ 'userpassword': PW_DM,`
		`+ 'givenName': 'anuj_{}'.format(self.args[1]),`
		`+ 'mail': 'anuj_{}@example.com'.format(self.args[1]),`
		`+ 'objectclass': 'top account posixaccount organizationalPerson '`
		`+ 'inetOrgPerson testperson'.split(),`
		`+ 'testUserAccountControl': '8388608',`
		`+ 'testUserStatus': 'PasswordExpired',`
		`+ 'uidNumber': str(self.args[1]),`
		`+ 'gidNumber': str(self.args[1]),`
		`+ 'homeDirectory': '/home/' + 'testwise_{}'.format(self.args[1])`
		`+ })`
		`+`
		`+ def user_create_52(self):`
		`+ """`
		`+ Will create users with different testUserAccountControl(16777216)`
		`+ """`
		`+ self.args[0].create(properties={`
		`+ 'telephoneNumber': '98989819{}'.format(self.args[1]),`
		`+ 'uid': 'bditwfilter52_test{}'.format(self.args[1]),`
		`+ 'sn': 'bditwfilter52_test{}'.format(self.args[1]),`
		`+ 'cn': 'bit bditwfilter52_test{}'.format(self.args[1]),`
		`+ 'userpassword': PW_DM,`
		`+ 'givenName': 'bditwfilter52_test{}'.format(self.args[1]),`
		`+ 'mail': 'bditwfilter52_test{}@example.com'.format(self.args[1]),`
		`+ 'objectclass': 'top account posixaccount organizationalPerson '`
		`+ 'inetOrgPerson testperson'.split(),`
		`+ 'testUserAccountControl': '16777216',`
		`+ 'testUserStatus': 'PasswordExpired',`
		`+ 'uidNumber': str(self.args[1]),`
		`+ 'gidNumber': str(self.args[1]),`
		`+ 'homeDirectory': '/home/' + 'bditwfilter52_test{}'.format(self.args[1])`
		`+ })`
		`+`
		`+`
		`+ @pytest.fixture(scope="module")`
		`+ def _create_schema(request, topo):`
		`+ Schema(topo.standalone).\`
		`+ add('attributetypes',`
		`+ ["( NAME 'testUserAccountControl' DESC 'Attribute Bitwise filteri-Multi-Valued'"`
		`+ "SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 )",`
		`+ "( NAME 'testUserStatus' DESC 'State of User account active/disabled'"`
		`+ "SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )"])`
		`+`
		`+ Schema(topo.standalone).\`
		`+ add('objectClasses', "( NAME 'testperson' SUP top STRUCTURAL MUST "`
		`+ "( sn $ cn $ testUserAccountControl $ "`
		`+ "testUserStatus )MAY( userPassword $ telephoneNumber $ "`
		`+ "seeAlso $ description ) X-ORIGIN 'BitWise' )")`
		`+`
		`+ # Creating Backend`
		`+ backends = Backends(topo.standalone)`
		`+ backend = backends.create(properties={'nsslapd-suffix': SUFFIX, 'cn': 'AnujRoot'})`
		`+`
		`+ # Creating suffix`
		`+ suffix = Domain(topo.standalone, SUFFIX).create(properties={'dc': 'anuj'})`
		`+`
		`+ # Creating users`
		`+ users = UserAccounts(topo.standalone, suffix.dn, rdn=None)`
		`+ for user in [('btestuser1', ['514'], ['Disabled'], 100),`
		`+ ('btestuser2', ['65536'], ['PasswordNeverExpired'], 101),`
		`+ ('btestuser3', ['8388608'], ['PasswordExpired'], 102),`
		`+ ('btestuser4', ['256'], ['TempDuplicateAccount'], 103),`
		`+ ('btestuser5', ['16777216'], ['TrustedAuthDelegation'], 104),`
		`+ ('btestuser6', ['528'], ['AccountLocked'], 105),`
		`+ ('btestuser7', ['513'], ['AccountActive'], 106),`
		`+ ('btestuser11', ['655236'], ['TestStatus1'], 107),`
		`+ ('btestuser12', ['665522'], ['TestStatus2'], 108),`
		`+ ('btestuser13', ['266552'], ['TestStatus3'], 109),`
		`+ ('btestuser8', ['98536', '99512', '99528'],`
		`+ ['AccountActive', 'PasswordExxpired', 'AccountLocked'], 110),`
		`+ ('btestuser9', ['87536', '912', ], ['AccountActive',`
		`+ 'PasswordNeverExpired', ], 111),`
		`+ ('btestuser10', ['89536', '97546', '96579'],`
		`+ ['TestVerify1', 'TestVerify2', 'TestVerify3'], 112)]:`
		`+ CreateUsers(users, user[0], user[1], user[2], user[3]).user_create()`
		`+`
		`+ def fin():`
		`+ """`
		`+ Deletes entries after the test.`
		`+ """`
		`+ for user in users.list():`
		`+ user.delete()`
		`+`
		`+ suffix.delete()`
		`+ backend.delete()`
		`+`
		`+ request.addfinalizer(fin)`
		`+`
		`+`
		`+ def increasesizelimit(topo, size):`
		`+ """`
		`+ Will change nsslapd-sizelimit to desire value`
		`+ """`
		`+ topo.standalone.config.set('nsslapd-sizelimit', str(size))`
		`+`
		`+`
		`+ def test_bitwise_plugin_status(topo, _create_schema):`
		`+ """`
		`+ Checking bitwise plugin enabled or not, by default it should be enabled.`
		`+ If disabled, this test case would enable the plugin`
		`+ :id:3ade097e-9ebd-11e8-b2e7-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ # Assert plugin BitwisePlugin is on`
		`+ assert BitwisePlugin(topo.standalone).status()`
		`+`
		`+`
		`+ def test_search_disabled_accounts(topo, _create_schema):`
		`+ """`
		`+ Searching for integer Disabled Accounts.`
		`+ Bitwise AND operator should match each integer, so it should return one entry.`
		`+ :id:467ef0ea-9ebd-11e8-a37f-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+`
		`+ """`
		`+ assert len(Accounts(topo.standalone, SUFFIX).filter(FILTER_CONTROL)) == 2`
		`+`
		`+`
		`+ def test_plugin_can_be_disabled(topo, _create_schema):`
		`+ """`
		`+ Verify whether plugin can be disabled`
		`+ :id:4ed21588-9ebd-11e8-b862-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ bitwise = BitwisePlugin(topo.standalone)`
		`+ assert bitwise.status()`
		`+ # make BitwisePlugin off`
		`+ bitwise.disable()`
		`+ topo.standalone.restart()`
		`+ assert not bitwise.status()`
		`+`
		`+`
		`+ def test_plugin_is_disabled(topo, _create_schema):`
		`+ """`
		`+ Testing Bitwise search when plugin is disabled`
		`+ Bitwise search filter should give proper error message`
		`+ :id:54bebbfe-9ebd-11e8-8ca4-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ with pytest.raises(ldap.UNAVAILABLE_CRITICAL_EXTENSION):`
		`+ Accounts(topo.standalone, SUFFIX).filter(FILTER_CONTROL)`
		`+`
		`+`
		`+ def test_enabling_works_fine(topo, _create_schema):`
		`+ """`
		`+ Enabling the plugin to make sure re-enabling works fine`
		`+ :id:5a2fc2b8-9ebd-11e8-8e18-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ # make BitwisePlugin off`
		`+ bitwise = BitwisePlugin(topo.standalone)`
		`+ bitwise.disable()`
		`+ # make BitwisePlugin on again`
		`+ bitwise.enable()`
		`+ topo.standalone.restart()`
		`+ assert bitwise.status()`
		`+ assert len(Accounts(topo.standalone, SUFFIX).filter(FILTER_CONTROL)) == 2`
		`+`
		`+`
		`+ @pytest.mark.parametrize("filter_name, value", [`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=513))", 1),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=16777216))", 1),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=8388608))", 1),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.804:=5))", 3),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.804:=8))", 3),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.804:=7))", 5),`
		`+ (f"(& ({FILTER_TESTERPERSON}) (testUserAccountControl:1.2.840.113556.1.4.804:=7))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}) (&(testUserAccountControl:1.2.840.113556.1.4.803:=98536)"`
		`+ "(testUserAccountControl:1.2.840.113556.1.4.803:=912)))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}) (&(testUserAccountControl:1.2.840.113556.1.4.804:=87)"`
		`+ "(testUserAccountControl:1.2.840.113556.1.4.804:=91)))", 8),`
		`+ (f"(& ({FILTER_TESTPERSON}) (&(testUserAccountControl:1.2.840.113556.1.4.803:=89536)"`
		`+ "(testUserAccountControl:1.2.840.113556.1.4.804:=79)))", 1),`
		`+ (f"(& ({FILTER_TESTPERSON}) (\|(testUserAccountControl:1.2.840.113556.1.4.803:=89536)"`
		`+ "(testUserAccountControl:1.2.840.113556.1.4.804:=79)))", 8),`
		`+ (f"(& ({FILTER_TESTPERSON}) (\|(testUserAccountControl:1.2.840.113556.1.4.803:=89)"`
		`+ "(testUserAccountControl:1.2.840.113556.1.4.803:=536)))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=x))", 13),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=&\\*#$%))", 13),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=-65536))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=-1))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=-))", 13),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=))", 13),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=\\*))", 13),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.804:=\\*))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=6552))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}\\))(testUserAccountControl:1.2.840.113556.1.4.804:=6552))", 0),`
		`+ (f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=65536))", 5)`
		`+ ])`
		`+ def test_all_together(topo, _create_schema, filter_name, value):`
		`+ """Target_set_with_ldap_instead_of_ldap`
		`+ :id: ba7f5106-9ebd-11e8-9ad6-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ assert len(Accounts(topo.standalone, SUFFIX).filter(filter_name)) == value`
		`+`
		`+`
		`+ def test_5_entries(topo, _create_schema):`
		`+ """`
		`+ Bitwise filter test for 5 entries`
		`+ By default the size limit is 2000`
		`+ Inorder to perform stress tests, we need to icrease the nsslapd-sizelimit.`
		`+ IncrSizeLimit 52000`
		`+ :id:e939aa64-9ebd-11e8-815e-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ filter51 = f"(& ({FILTER_TESTPERSON}) (testUserAccountControl:1.2.840.113556.1.4.803:=8388608))"`
		`+ increasesizelimit(topo, 52000)`
		`+ users = UserAccounts(topo.standalone, SUFFIX, rdn=None)`
		`+ for i in range(5):`
		`+ CreateUsers(users, i).create_users_other()`
		`+ assert len(Accounts(topo.standalone, SUFFIX).filter(filter51)) == 6`
		`+ increasesizelimit(topo, 2000)`
		`+`
		`+`
		`+ def test_5_entries1(topo, _create_schema):`
		`+ """`
		`+ Bitwise filter for 5 entries`
		`+ By default the size limit is 2000`
		`+ Inorder to perform stress tests, we need to icrease the nsslapd-sizelimit.`
		`+ IncrSizeLimit 52000`
		`+ :id:ef8b050c-9ebd-11e8-979d-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ filter52 = f"(& ({FILTER_TESTPERSON})(testUserAccountControl:1.2.840.113556.1.4.804:=16777216))"`
		`+ increasesizelimit(topo, 52000)`
		`+ users = UserAccounts(topo.standalone, SUFFIX, rdn=None)`
		`+ for i in range(5):`
		`+ CreateUsers(users, i).user_create_52()`
		`+ assert len(Accounts(topo.standalone, SUFFIX).filter(filter52)) == 6`
		`+ increasesizelimit(topo, 2000)`
		`+`
		`+`
		`+ def test_5_entries3(topo, _create_schema):`
		`+ """`
		`+ Bitwise filter test for entries`
		`+ By default the size limit is 2000`
		`+ Inorder to perform stress tests, we need to icrease the nsslapd-sizelimit.`
		`+ IncrSizeLimit 52000`
		`+ :id:f5b06648-9ebd-11e8-b08f-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ increasesizelimit(topo, 52000)`
		`+ assert len(Accounts(topo.standalone, SUFFIX).filter(`
		`+ "(testUserAccountControl:1.2.840.113556.1.4.803:=8388608, "`
		`+ "['attrlist=cn:sn:uid:testUserAccountControl'])")) == 6`
		`+ increasesizelimit(topo, 2000)`
		`+`
		`+`
		`+ def test_5_entries4(topo, _create_schema):`
		`+ """`
		`+ Bitwise filter for entries`
		`+ By default the size limit is 2000`
		`+ Inorder to perform stress tests, we need to icrease the nsslapd-sizelimit.`
		`+ IncrSizeLimit 52000`
		`+ :id:fa5f7a4e-9ebd-11e8-ad54-8c16451d917b`
		`+ :setup: Standalone`
		`+ :steps:`
		`+ 1. Create Filter rules.`
		`+ 2. Try to pass filter rules as per the condition .`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ """`
		`+ increasesizelimit(topo, 52000)`
		`+ assert len(Accounts(topo.standalone, SUFFIX).`
		`+ filter("(testUserAccountControl:1.2.840.113556.1.4.804:=16777216,"`
		`+ "['attrlist=cn:sn:uid:testUserAccountControl'])")) == 6`
		`+ increasesizelimit(topo, 2000)`
		`+`
		`+`
		`+ if __name__ == '__main__':`
		`+ CURRENT_FILE = os.path.realpath(__file__)`
		`+ pytest.main("-s -v %s" % CURRENT_FILE)`

aborah commented 5 years ago

Investigate and port TET matching rules filter tests

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

Reviewed by: ???

firstyear commented on line 226 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 5 years ago

Raw search usage.

firstyear commented on line 281 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 5 years ago

no one liners

firstyear commented on line 307 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 5 years ago

What is the point of this restart?

firstyear commented on line 130 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 5 years ago

This is ... not right at all. You should NEVER use useraccount on a NOT user account type! There is a inst.config.set_<VALUE>() series of functions.
You don't need this helper at all.

firstyear commented on line 19 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 5 years ago

This list is too complex and hard to follow.

firstyear commented 5 years ago

Hey there,

I have given lots of feedback on other test cases you have provided, but it seems you may have missed it. I provide feedback not just on a patch, but to help educate you, to be a better programmer so that every time you submit a patch it improves and lifts the quality of your work and what we accept. I'm seeing some patterns in here that are things I have already commented on in past patches. I'd really like you to do a self review of your work, keeping in mind the many comments I have provided you already. Additionally, I'd really like to see you ask questions to the 389-devel list when you are stuck, rather than inventing solutions. An example is you could have asked what is the right way to set a value in cn=config, becauuse it is very little effort for simon or I to answer, and would save you the time of patch + review etc.

So please review your work with all my comments in mind thanks.

tbordaz commented on line 279 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 5 years ago

I am not sure to understand the need of this setting+restart
If the search is done by DM, sizelimit is not enforced and any value should be fine.
If it is not DM, then the number of returned entry is generally low (e.g. 6) so why this setting to 52000 ? Do you need to set lookthroughlimit rather that size ?

tbordaz commented on line 177 of dirsrvtests/tests/suites/filter/filterscanlimit_test.py 5 years ago

Is this testcase really pass ?
IMHO some should succeed (e.g. 4000, 'limit=0 type=sub', 'type=eq') but most of the others setting should trigger something like unwilling to perform

vashirov commented 5 years ago

Raw search usage.

What is the recommended way to search with a specific filter on arbitrary LDAP entry?

vashirov commented 5 years ago

Is this testcase really pass ?
IMHO some should succeed (e.g. 4000, 'limit=0 type=sub', 'type=eq') but most of the others setting should trigger something like unwilling to perform

This test case passes, but it shouldn't. All these values are accepted, only errors are logged. It should really reject incorrect values.

vashirov commented 5 years ago

This list is too complex and hard to follow.

This remark is not helpful, please suggest a better way.

tbordaz commented on line 177 of dirsrvtests/tests/suites/filter/filterscanlimit_test.py 5 years ago

Surprisingly I was able to apply successfully all those settings. But some of them should fail (they are invalid).
Also I applied those setting on an index entry, but the code seems to apply it on a userAccount that make no real sense

aborah commented 5 years ago

Surprisingly I was able to apply successfully all those settings. But some of them should fail (they are invalid).
Also I applied those setting on an index entry, but the code seems to apply it on a userAccount that make no real sense

def test_error_handling_for_invalid_configuration(topo):
""""
Error handling for invalid configuration
Starting...test cases for bug1011539
Index config error handling does not exist - you can add any old thing
:id:377950f6-9f06-11e8-831b-8c16451d917b
:setup: Standalone instance
:steps:
1. Create Instance
2. Try change nsIndexIDListScanLimit
:expectedresults:
1. This should pass
2. This should pass
"""
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'4000')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 flags=bogus')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 limit=1')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=eq type=eq')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=sub type=eq')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 flags=AND flags=AND')])
assert topo.standalone.modify_s(GIVEN_NAME, [
(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=eq values=foo values=foo')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=eq values=foo,foo')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=sub')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=eq values=notvalid')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit')])
assert topo.standalone.modify_s(GIVEN_NAME, [
(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=eq values=notavaliddn')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=pres values=bogus')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=eq,sub values=bogus')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=1 type=')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=1 flags=')])
assert topo.standalone.modify_s(GIVEN_NAME,
[(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=1 type=eq values=')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=-2')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'type=eq')])
assert topo.standalone.modify_s(GIVEN_NAME, [(ldap.MOD_REPLACE, 'nsIndexIDListScanLimit', b'limit=0 type=bogus')])

This was the actual test case , (This one is also passed ) as raw modify_s is not allowed , i have used userAccount . With topo.standalone.modify_s it takes all the invalid values

tbordaz commented on line 177 of dirsrvtests/tests/suites/filter/filterscanlimit_test.py 5 years ago

https://pagure.io/389-ds-base/issue/50201 to track the successful update of invalid values

rebased onto 1e272ef64001fb06fbce4f8135114a3771b9da9a

5 years ago

aborah commented 5 years ago

Changes are made , Please check

rebased onto 3a38e10a163e45fbb0a54edc518feb72896d2b96

5 years ago

rebased onto 6698871209c2718fda114122bb2eedb293f161c3

4 years ago

rebased onto 6c5df7e22a7c85d7072bf0028fbe01475df45148

4 years ago

rebased onto a4e9164a55ab956ef40961cfeac6a0daa7384972

4 years ago

rebased onto f1dd1e76256e866920fd7c66391fcf6689a22dd8

4 years ago

spichugi commented on line 60 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

Please, check PEP8 for the function names
https://www.python.org/dev/peps/pep-0008/

spichugi commented on line 26 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

We should use the existing plugin classes. If there is no class for Bitwise Plugin in plugins.py, we should create one.

spichugi commented on line 196 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

You use the same type of object in three places. It will be more efficient to create the class instance one time in the beginning and then reuse it.

spichugi commented on line 30 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

More meaningful constant names will increase readability.

aborah commented 4 years ago

We should use the existing plugin classes. If there is no class for Bitwise Plugin in plugins.py, we should create one.

'cn=Bitwise Plugin,cn=plugins,cn=config' is already there in plugins.py

(Pdb) for i in Plugins(topo.standalone, DEFAULT_SUFFIX).list(): i.dn
'cn=7-bit check,cn=plugins,cn=config'
'cn=Account Policy Plugin,cn=plugins,cn=config'
'cn=Account Usability Plugin,cn=plugins,cn=config'
'cn=ACL Plugin,cn=plugins,cn=config'
'cn=ACL preoperation,cn=plugins,cn=config'
'cn=attribute uniqueness,cn=plugins,cn=config'
'cn=Auto Membership Plugin,cn=plugins,cn=config'
'cn=Binary Syntax,cn=plugins,cn=config'
'cn=Bit String Syntax,cn=plugins,cn=config'
'cn=Bitwise Plugin,cn=plugins,cn=config'
'cn=Boolean Syntax,cn=plugins,cn=config'
'cn=Case Exact String Syntax,cn=plugins,cn=config'
'cn=Case Ignore String Syntax,cn=plugins,cn=config'
'cn=chaining database,cn=plugins,cn=config'
'cn=Class of Service,cn=plugins,cn=config'
'cn=Content Synchronization,cn=plugins,cn=config'

spichugi commented 4 years ago

We should use the existing plugin classes. If there is no class for Bitwise Plugin in plugins.py, we should create one.

'cn=Bitwise Plugin,cn=plugins,cn=config' is already there in plugins.py
(Pdb) for i in Plugins(topo.standalone, DEFAULT_SUFFIX).list(): i.dn

I mean Python classes which are defined like class PluginName(Plugin):

Edited 4 years ago by spichugi

spichugi commented on line 307 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

Please, run --pylint.
Also, it will mention that eval is a bad practice. It is slow, hard to debug, unsafe and hard to read and follow.

Please, rework it... I see that the only things that repeat in params are the filters and the number for the len().
So you can use them for the parametrization.

aborah commented 4 years ago

We should use the existing plugin classes. If there is no class for Bitwise Plugin in plugins.py, we should create one.
'cn=Bitwise Plugin,cn=plugins,cn=config' is already there in plugins.py
(Pdb) for i in Plugins(topo.standalone, DEFAULT_SUFFIX).list(): i.dn

I mean Python classes which are defined like class PluginName(Plugin):

PR raised: https://pagure.io/389-ds-base/pull-request/50359

spichugi commented on line 194 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

Also, we have 'disable' and 'enable' for plugins

spichugi commented on line 228 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

And please, make sure that the tests may run separately. Do not assume that the plugin is disabled.

rebased onto 25e1839217819a952c7c0e58daef00b240892607

4 years ago

aborah commented 4 years ago

@spichugi all changes are done as per your suggestion

spichugi commented on line 28 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

I mean that it will be nicer to have the constant names that reflect the actual purpose.
You have only three of them here and each of them is used for the different situations.
So, please, try always name things meaningfully.

spichugi commented on line 256 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

Also, now I am thinking that it will be more failproof if we will create a separate backend instead of using the default one. It will help us to make sure that the number of returned users are exact and won't be changed in the future (like DEFAULT_SUFFIX can have a different number of sample entries in different versions)

rebased onto 85607d12204494b4d326ae9d047701f5d5a084fe

4 years ago

aborah commented 4 years ago

@spichugi all changes are done as per your suggestion , ready with new back end

rebased onto b2f5b3bab3612d3d22011e89f43f4d0782f9b926

4 years ago

spichugi commented 4 years ago

Please, rebase PR with the latest master.
Also, check pylint output for your test case, it has a few issues.

rebased onto 52a8242f0383fb8e33fbe10f4a2a661c689e6a72

4 years ago

rebased onto 184d7035dce471a09510d58c341751c38713f831

4 years ago

aborah commented 4 years ago

@spichugi rebased .

pylint is giving error like :
Anomalous backslash in string: '*'. String constant might be missing an r prefix. (anomalous-backslash-in-string)

Which is part of filter . Cant be corrected

mhonek commented 4 years ago

pylint is giving error like :
Anomalous backslash in string: '*'. String constant might be missing an r prefix. (anomalous-backslash-in-string)
Which is part of filter . Cant be corrected

@aborah

>>> "\*"
'\\*'
>>> "\\*"
'\\*'
>>> r"\*"
'\\*'
>>> r"\\*"
'\\\\*'
>>> b"\*"
b'\\*'
>>> b"\\*"
b'\\*'
>>> "\n"
'\n'
>>> "\\n"
'\\n'

Notice how in the first case Python converts one backslash to two of them? It's Python being nice to you and not failing just because it does not know the escape sequence. :) For comparison, the \n is not changed into \\n because the \n actually is understood by Python as a request for a newline character.

That said, you should properly rewrite the Python strings so that they evaluate to the actually expected sequences. Just check out by hand with a Python interpreter on the fly.

rebased onto d934b5041f3269dd92eeab6590d1cf8e2fb5e510

4 years ago

aborah commented 4 years ago

@spichugi @mhonek all changes are done as per your suggestion

spichugi commented on line 123 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

Please, don't ignore problems if you don't understand something. It is okay to ask if you haven't found the answer.
Pylint states W:122, 4: Global variable 'SUFFIX' undefined at the module level (global-variable-undefined)
So you should define the default value on the module level.

Edited 4 years ago by spichugi

rebased onto 607fc264ffd569467cb4780d63a3536ac62236f1

4 years ago

aborah commented 4 years ago

@spichugi all changes are done as per your suggestion

spichugi commented on line 121 of dirsrvtests/tests/suites/filter/bitw_filter_test.py 4 years ago

You can use SUFFIX here and everywhere instead of 'dc=anuj,dc=com'

rebased onto fdb52106601ea6d5537e26aa3ece24e08255d8bb

4 years ago

aborah commented 4 years ago

@spichugi all changes are done as per your suggestion

rebased onto d0da028