Issue #3737
closed[tests only] memory error in unittests when mock.MagicMock(parent='anything') is used
Description
This issue is a PSA.
- something to look for in code review
- searchable in case someone else runs into it
tldr; Don't set the `parent` attribute on a mock.MagicMock object
When running unittests for uploads, a PulpException was raised (because of a mistake in my test code). This problem won't affect users, but makes these tests fragile and very challenging to debug if something does go wrong.
Uploads are handled by the steps system, which attempts to walk up the step tree, appending each exception.
https://github.com/pulp/pulp/blob/2-master/server/pulp/plugins/util/publish_step.py#L268-L281
Throughout, test_uploads create a mock parent:
parent = mock.MagicMock(file_path=img, parent=None, uploaded_unit=None)
step = upload.AddUnits(step_type=constants.UPLOAD_STEP_SAVE,
working_dir=step_work_dir)
step.parent = parent
Even though it explicitly sets `parent=None`, mock ignores this. `parent` is an explicit kwarg in mockland, and it appears to be ignored here.
https://github.com/testing-cabal/mock/blob/master/mock/mock.py#L1035
In [1]: import mock
In [2]: x = mock.MagicMock(parent=None)
In [3]: x.parent
Out[3]: <MagicMock name='mock.parent' id='140366001241808'>
So, when a PulpException is raised in a unit test, it results in the steps system infinitely recursing through mocks until we get a memory error.
(pulp) [vagrant@pulp2 pulp_docker]$ ./run-tests.py
Running flake8
Running Unit Tests
............................................................SSSSSSSSSSSSSS............................................E.EETraceback (most recent call last):
File "/home/vagrant/.virtualenvs/pulp/bin/nosetests", line 11, in <module>
sys.exit(run_exit())
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/core.py", line 121, in __init__
**extra_args)
File "/usr/lib64/python2.7/unittest/main.py", line 95, in __init__
self.runTests()
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/core.py", line 207, in runTests
result = self.testRunner.run(self.test)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/core.py", line 62, in run
test(result)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 177, in __call__
return self.run(*arg, **kw)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/suite.py", line 224, in run
test(orig)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/case.py", line 45, in __call__
return self.run(*arg, **kwarg)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/case.py", line 138, in run
result.addError(self, err)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/proxy.py", line 132, in addError
self.result.addError(self.test, self._prepareErr(err))
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/result.py", line 61, in addError
exc_info = self._exc_info_to_string(err, test)
File "/home/vagrant/.virtualenvs/pulp/lib/python2.7/site-packages/nose/result.py", line 182, in _exc_info_to_string
from nose.plugins.skip import SkipTest
MemoryError
Updated by CodeHeeler almost 6 years ago
- Triaged changed from No to Yes
- Sprint set to Sprint 38
Updated by dkliban@redhat.com almost 6 years ago
- Sprint changed from Sprint 39 to Sprint 40
Updated by amacdona@redhat.com over 5 years ago
- Sprint changed from Sprint 43 to Sprint 44
Updated by amacdona@redhat.com over 5 years ago
- Status changed from ASSIGNED to NEW
- Assignee deleted (
amacdona@redhat.com) - Sprint deleted (
Sprint 45)
Updated by bmbouter about 5 years ago
- Status changed from NEW to CLOSED - WONTFIX
Updated by bmbouter about 5 years ago
Pulp 2 is approaching maintenance mode, and this Pulp 2 ticket is not being actively worked on. As such, it is being closed as WONTFIX. Pulp 2 is still accepting contributions though, so if you want to contribute a fix for this ticket, please reopen or comment on it. If you don't have permissions to reopen this ticket, or you want to discuss an issue, please reach out via the developer mailing list.