Project

Profile

Help

Issue #3737

[tests only] memory error in unittests when mock.MagicMock(parent='anything') is used

Added by amacdona@redhat.com over 2 years ago. Updated over 1 year ago.

Status:
CLOSED - WONTFIX
Priority:
Normal
Assignee:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version - Docker:
Platform Release:
Target Release - Docker:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
Quarter:

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

History

#1 Updated by CodeHeeler over 2 years ago

  • Triaged changed from No to Yes
  • Sprint set to Sprint 38

#2 Updated by rchan over 2 years ago

  • Sprint changed from Sprint 38 to Sprint 39

#3 Updated by dkliban@redhat.com over 2 years ago

  • Sprint changed from Sprint 39 to Sprint 40

#4 Updated by rchan about 2 years ago

  • Sprint changed from Sprint 40 to Sprint 41

#5 Updated by rchan about 2 years ago

  • Sprint changed from Sprint 41 to Sprint 42

#6 Updated by rchan about 2 years ago

  • Sprint changed from Sprint 42 to Sprint 43

#7 Updated by amacdona@redhat.com about 2 years ago

  • Sprint changed from Sprint 43 to Sprint 44

#8 Updated by rchan almost 2 years ago

  • Sprint changed from Sprint 44 to Sprint 45

#9 Updated by amacdona@redhat.com almost 2 years ago

  • Status changed from ASSIGNED to NEW
  • Assignee deleted (amacdona@redhat.com)
  • Sprint deleted (Sprint 45)

#10 Updated by bmbouter over 1 year ago

  • Status changed from NEW to CLOSED - WONTFIX

#11 Updated by bmbouter over 1 year 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.

#12 Updated by bmbouter over 1 year ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF