All tests under dirsrv_test.py fail due to an undefined serverid attribute of the DirSrv object:
sudo py.test -s lib389/lib389/tests/dirsrv_test.py
_________________________________________________________________________________________ ERROR at setup of test_allocate _________________________________________________________________________________________ request = <SubRequest 'topology' for <Function 'test_allocate'>> @pytest.fixture(scope='module') def topology(request): instance = DirSrv(verbose=False) if instance.exists(): > instance.delete() lib389/tests/dirsrv_test.py:36: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ lib389/__init__.py:161: in inner return f(*args, **kwargs) lib389/__init__.py:977: in delete cmd = "%s -i %s%s" % (prog, DEFAULT_INST_HEAD, self.serverid) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <lib389.DirSrv object at 0x7fee9a91add0>, name = 'serverid' def __getattr__(self,name): if self.CLASSATTR_OPTION_MAPPING.has_key(name): return self.get_option(self.CLASSATTR_OPTION_MAPPING[name]) elif self.__dict__.has_key(name): return self.__dict__[name] else: raise AttributeError,'%s has no attribute %s' % ( > self.__class__.__name__,repr(name) ) E AttributeError: DirSrv has no attribute 'serverid' /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:136: AttributeError __________________________________________________________________________________________ ERROR at setup of test_create __________________________________________________________________________________________ request = <SubRequest 'topology' for <Function 'test_allocate'>> @pytest.fixture(scope='module') def topology(request): instance = DirSrv(verbose=False) if instance.exists(): > instance.delete() lib389/tests/dirsrv_test.py:36: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ lib389/__init__.py:161: in inner return f(*args, **kwargs) lib389/__init__.py:977: in delete cmd = "%s -i %s%s" % (prog, DEFAULT_INST_HEAD, self.serverid) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <lib389.DirSrv object at 0x7fee9a91add0>, name = 'serverid' def __getattr__(self,name): if self.CLASSATTR_OPTION_MAPPING.has_key(name): return self.get_option(self.CLASSATTR_OPTION_MAPPING[name]) elif self.__dict__.has_key(name): return self.__dict__[name] else: raise AttributeError,'%s has no attribute %s' % ( > self.__class__.__name__,repr(name) ) E AttributeError: DirSrv has no attribute 'serverid' /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:136: AttributeError ___________________________________________________________________________________________ ERROR at setup of test_open ___________________________________________________________________________________________ request = <SubRequest 'topology' for <Function 'test_allocate'>> @pytest.fixture(scope='module') def topology(request): instance = DirSrv(verbose=False) if instance.exists(): > instance.delete() lib389/tests/dirsrv_test.py:36: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ lib389/__init__.py:161: in inner return f(*args, **kwargs) lib389/__init__.py:977: in delete cmd = "%s -i %s%s" % (prog, DEFAULT_INST_HEAD, self.serverid) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <lib389.DirSrv object at 0x7fee9a91add0>, name = 'serverid' def __getattr__(self,name): if self.CLASSATTR_OPTION_MAPPING.has_key(name): return self.get_option(self.CLASSATTR_OPTION_MAPPING[name]) elif self.__dict__.has_key(name): return self.__dict__[name] else: raise AttributeError,'%s has no attribute %s' % ( > self.__class__.__name__,repr(name) ) E AttributeError: DirSrv has no attribute 'serverid' /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:136: AttributeError __________________________________________________________________________________________ ERROR at setup of test_close ___________________________________________________________________________________________ request = <SubRequest 'topology' for <Function 'test_allocate'>> @pytest.fixture(scope='module') def topology(request): instance = DirSrv(verbose=False) if instance.exists(): > instance.delete() lib389/tests/dirsrv_test.py:36: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ lib389/__init__.py:161: in inner return f(*args, **kwargs) lib389/__init__.py:977: in delete cmd = "%s -i %s%s" % (prog, DEFAULT_INST_HEAD, self.serverid) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <lib389.DirSrv object at 0x7fee9a91add0>, name = 'serverid' def __getattr__(self,name): if self.CLASSATTR_OPTION_MAPPING.has_key(name): return self.get_option(self.CLASSATTR_OPTION_MAPPING[name]) elif self.__dict__.has_key(name): return self.__dict__[name] else: raise AttributeError,'%s has no attribute %s' % ( > self.__class__.__name__,repr(name) ) E AttributeError: DirSrv has no attribute 'serverid' /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:136: AttributeError __________________________________________________________________________________________ ERROR at setup of test_delete __________________________________________________________________________________________ request = <SubRequest 'topology' for <Function 'test_allocate'>> @pytest.fixture(scope='module') def topology(request): instance = DirSrv(verbose=False) if instance.exists(): > instance.delete() lib389/tests/dirsrv_test.py:36: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ lib389/__init__.py:161: in inner return f(*args, **kwargs) lib389/__init__.py:977: in delete cmd = "%s -i %s%s" % (prog, DEFAULT_INST_HEAD, self.serverid) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <lib389.DirSrv object at 0x7fee9a91add0>, name = 'serverid' def __getattr__(self,name): if self.CLASSATTR_OPTION_MAPPING.has_key(name): return self.get_option(self.CLASSATTR_OPTION_MAPPING[name]) elif self.__dict__.has_key(name): return self.__dict__[name] else: raise AttributeError,'%s has no attribute %s' % ( > self.__class__.__name__,repr(name) ) E AttributeError: DirSrv has no attribute 'serverid' /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:136: AttributeError
The attribute is declared in the following line, but it's under a conditional statement: https://pagure.io/lib389/blob/master/f/lib389/__init__.py#_496
if self.isLocal: self.userid = args.get(SER_USER_ID) if not self.userid: if os.getuid() == 0: # as root run as default user self.userid = DEFAULT_USER else: self.userid = pwd.getpwuid(os.getuid())[0] # Settings from args of server attributes self.serverid = args.get(SER_SERVERID_PROP, None)
I wonder if this means that isLocal isn't always set properly.
Rather than setting it in allocate(), maybe we should set it in _createDirsrv(), because if we created it, it must be local.
Alternately, we need to fix the isLocal check, which is probably needed anyway. If we set SER_SERVERID_PROP, we probably want isLocal I think, but it's really hard to know. It's a side effect of historical decisions to have local vs remote interactions in one. Really they should be seperate IMO.
Actually, I think that the problem is that instance.allocate() is never called in the following test fixture: https://pagure.io/lib389/blob/master/f/lib389/tests/dirsrv_test.py#_32
@pytest.fixture(scope='module') def topology(request): instance = DirSrv(verbose=False) if instance.exists(): instance.delete() def fin(): if instance.exists(): instance.delete() request.addfinalizer(fin) return TopologyInstance(instance)
All the tests in the same module use this fixture and they fail on instance.delete(). That happens because delete() expects to find a serverid attribute but it doesn't. The serverid attributed is supposed to be set in allocate() but we never call it.
Metadata Update from @ilias95: - Issue assigned to ilias95
I think that we CANNOT just add an "instance.allocate()" in the test fixture because the test_allocate test case uses this fixture and it attempts to test this exact thing; if the allocate() method works properly.
So, probably the best solution is to indeed move some code under _createDirsrv(), but we should carefully decide on which chunks of code exactly.
Finally, I think it may be simpler. It's possible that the problem resides inside the test fixture itself. I added a few comments on the code below:
@pytest.fixture(scope='module') def topology(request): instance = DirSrv(verbose=False) if instance.exists(): # we just created the instance to test on it, why do we delete it? instance.delete() # do we really need this? def fin(): # if we can remove the instance.delete() statement above if instance.exists(): # this is executed! instance.delete() # this is NEVER executed! request.addfinalizer(fin) return TopologyInstance(instance)
Removing the first conditional statement makes all tests to pass. I'm submitting a patch, please review carefully.
<img alt="0001-Ticket-21-Test-failures-on-dirsrv_test.py-due-to-und.patch" src="/lib389/issue/raw/files/f58262938481e98e5d8d33c29e68b541e05e33e7b8d04f371a9e71aac6605c11-0001-Ticket-21-Test-failures-on-dirsrv_test.py-due-to-und.patch" />
Metadata Update from @ilias95: - Custom field Review Status adjusted to review
Hmmm, I don't quite agree with this fix.
I certainly acknowledge it's a fault, but we still need the delete. This is so that if a test crashes partway through, the instance is still there, so we need to cleanup before we start again.
This test specifically looks like it's testing creates / deletes, so it doesn't actually build a new instance. Remember, DirSrv() makes a new object, but it's actually the the call to inst.create() that installs it.
the inst.allocate function actually just sets up the dirsrv variables, so perhaps the fix is what I have attached.
<img alt="0001-Ticket-21-Missing-serverid-in-dirsrv_test-due-to-inc.patch" src="/lib389/issue/raw/files/f6d5e9b2bdbc0b95e274632abe908051ce13b45bc39a6b229241f8c2b53461ee-0001-Ticket-21-Missing-serverid-in-dirsrv_test-due-to-inc.patch" />
I'm not sure if I understand this correctly, but if not, please correct me.
The tests now are executed in a sequential way and each one depends on the previous. I'm saying that because they all have an assert topology.instance.state equals with the state that the last test was supposed to leave the instance at.
So, if a test crashes partway through it doesn't make sense to continue with the other tests in any case. Is all this correct?
That test is quite old, and I believe it is indeed the case - each test depends on the previous.
So yes, if a test fails, it makes no sense to continue. I think you can do this with py.test -x ? Or is there a way to flag this is the test file?
I asked this because you said the rationale behind using delete on this point is that if a test crashes partway through, the instance is still there, so we need to cleanup before we start again. But if it makes no sense to continue after a failure anyway, do we still need the delete code in the fixture?
Furthermore, it seems a bit logically weird to me, to have a test case called "test_allocate", but the actual call to allocate() to happen in the fixture instead of the test case.
Yes, we still need it in the fixture. Because this scenario may occur
ticket4xxxx_test.py fails, and leaves the server behind. now test_allocate starts and ... well the server is already there, so allocate fails.
It's better to always try and remove the instance, even if it's a no-op, because you don't know what order the tests will be run.
The naming is silly. The test is creating instances (thus, test_allocate). however, remember the function dirsrv.allocate() does NOT create an instance: it only sets up the dirsrv object to know how to interact with an instance, and it is the call to .create() that does the allocation.
Yes, I hate those function names. No I can't change it.
You could rename test_allocate to test_create_instance or test_deploy or something though ....
Ok. Your patch solves the issue for me and the tests pass. Thanks!
Metadata Update from @spichugi: - Custom field Review Status adjusted to ack (was: review)
commit 2814d11 To ssh://git@pagure.io/lib389.git de4c2a6..2814d11 master -> master
Metadata Update from @firstyear: - Issue close_status updated to: Fixed - Issue status updated to: Closed (was: Open)
Login to comment on this ticket.