Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable bytecode writing due to pytest's assertion rewrite (DISCUSSION) #810

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@nicoddemus
Copy link
Contributor

nicoddemus commented Oct 6, 2016

After some investigation I found what the problem related in pytest-dev/pytest#1888 is:

  1. pytest installs an import hook which rewrites asserts in test modules (since 2.X series) and plugins and conftest files (new in 3.X).
  2. The hook tries to cache the changed bytecode to disk, and to do that it calls os.mkdir.
  3. Now, setuptools executes some tests in a sandboxed environment which disallows certain functions that modify the disk from being called, such as os.mkdir.
  4. The sandboxed environment traps the call to os.mkdir, but to raise an error it tries to import SandboxViolation, which again triggers the import hook, which again tries to call os.mkdir, and so on.

Here's the relevant part of the traceback from a setuptools job from my fork which uses pytest 3.0.3:

self = <_pytest.assertion.rewrite.AssertionRewritingHook object at 0x7fac57374198>
name = 'setuptools.extern'
path = ['/home/travis/build/nicoddemus/setuptools/setuptools']

<strip some code>

        # The requested module looks like a test file, so rewrite it. This is
        # the most magical part of the process: load the source, rewrite the
        # asserts, and load the rewritten source. We also cache the rewritten
        # module code in a special pyc. We must be aware of the possibility of
        # concurrent pytest processes rewriting and loading pycs. To avoid
        # tricky race conditions, we maintain the following invariant: The
        # cached pyc is always a complete, valid pyc. Operations on it must be
        # atomic. POSIX's atomic rename comes in handy.
        write = not sys.dont_write_bytecode
        cache_dir = os.path.join(fn_pypath.dirname, "__pycache__")
        if write:
            try:
>               os.mkdir(cache_dir)
../../../virtualenv/python3.5.2/lib/python3.5/site-packages/_pytest/assertion/rewrite.py:107: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <setuptools.sandbox.DirectorySandbox object at 0x7fac5504db38>
path = '/home/travis/build/nicoddemus/setuptools/setuptools/__pycache__'
args = (), kw = {}
    def wrap(self, path, *args, **kw):
        if self._active:
>           path = self._remap_input(name, path, *args, **kw)
setuptools/sandbox.py:302: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <setuptools.sandbox.DirectorySandbox object at 0x7fac5504db38>
operation = 'mkdir'
path = '/home/travis/build/nicoddemus/setuptools/setuptools/__pycache__'
args = (), kw = {}
    def _remap_input(self, operation, path, *args, **kw):
        """Called for path inputs"""
        if operation in self.write_ops and not self._ok(path):
>           self._violation(operation, os.path.realpath(path), *args, **kw)
setuptools/sandbox.py:448: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <setuptools.sandbox.DirectorySandbox object at 0x7fac5504db38>
operation = 'mkdir'
args = ('/home/travis/build/nicoddemus/setuptools/setuptools/__pycache__',)
kw = {}
    def _violation(self, operation, *args, **kw):
>       from setuptools.sandbox import SandboxViolation
setuptools/sandbox.py:403: 

This explains why pytest-dev/pytest#1891 fixed this issue in pytest-3.0: pytest was trying to rewrite a bunch of modules that shouldn't be rewritten in the first place because they were not pytest plugins.

This also explains why this broke again in 3.0.3: pytest-dev/pytest#1934 was introduced. Now pytest will rewrite plugins installed in development mode, so it again tries to rewrite the setuptools own plugin during the sandboxed tests, triggering the sandbox violation.

As a quick workaround, I disabled bytecode writing so pytest will re-generate the bytecode but won't cache it into disk.

Another possible solution would be to use --assert=plain which disables the rewrite-hook completely, but then rich assertion errors would be lost.

I'm opening this PR in order to discuss other possible solutions to this problem.

Disable bytecode writing due to pytest's assertion rewrite
setuptools test suite uses a sandboxed environment in some
tests that install wrappers around some builtin
functions that access disk (for example os.mkdir).

pytest's assertion rewriting mechanism tries to cache
the rewritten bytecode by calling "os.mkdir" during
the import process, and that breaks the entire suite.

As a quick around, disable bytecode writing when running
the tests.

Related to pytest-dev/pytest#1888

@nicoddemus nicoddemus force-pushed the nicoddemus:pytest-1888 branch from de69aec to 40b1ac3 Oct 6, 2016

@jaraco

This comment has been minimized.

Copy link
Member

jaraco commented Oct 14, 2016

Thanks very much for the extensive troubleshooting and detailed description. I think the solution here will be to avoid the sandboxing altogether by relying on tox as the test runner or possible rwt as the dependency resolver instead of setup.py test. I'm moving in the direction of deprecating that altogether due to the unintended interactions between setuptools build constructs (e.g. sandboxing) and the test runner.

@nicoddemus

This comment has been minimized.

Copy link
Contributor Author

nicoddemus commented Oct 14, 2016

I think the solution here will be to avoid the sandboxing altogether by relying on tox as the test runner or possible rwt as the dependency resolver instead of setup.py test.

Is sandboxing done by setup.py test? Oh I thought it was done somewhere in the test suite, but didn't actually searched for it.

I'm moving in the direction of deprecating that altogether due to the unintended interactions between setuptools build constructs (e.g. sandboxing) and the test runner.

What should we do with this PR? IMHO I think it is an acceptable workaround for now, it causes a very minimal (I suspect negligible) performance loss when collecting the test suite and only on local runs, as the CI servers use clean VMs for all executions.

@jaraco

This comment has been minimized.

Copy link
Member

jaraco commented Oct 15, 2016

After switching the codebase to use tox for test preparation, the tests now pass. Thanks so much for the help on this.

@jaraco jaraco closed this Oct 15, 2016

@nicoddemus

This comment has been minimized.

Copy link
Contributor Author

nicoddemus commented Oct 15, 2016

Glad I could help!

@nicoddemus nicoddemus deleted the nicoddemus:pytest-1888 branch Oct 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.