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

Monkeypatching "compile" breaks pytest sometimes #1985

Closed
4 tasks done
suranap opened this issue Oct 4, 2016 · 10 comments
Closed
4 tasks done

Monkeypatching "compile" breaks pytest sometimes #1985

suranap opened this issue Oct 4, 2016 · 10 comments

Comments

@suranap
Copy link

suranap commented Oct 4, 2016

Thanks for submitting an issue!

Here's a quick checklist in what to include:

  • Include a detailed description of the bug or suggestion

We use the builtin compile function to process Python code at runtime. I wrote a unit test that replaced compile with a function that raises many different types of exceptions. Curiously, some of those exceptions caused an INTERNALERROR in pytest. I don't know what's happening, but it appears monkeypatch is replacing compile when pytest is trying to use it. This is strange, because monkeypatch shouldn't be called until the unit test executes.

  • pip list of the virtual environment you are using
$ pip list 
pip (8.1.1)
py (1.4.31)
pytest (3.0.3)
setuptools (20.10.1)
  • pytest and operating system versions

$ py.test --version This is pytest version 3.0.3, imported from /Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/pytest.py

  • Minimal example if possible
import pytest 

def my_func():
    code = compile("", "name", "exec")

@pytest.mark.parametrize('eclass', [SyntaxError, ArithmeticError])
def test_my_func(monkeypatch, eclass):
    def raise_ex(*args):
        raise eclass()

    monkeypatch.setattr('builtins.compile', raise_ex)
    with pytest.raises(SyntaxError):
        my_func()
$ py.test -v example.py 
============================== test session starts ===============================
platform darwin -- Python 3.5.2, pytest-3.0.3, py-1.4.31, pluggy-0.4.0 -- /Users/surana/Projects/tmp/pytesting/bin/python3.5
cachedir: .cache
rootdir: /Users/surana/Projects/tmp/pytesting, inifile: 
collected 2 items 

example.py::test_my_func[SyntaxError] PASSED
example.py::test_my_func[ArithmeticError] 
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/main.py", line 96, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/main.py", line 131, in _main
INTERNALERROR>     config.hook.pytest_runtestloop(session=session)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 745, in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 339, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 334, in <lambda>
INTERNALERROR>     _MultiCall(methods, kwargs, hook.spec_opts).execute()
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 614, in execute
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/main.py", line 152, in pytest_runtestloop
INTERNALERROR>     item.config.hook.pytest_runtest_protocol(item=item, nextitem=nextitem)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 745, in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 339, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 334, in <lambda>
INTERNALERROR>     _MultiCall(methods, kwargs, hook.spec_opts).execute()
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 613, in execute
INTERNALERROR>     return _wrapped_call(hook_impl.function(*args), self.execute)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 254, in _wrapped_call
INTERNALERROR>     return call_outcome.get_result()
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 279, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 265, in __init__
INTERNALERROR>     self.result = func()
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 614, in execute
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/runner.py", line 66, in pytest_runtest_protocol
INTERNALERROR>     runtestprotocol(item, nextitem=nextitem)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/runner.py", line 79, in runtestprotocol
INTERNALERROR>     reports.append(call_and_report(item, "call", log))
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/runner.py", line 135, in call_and_report
INTERNALERROR>     report = hook.pytest_runtest_makereport(item=item, call=call)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 745, in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 339, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 334, in <lambda>
INTERNALERROR>     _MultiCall(methods, kwargs, hook.spec_opts).execute()
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 613, in execute
INTERNALERROR>     return _wrapped_call(hook_impl.function(*args), self.execute)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 250, in _wrapped_call
INTERNALERROR>     wrap_controller.send(call_outcome)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/skipping.py", line 222, in pytest_runtest_makereport
INTERNALERROR>     rep = outcome.get_result()
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 279, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 265, in __init__
INTERNALERROR>     self.result = func()
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 614, in execute
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/runner.py", line 272, in pytest_runtest_makereport
INTERNALERROR>     longrepr = item.repr_failure(excinfo)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/python.py", line 587, in repr_failure
INTERNALERROR>     return self._repr_failure_py(excinfo, style=style)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/python.py", line 580, in _repr_failure_py
INTERNALERROR>     style=style)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/main.py", line 433, in _repr_failure_py
INTERNALERROR>     style=style, tbfilter=tbfilter)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/_code/code.py", line 423, in getrepr
INTERNALERROR>     return fmt.repr_excinfo(self)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/_code/code.py", line 626, in repr_excinfo
INTERNALERROR>     reprtraceback = self.repr_traceback(excinfo)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/_code/code.py", line 607, in repr_traceback
INTERNALERROR>     reprentry = self.repr_traceback_entry(entry, einfo)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/_code/code.py", line 554, in repr_traceback_entry
INTERNALERROR>     source = self._getentrysource(entry)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/_code/code.py", line 479, in _getentrysource
INTERNALERROR>     source = entry.getsource(self.astcache)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/_code/code.py", line 205, in getsource
INTERNALERROR>     astnode=astnode)
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/lib/python3.5/site-packages/_pytest/_code/source.py", line 349, in getstatementrange_ast
INTERNALERROR>     astnode = compile(content, "source", "exec", 1024)  # 1024 for AST
INTERNALERROR>   File "/Users/surana/Projects/tmp/pytesting/example.py", line 10, in raise_ex
INTERNALERROR>     raise eclass()
INTERNALERROR> ArithmeticError

============================ 1 passed in 0.03 seconds ============================
@nicoddemus
Copy link
Member

nicoddemus commented Oct 4, 2016

Hey @suranap,

Unfortunately the code which generates the traceback representation calls compile behind the scenes for reasons I'm not entirely sure at the moment.

What happens if you call it with pytest --tbstyle=native?

@suranap
Copy link
Author

suranap commented Oct 5, 2016

Unfortunately my version of pytest does not have that flag:

$ py.test --tbstyle=native example.py 
usage: py.test [options] [file_or_dir] [file_or_dir] [...]
py.test: error: unrecognized arguments: --tbstyle=native
  inifile: None
  rootdir: /Users/surana/Projects/tmp/pytesting

@nicoddemus
Copy link
Member

Oops sorry I meant --tb=native:

$ pytest -h
...
  --tb=style            traceback print mode (auto/long/short/line/native/no).

@RonnyPfannschmidt
Copy link
Member

its an expected breakage, the ast is used to find statement ranges and statements
and python does that in the worst possible way - adding a magic argument to compile

if you monkey-patch compile, just leave the ast making intact

we should decide if we want to proof py.test against that kind of usage

@nicoddemus
Copy link
Member

and python does that in the worst possible way - adding a magic argument to compile

Would it be possible to use the ast module instead?

we should decide if we want to proof py.test against that kind of usage

IMHO pytest shouldn't go through hoops to support monkey-patching builtins like open, compile etc... they seem rare edge cases and it makes maintenance more difficult if the pytest's code base cannot rely on Python's builtins.

Having said that I wouldn't be opposed to accept a PR that replaced the compile call used to obtain the AST by an equivalent call to the ast module, but I would think twice before claiming that monkey-patching compile is OK and will continue to be supported in future releases.

@RonnyPfannschmidt
Copy link
Member

last i checked ast calls compile in at least one supported python version

@suranap
Copy link
Author

suranap commented Oct 5, 2016

I ran it with --tb=native and it works. At a minimum, add some documentation warning about patching builtins. I solved this by having a wrapper function call_compile and patching that.

When does monkeypatch undo it's patching? Should it undo before it processes the traceback? That feels like the safer operation. Undo any nonsense the user may have done before running pytest's code.

@RonnyPfannschmidt
Copy link
Member

fixture teardown (and undo) happens after making reports
so we cant undo them

i think its perfectly fine to fall over if people "steal our feet"

@nicoddemus
Copy link
Member

@suranap

At a minimum, add some documentation warning about patching builtins.

Fair enough, see #1986.

Glad to know at least passing --tb=native works.

Should we close this now?

@RonnyPfannschmidt
Copy link
Member

closing as footguns are footguns after all

RonnyPfannschmidt pushed a commit that referenced this issue Oct 5, 2016
* Add note about not monkey-patching builtins

Related to #1985

* Mention -s as well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants