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

tests mutating sys.gettrace() w/o re-instating previous state #55199

Closed
brettcannon opened this issue Jan 23, 2011 · 27 comments
Closed

tests mutating sys.gettrace() w/o re-instating previous state #55199

brettcannon opened this issue Jan 23, 2011 · 27 comments
Assignees
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@brettcannon
Copy link
Member

BPO 10990
Nosy @brettcannon
Files
  • issue10990.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/brettcannon'
    closed_at = <Date 2011-02-21.19:31:27.573>
    created_at = <Date 2011-01-23.23:21:15.978>
    labels = ['type-bug', 'tests']
    title = 'tests mutating sys.gettrace() w/o re-instating previous state'
    updated_at = <Date 2011-02-21.19:31:27.572>
    user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2011-02-21.19:31:27.572>
    actor = 'brett.cannon'
    assignee = 'brett.cannon'
    closed = True
    closed_date = <Date 2011-02-21.19:31:27.573>
    closer = 'brett.cannon'
    components = ['Tests']
    creation = <Date 2011-01-23.23:21:15.978>
    creator = 'brett.cannon'
    dependencies = []
    files = ['20718']
    hgrepos = []
    issue_num = 10990
    keywords = ['patch']
    message_count = 27.0
    messages = ['126908', '126966', '126967', '126969', '126970', '126971', '126972', '126973', '126974', '126975', '126976', '126977', '126980', '126981', '127048', '127051', '127052', '127057', '127058', '127077', '127081', '127084', '127235', '127333', '127548', '128182', '128985']
    nosy_count = 2.0
    nosy_names = ['brett.cannon', 'Kristian.Vlaardingerbroek']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue10990'
    versions = ['Python 3.3']

    @brettcannon
    Copy link
    Member Author

    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

    @brettcannon brettcannon added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Jan 23, 2011
    @brettcannon
    Copy link
    Member Author

    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.

    @KristianVlaardingerbroek
    Copy link
    Mannequin

    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

    @brettcannon
    Copy link
    Member Author

    test_pdb uses pdb.set_trace() w/o putting the original trace function back.

    @brettcannon
    Copy link
    Member Author

    test_scope blindly resets the trace function to None.

    @brettcannon
    Copy link
    Member Author

    And here is a revelation: test_sys_settrace clobbers the trace function blindly.

    @brettcannon
    Copy link
    Member Author

    test_zipimport_support fails because test_doctest fails; it re-runs the tests from a zipfile.

    @brettcannon
    Copy link
    Member Author

    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.

    @brettcannon
    Copy link
    Member Author

    test_runpy fails because of a recursion depth test (test_main_recursion_error).

    @brettcannon
    Copy link
    Member Author

    test_richcmp is failing because of a recursion test (test_recursion)

    @brettcannon
    Copy link
    Member Author

    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).

    @brettcannon
    Copy link
    Member Author

    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).

    @brettcannon
    Copy link
    Member Author

    Here is a patch that fixes test_scope.

    @brettcannon
    Copy link
    Member Author

    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.

    @KristianVlaardingerbroek
    Copy link
    Mannequin

    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)

    @brettcannon
    Copy link
    Member Author

    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).

    @brettcannon
    Copy link
    Member Author

    Here is a patch for test_pdb; the context manager made this dirt-simple to fix.

    @brettcannon
    Copy link
    Member Author

    Patch for doctest/test_doctest (yes, both files were wiping out the trace function.

    @brettcannon
    Copy link
    Member Author

    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.

    @brettcannon
    Copy link
    Member Author

    I have a patch that I am testing right now which deals which fixes all the test suites.

    @brettcannon brettcannon self-assigned this Jan 26, 2011
    @brettcannon
    Copy link
    Member Author

    OK, here is a single patch (from hg outgoing --patch) that fixes all the tests by introducing the test.support.no_tracing decorator.

    @brettcannon
    Copy link
    Member Author

    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).

    @KristianVlaardingerbroek
    Copy link
    Mannequin

    Cleaned up patch file. Removed non-related diffs and redundant updates.

    refcount_test decorator is still in there.

    @brettcannon
    Copy link
    Member Author

    Attached is a fixed copy of Kristian's patch; error in test_trace where self.settrace was being called.

    @brettcannon
    Copy link
    Member Author

    Sorry about that. New patch attached.

    @brettcannon
    Copy link
    Member Author

    Updated patch that applies cleanly.

    @brettcannon
    Copy link
    Member Author

    r88467 has it for 3.3.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant