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

Report skipped ctypes tests as skipped #63692

Closed
serhiy-storchaka opened this issue Nov 4, 2013 · 11 comments
Closed

Report skipped ctypes tests as skipped #63692

serhiy-storchaka opened this issue Nov 4, 2013 · 11 comments
Assignees
Labels
tests Tests in the Lib/test dir topic-ctypes type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 19493
Nosy @amauryfa, @abalkin, @meadori, @zware, @serhiy-storchaka
Files
  • skip_tests_ctypes.patch
  • skip_tests_ctypes.v2.diff
  • skip_tests_ctypes.v3.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-06-13.19:43:25.046>
    created_at = <Date 2013-11-04.12:57:58.404>
    labels = ['ctypes', 'type-feature', 'tests']
    title = 'Report skipped ctypes tests as skipped'
    updated_at = <Date 2014-07-23.19:40:59.474>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-07-23.19:40:59.474>
    actor = 'python-dev'
    assignee = 'zach.ware'
    closed = True
    closed_date = <Date 2014-06-13.19:43:25.046>
    closer = 'zach.ware'
    components = ['Tests', 'ctypes']
    creation = <Date 2013-11-04.12:57:58.404>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['32495', '32634', '33216']
    hgrepos = []
    issue_num = 19493
    keywords = ['patch']
    message_count = 11.0
    messages = ['202124', '202775', '202791', '202926', '206628', '206643', '206649', '220479', '220488', '220489', '223769']
    nosy_count = 6.0
    nosy_names = ['amaury.forgeotdarc', 'belopolsky', '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/issue19493'
    versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

    @serhiy-storchaka
    Copy link
    Member Author

    Some skipped ctypes tests are reported as passed. Proposed patch adds explicit reporting them as skipped.

    See also bpo-18702.

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

    zware commented Nov 13, 2013

    Grepping with the same regexes I used for bpo-19572 come up with some extra skips in the ctypes tests too; would you like me to create a new patch including them or would you like to do it, Serhiy?

    @serhiy-storchaka
    Copy link
    Member Author

    Grepping with the same regexes I used for bpo-19572 come up with some extra
    skips in the ctypes tests too; would you like me to create a new patch
    including them or would you like to do it, Serhiy?

    Please do this Zachary.

    @zware
    Copy link
    Member

    zware commented Nov 15, 2013

    Here's a patch. It turned out to be much more extensive than I expected, and the diff turned into a huge huge ugly monster that I hope Rietveld can help to make sense of, since the majority of the diff is simply changes in indentation level.

    The most major change of this patch is the addition of a "needs_symbol" decorator to ctypes.test.__init__, which is then used throughout the tests. Pre-patch, the tests are littered with constructs like this:

    """
    class TestSomething(unittest.TestCase):
    try:
    c_wchar
    except NameError:
    pass
    else:
    def test_something_using_c_wchar(self):
    ...
    """

    These have all (I think) been simplified to:

    """
    class TestSomething(unittest.TestCase):
    @needs_symbol('c_wchar')
    def test_something_using_c_wchar(self):
    ...
    """

    There are also several instances of tests guarded by "if sys.platform = 'win32':" or similar, which have been turned into @unittest.skipUnless, and several tests which were commented out which have been uncommented and given unconditional skips.

    On my Gentoo machine at home, the patch takes the ctypes test suite from 348 tests with 1 skip to 422 tests with 78 skips.

    @zware
    Copy link
    Member

    zware commented Dec 19, 2013

    Here's a new patch addressing your review comment, Serhiy. It also addresses some failures on Windows in test_values: Win_ValuesTestCase depends on 'pydll' being defined in the module toplevel and shadowing ctypes.pydll; this definition was removed some years ago (before ctypes was merged into Python). I replaced each instance of 'pydll' with 'pythonapi', which makes the tests pass (with appropriate update to test_frozentable's expectations), but I don't understand all of ctypes well enough to be sure that it is definitely the correct fix.

    Also, a few long lines that were already touched have been split (without messing with other long lines) and a couple of tests have been converted from "def X_test" to "def test_X" with an unconditional skip. Another empty file has also been removed: test_errcheck is completely commented out except for a couple of imports, so it is removed entirely. The test that calls doctest.testmod in test_objects has been adjusted to fail the test if any of the doctests fail, and to not care about sys.version. Finally, test_wintypes has been rearranged to skip the test class rather than the module on non-Windows platforms to keep the same number of tests between platforms.

    @serhiy-storchaka
    Copy link
    Member Author

    I can't say anything about pydll, other changes LGTM. Except that I'm not sure that test_wintypes needs a fix.

    @zware
    Copy link
    Member

    zware commented Dec 19, 2013

    I'd prefer to keep the change to test_wintypes, simply because I was rather surprised to find an extra test being run on Windows.

    As for the pydll/pythonapi issue, any thoughts from Amaury, Meador, or Alexander? The relevant change that removed the definition of pydll in test_values can be found here[1]. That also makes me wonder why exactly the test is named "*Win*_ValuesTestCase" and whether it actually needs a skip or just a rename, since there doesn't appear to me to be anything related to Windows about the test.

    [1] http://ctypes.cvs.sourceforge.net/viewvc/ctypes/ctypes/ctypes/test/test_values.py?hideattic=0&r1=1.1.2.1&r2=1.1.2.2

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 13, 2014

    New changeset 6f63fff5c120 by Zachary Ware in branch '3.4':
    Issue bpo-19493: Refactor ctypes test package.
    http://hg.python.org/cpython/rev/6f63fff5c120

    New changeset 86d14cf2a6a8 by Zachary Ware in branch 'default':
    Issue bpo-19493: Merge with 3.4
    http://hg.python.org/cpython/rev/86d14cf2a6a8

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 13, 2014

    New changeset 08a2b36f6287 by Zachary Ware in branch '2.7':
    Issue bpo-19493: Backport 6f63fff5c120
    http://hg.python.org/cpython/rev/08a2b36f6287

    @zware
    Copy link
    Member

    zware commented Jun 13, 2014

    Committed; thanks for the review, Serhiy.

    @zware zware closed this as completed Jun 13, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 23, 2014

    New changeset 49a2bed5185a by Zachary Ware in branch '2.7':
    Issue bpo-19493: Fix two uses of ctypes.test.requires (it's not a decorator)
    http://hg.python.org/cpython/rev/49a2bed5185a

    New changeset 374a9a259c09 by Zachary Ware in branch '3.4':
    Issue bpo-19493: Fix two uses of ctypes.test.requires (it's not a decorator)
    http://hg.python.org/cpython/rev/374a9a259c09

    @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