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

Clean up ctypes.test, use unittest test discovery #66258

Closed
zware opened this issue Jul 24, 2014 · 11 comments
Closed

Clean up ctypes.test, use unittest test discovery #66258

zware opened this issue Jul 24, 2014 · 11 comments
Assignees
Labels
tests Tests in the Lib/test dir topic-ctypes type-feature A feature request or enhancement

Comments

@zware
Copy link
Member

zware commented Jul 24, 2014

BPO 22060
Nosy @amauryfa, @abalkin, @pitrou, @ezio-melotti, @voidspace, @meadori, @zware, @serhiy-storchaka
Files
  • ctypes.test-cleanup.diff
  • issue22060.diff
  • issue22060.v2.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/zware'
    closed_at = <Date 2014-08-08.18:35:41.980>
    created_at = <Date 2014-07-24.20:32:05.820>
    labels = ['ctypes', 'type-feature', 'tests']
    title = 'Clean up ctypes.test, use unittest test discovery'
    updated_at = <Date 2014-08-08.18:37:28.567>
    user = 'https://github.com/zware'

    bugs.python.org fields:

    activity = <Date 2014-08-08.18:37:28.567>
    actor = 'zach.ware'
    assignee = 'zach.ware'
    closed = True
    closed_date = <Date 2014-08-08.18:35:41.980>
    closer = 'python-dev'
    components = ['Tests', 'ctypes']
    creation = <Date 2014-07-24.20:32:05.820>
    creator = 'zach.ware'
    dependencies = []
    files = ['36073', '36305', '36314']
    hgrepos = []
    issue_num = 22060
    keywords = ['patch']
    message_count = 11.0
    messages = ['223891', '224956', '224960', '225041', '225062', '225064', '225075', '225076', '225080', '225081', '225082']
    nosy_count = 9.0
    nosy_names = ['amaury.forgeotdarc', 'belopolsky', 'pitrou', 'ezio.melotti', 'michael.foord', 'meador.inge', 'python-dev', 'zach.ware', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22060'
    versions = ['Python 3.4', 'Python 3.5']

    @zware
    Copy link
    Member Author

    zware commented Jul 24, 2014

    Attached is a patch that aims to clean up the ctypes tests a bit, namely by removing the custom resource management (which conflicts with regrtest), the custom test discovery (which is better left to unittest), and the custom test running (which is better covered by unittest and regrtest). The one thing I'm not entirely confident about removing is the custom refleak testing, but it does not seem to work correctly in 3.x anyway (though in 2.7, the custom refleak hunter reports "leaks" that the regrtest refleak hunter does not).

    There were only a few uses of the custom resource management, all of which were replaced or removed. test_SEH in test_win32 used "requires('SEH')", but that test should now be sufficiently guarded with unittest skip decorators (only trying the test on Windows, with Python built in Release configuration by MSVC). test_PyLong_Long in test_python_api used "requires('refcount')", but that should be covered by the @support.refcount_test decorator (added long after the 'requires' call). Two instances of "is_resource_enabled('printing')" were replaced by "if test.support.verbose".

    The same number of tests run (all successfully) on Windows, I have not yet tested on any other platforms.

    @zware zware added tests Tests in the Lib/test dir topic-ctypes type-feature A feature request or enhancement labels Jul 24, 2014
    @serhiy-storchaka
    Copy link
    Member

    Changes look simple and straightforward. LGTM.

    It would be good also move tests into the Lib/test/test_ctypes subdirectory and eliminate Lib/test/test_ctypes.py (may be in separate issue). And may be move initialization and verbose printinting in setUpModule() functions.

    @zware
    Copy link
    Member Author

    zware commented Aug 6, 2014

    Serhiy Storchaka added the comment:
    It would be good also move tests into the Lib/test/test_ctypes subdirectory and eliminate Lib/test/test_ctypes.py (may be in separate issue).

    I plan to do so as part of bpo-10572, if approved (or if there's no
    vehemently negative feedback on that issue within a couple weeks).

    And may be move initialization and verbose printinting in setUpModule() functions.

    Fair point, I'll get a new patch up soon.

    @zware
    Copy link
    Member Author

    zware commented Aug 7, 2014

    Here's a new patch; have I missed anything else that ought to be in setUpModule?

    @serhiy-storchaka
    Copy link
    Member

    When the verbose variable is imported, it is impossible to control verbosity at runtime. It will be better to import only the test.support module itself and then access its attribute.

    @serhiy-storchaka
    Copy link
    Member

    But this doesn't matter. The patch LGTM.

    @zware
    Copy link
    Member Author

    zware commented Aug 8, 2014

    In pre-commit testing, I realized I was getting skips that I shouldn't have been getting, and it turns out that I screwed up both setUpModule functions from the previous patch. New patch fixes test_loading and test_find to use the skipTest method instead of skip decorators, so the necessary variables are actually set when they're looked for. I also combined the setUpModule in test_find with the setUp method of the TestOpenGL_libs into a setUpClass classmethod, which makes things a little simpler and cleaner.

    Since I was in there anyway, I went ahead and fixed your point about test.support.verbose. I had not thought about that issue previously.

    I'll go ahead and commit later today unless something else jumps out as being wrong.

    @zware
    Copy link
    Member Author

    zware commented Aug 8, 2014

    Maybe this one is actually right...

    @serhiy-storchaka
    Copy link
    Member

    Please commit.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 8, 2014

    New changeset fc99cf3615bb by Zachary Ware in branch '3.4':
    Issue bpo-22060: Clean up/simplify test_ctypes, use test discovery
    http://hg.python.org/cpython/rev/fc99cf3615bb

    New changeset 748fb6d622e8 by Zachary Ware in branch 'default':
    Closes bpo-22060: Merge with 3.4
    http://hg.python.org/cpython/rev/748fb6d622e8

    @python-dev python-dev mannequin closed this as completed Aug 8, 2014
    @zware
    Copy link
    Member Author

    zware commented Aug 8, 2014

    Done. Thank you for the reviews, Serhiy (and my apologies about all the spam with bad patches earlier today).

    @zware zware self-assigned this Aug 8, 2014
    @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 topic-ctypes type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants