classification
Title: tests mutating sys.gettrace() w/o re-instating previous state
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Kristian.Vlaardingerbroek, brett.cannon
Priority: normal Keywords: patch

Created on 2011-01-23 23:21 by brett.cannon, last changed 2011-02-21 19:31 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
issue10990.diff brett.cannon, 2011-02-08 18:48
Messages (27)
msg126908 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-23 23:21
The attached patch adds resource monitoring to test.regrtest to detect which tests are changing the trace function w/o putting back to what it was previously. The tests listed below are thus all being naughty. This is a meta-issue to help track which tests need to be changed to stop this behavior.

test_doctest
test_exceptions
test_io
test_pdb
test_pickle
test_pickletools
test_richcmp
test_runpy
test_scope
test_sys_settrace
test_zipimport_support
msg126966 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:08
Cinder on IRC found that test_exception's RuntimeError test triggers a trace_trampoline() line of code which resets the trace function as an exception gets triggered in the trace function itself.

test_doctest is being messy and setting pdb's trace function w/o putting back what it wrote over.
msg126967 - (view) Author: Kristian Vlaardingerbroek (Kristian.Vlaardingerbroek) Date: 2011-01-25 00:12
test_pickle and test_pickletools both call test_bad_getattr in pickletester.py:1005

This results in a RuntimeError which leads to the same result as test_exceptions
msg126969 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:31
test_pdb uses pdb.set_trace() w/o putting the original trace function back.
msg126970 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:37
test_scope blindly resets the trace function to None.
msg126971 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:38
And here is a revelation: test_sys_settrace clobbers the trace function blindly.
msg126972 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:41
test_zipimport_support fails because test_doctest fails; it re-runs the tests from a zipfile.
msg126973 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:44
test_io is causing coverage.py to complain thanks to the TextIOWrapperTests, and the regrtest check is complaining about SignalsTests. Don't know why specifically for either.
msg126974 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:51
test_runpy fails because of a recursion depth test (test_main_recursion_error).
msg126975 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 00:52
test_richcmp is failing because of a recursion test (test_recursion)
msg126976 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 01:09
For test_io.*SignalsTests, its all the tests calling check_interrupted_write(). For TestIOWrapperTests its test_threads_write() (although only coverage.py is complaining, not regrtest).
msg126977 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 01:16
OK, now that all the modules have been analyzed, let's see what is what.

The modules not playing nicely with others by blindly reseting the trace module:

test_doctest
test_pdb
test_scope
test_sys_settrace
test_zipimport_support (because of test_doctest)

And the tests failing because of recursion depth:
test_exceptions
test_pickle
test_pickletools
test_richcmp
test_runpy

And test_io is just special thanks to signals and threading.

Here is my thinking on how to solve this. The tests that are not playing nicely with sys.settrace() should (a) be decorated with test.support.cpython_only, and (b) use addCleanup() to properly put the trace function back.

For the recursion depth tests, either the cause (which is probably trace_trampoline()) needs to be analyzed to decide if some other semantics are needed or need something to unset the trace function and then put it back, but only if sys.gettrace() exists (e.g., don't block on non-CPython VMs).
msg126980 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 01:57
Here is a patch that fixes test_scope.
msg126981 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 01:59
Here is a partial patch for test_sys_settrace. It fails on test_19_no_jump_without_trace_function for some reason I don't understand. It also doesn't protect against it being CPython-only as that is a function decorator and basically the whole module needs to be skipped.
msg127048 - (view) Author: Kristian Vlaardingerbroek (Kristian.Vlaardingerbroek) Date: 2011-01-25 20:28
test_trace can be added to this list, each call to runfunc does a sys.settrace()

121:        self.tracer.runfunc(traced_func_loop, 2, 3)
133:        self.tracer.runfunc(traced_func_importing, 2, 5)
145:        self.tracer.runfunc(traced_func_calling_generator)
160:        self.tracer.runfunc(traced_caller_list_comprehension)
183:            tracer.runfunc(method, 20)
225:        self.tracer.runfunc(traced_func_simple_caller, 1)
234:        self.tracer.runfunc(traced_func_importing_caller, 1)
248:        self.tracer.runfunc(obj.inst_method_calling, 1)
266:        self.tracer.runfunc(traced_func_importing_caller, 1)
msg127051 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 21:09
Thanks for the diagnosis, Kristian. Attached is a patch for test_trace which fixes its over-zealous setting of the trace function (doesn't address the failures, though).
msg127052 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 21:14
Here is a patch for test_pdb; the context manager made this dirt-simple to fix.
msg127057 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 21:39
Patch for doctest/test_doctest (yes, both files were wiping out the trace function.
msg127058 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-25 21:39
And with this patch for test_zipimport_support to work thanks to the test_doctest changes, all of the test suites blasting the trace function *should* be fixed.
msg127077 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-26 00:35
I have a patch that I am testing right now which deals which fixes all the test suites.
msg127081 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-26 02:12
OK, here is a single patch (from `hg outgoing --patch`) that fixes all the tests by introducing the test.support.no_tracing decorator.
msg127084 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-26 02:27
OK, here is an updated patch that fixes the introduced failure in test_sys_settrace. This should be ready to go for Python 3.3 once the tree opens up (unless someone reviews it and finds a problem).
msg127235 - (view) Author: Kristian Vlaardingerbroek (Kristian.Vlaardingerbroek) Date: 2011-01-27 22:44
Cleaned up patch file. Removed non-related diffs and redundant updates.

refcount_test decorator is still in there.
msg127333 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-28 20:04
Attached is a fixed copy of Kristian's patch; error in test_trace where self.settrace was being called.
msg127548 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-01-30 20:26
Sorry about that. New patch attached.
msg128182 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-02-08 18:48
Updated patch that applies cleanly.
msg128985 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-02-21 19:31
r88467 has it for 3.3.
History
Date User Action Args
2011-02-21 19:31:27brett.cannonsetstatus: open -> closed

messages: + msg128985
resolution: accepted -> fixed
stage: patch review -> resolved
2011-02-08 18:48:16brett.cannonsetfiles: - issue_10990.diff
2011-02-08 18:48:06brett.cannonsetfiles: + issue10990.diff

messages: + msg128182
2011-01-30 20:26:39brett.cannonsetfiles: + issue_10990.diff

messages: + msg127548
2011-01-30 20:26:09brett.cannonsetfiles: - issue10990.diff
2011-01-28 20:04:44brett.cannonsetfiles: - trace_fxn_protected.diff
2011-01-28 20:04:38brett.cannonsetfiles: - issue10990.diff
2011-01-28 20:04:31brett.cannonsetfiles: + issue10990.diff

messages: + msg127333
2011-01-27 22:44:52Kristian.Vlaardingerbroeksetfiles: + issue10990.diff

messages: + msg127235
2011-01-26 02:27:27brett.cannonsetfiles: + trace_fxn_protected.diff
versions: - Python 3.2
messages: + msg127084

resolution: accepted
stage: patch review
2011-01-26 02:23:52brett.cannonsetfiles: - trace_fxn_protected.diff
2011-01-26 02:13:04brett.cannonsetfiles: - test_zipimport_support.diff
2011-01-26 02:13:01brett.cannonsetfiles: - test_doctest.diff
2011-01-26 02:12:58brett.cannonsetfiles: - test_gdb.diff
2011-01-26 02:12:55brett.cannonsetfiles: - test_trace.diff
2011-01-26 02:12:52brett.cannonsetfiles: - test_sys_settrace.diff
2011-01-26 02:12:48brett.cannonsetfiles: - test_scope.diff
2011-01-26 02:12:43brett.cannonsetfiles: - sys_gettrace_monitor.diff
2011-01-26 02:12:31brett.cannonsetfiles: + trace_fxn_protected.diff

messages: + msg127081
2011-01-26 00:35:36brett.cannonsetassignee: brett.cannon
messages: + msg127077
2011-01-25 21:39:51brett.cannonsetfiles: + test_zipimport_support.diff

messages: + msg127058
2011-01-25 21:39:02brett.cannonsetfiles: + test_doctest.diff

messages: + msg127057
2011-01-25 21:14:42brett.cannonsetfiles: + test_gdb.diff

messages: + msg127052
2011-01-25 21:09:08brett.cannonsetfiles: + test_trace.diff

messages: + msg127051
2011-01-25 20:28:49Kristian.Vlaardingerbroeksetmessages: + msg127048
2011-01-25 01:59:59brett.cannonsetfiles: + test_sys_settrace.diff

messages: + msg126981
2011-01-25 01:57:39brett.cannonsetfiles: + test_scope.diff

messages: + msg126980
2011-01-25 01:16:10brett.cannonsetmessages: + msg126977
stage: needs patch -> (no value)
2011-01-25 01:09:16brett.cannonsetmessages: + msg126976
2011-01-25 00:52:53brett.cannonsetmessages: + msg126975
2011-01-25 00:51:39brett.cannonsetmessages: + msg126974
2011-01-25 00:44:14brett.cannonsetmessages: + msg126973
2011-01-25 00:41:14brett.cannonsetmessages: + msg126972
2011-01-25 00:38:34brett.cannonsetmessages: + msg126971
2011-01-25 00:37:27brett.cannonsetmessages: + msg126970
2011-01-25 00:31:43brett.cannonsetmessages: + msg126969
2011-01-25 00:12:21Kristian.Vlaardingerbroeksetmessages: + msg126967
2011-01-25 00:08:15brett.cannonsetmessages: + msg126966
2011-01-24 10:28:37Kristian.Vlaardingerbroeksetnosy: + Kristian.Vlaardingerbroek
2011-01-23 23:21:15brett.cannoncreate