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

test_loading.py - false positive result for "def test_find" when find_library() is not functional or the (shared) library does not exist #72463

Closed
aixtools opened this issue Sep 26, 2016 · 11 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@aixtools
Copy link
Contributor

aixtools commented Sep 26, 2016

BPO 28276
Nosy @vadmium, @serhiy-storchaka, @aixtools
PRs
  • gh-72463: Fix Lib/ctypes/test_loading.py so that test_find reports skipped #18312
  • 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 = None
    closed_at = None
    created_at = <Date 2016-09-26.11:04:43.541>
    labels = ['type-bug', '3.9', '3.10', '3.11', 'ctypes', 'tests']
    title = 'test_loading.py - false positive result for "def test_find" when find_library() is not functional or the (shared) library does not exist'
    updated_at = <Date 2021-12-09.13:54:47.449>
    user = 'https://github.com/aixtools'

    bugs.python.org fields:

    activity = <Date 2021-12-09.13:54:47.449>
    actor = 'iritkatriel'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Tests', 'ctypes']
    creation = <Date 2016-09-26.11:04:43.541>
    creator = 'Michael.Felt'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 28276
    keywords = ['patch']
    message_count = 11.0
    messages = ['277411', '277415', '277480', '277543', '277609', '277610', '277793', '278091', '361228', '389854', '389918']
    nosy_count = 4.0
    nosy_names = ['martin.panter', 'serhiy.storchaka', 'Michael.Felt', 'aixtools@gmail.com']
    pr_nums = ['18312']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28276'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    Linked PRs

    @aixtools
    Copy link
    Contributor Author

    in Lib/ctypes/test/test_loading.py there is the following test

        def test_find(self):
            for name in ("c", "m"):
                lib = find_library(name)
                if lib:
                    cdll.LoadLibrary(lib)
                    CDLL(lib)
    1. on AIX, the test is "broken" because lib is None for both names because:
      a1) find_library is basically broken for AIX (needs ldconfig, gcc, and/or other tools not generally available on AIX (production) servers
      a2) "m" will always fail because there is no equivalent to libm.so - AIX libm.a only has static members - none are shared.
    2. IMHO - the test is misnamed "test_find" - as None is an acceptable answer - which is why it 'appears' to PASS on AIX

    As far as library names to look for I suggest switching "m" to "ssl" - as something that should be everywhere.

    And the test should be something more like:

        self.assertEqual(lib not None)

    OR is the test actually testing LoadLibrary and CDLL (as well)

    Back to my humble opinion: assuming the real test is to test LoadLibrary and CDLL by accepting None as a valid result from find_library - false POSITIVE is the result when find_library() is not working.

    @aixtools
    Copy link
    Contributor Author

    The assert to be added is much more simple:

    i.e., self.assertTrue(lib)

    And then you get something like:

    root@x064:[/data/prj/python/python-3.6.0.177/Lib/ctypes/test]../../../python -m unittest test_loading.py
    libc_name is libc.a(shr.o)
    sslib is None
    F.sss
    ======================================================================
    FAIL: test_find (test_loading.LoaderTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/data/prj/python/python-3.6.0.177/Lib/ctypes/test/test_loading.py", line 55, in test_find
        self.assertTrue(lib)
    AssertionError: None is not true

    Ran 7 tests in 0.126s

    FAILED (failures=1, skipped=5)

    @vadmium
    Copy link
    Member

    vadmium commented Sep 27, 2016

    The purpose of the test seems to be to check that finding and loading works for widely-available libraries. However I suspect find_library("c") can fail on other platforms as well. Presumably that is why there is the special case for Cygwin at the top of the file.

    Open SSL is also widely available, but not universal. I think it is even possible to build Python without SSL support, and the tests should not fail in this case.

    Perhaps it would be okay to remove "m" and just run the test on find_library("c"). If so, we can adjust the test to call self.skipTest("C library not found"), which will warn that the test could not be run.

    @vadmium vadmium added the tests Tests in the Lib/test dir label Sep 27, 2016
    @aixtools
    Copy link
    Contributor Author

    Actually, what may be needed are more "@Skip" blocks - as ctypes has always been platform dependent.

    OR - have a counter or boolean that starts as zero or false and increase each time find_library() returns a value - and the test fails if the counter is still zero, or boolean is still false.

    Further, perhaps a separate test_load() that is platform specific - with the correct - read expected - result from find_library(). I say expected because python has always been testing for libc.so.6 while I see in cloud-init that that are calls hard-coded to CDLL("libc.so.7") - but maybe that is something specific for Canonical aka ubuntu.

    However, it seems that expecting libc.so.6 to ALWAYS be present is not safe.

    FYI:

    michael@x071:[/data/prj/python/cloud-init]grep CDLL cloud-init-0*/cloudinit/*.py
    cloud-init-0.7.5/cloudinit/util.py: libc = ctypes.CDLL('/lib/libc.so.7')
    cloud-init-0.7.8/cloudinit/util.py: libc = ctypes.CDLL('/lib/libc.so.7')

    @aixtools
    Copy link
    Contributor Author

    Is this suitable?

        def test_find(self):
            # to track that at least one call to find_library() found something
            found = false
            for name in ("c", "m"):
                lib = find_library(name)
                if lib:
                    found = true
                    cdll.LoadLibrary(lib)
                    CDLL(lib)
            # test is FAILED if nothing was found
            self.assertTrue(found)

    @aixtools
    Copy link
    Contributor Author

    ok - false needs to be False and true needs to be True

    seemed so simple. Sigh.

    @vadmium
    Copy link
    Member

    vadmium commented Oct 1, 2016

    Other tests in this file skip the test if libc_name is None. So I think it would make more sense to skip the test rather than fail in test_find(). I.e.

    if not found:
        self.skipTest("Could not find c and m libraries")

    If you are confident that find_library() will always find libc on AIX, perhaps you can suggest an extra test (or add to an existing test), to first check for the AIX platform, and only then fail if find_library() returned None.

    @aixtoolsgmailcom
    Copy link
    Mannequin

    aixtoolsgmailcom mannequin commented Oct 4, 2016

    On 01-Oct-16 05:57, Martin Panter wrote:

    Martin Panter added the comment:

    Other tests in this file skip the test if libc_name is None. So I think it would make more sense to skip the test rather than fail in test_find(). I.e.

    if not found:
    self.skipTest("Could not find c and m libraries")

    If you are confident that find_library() will always find libc on AIX, perhaps you can suggest an extra test (or add to an existing test), to first check for the AIX platform, and only then fail if find_library() returned None.
    Yes, I am confident - it is part of the core run-time environment:
    # lslpp -w | grep libc.a
    /usr/ccs/lib/libc.a bos.rte.libc File
    /usr/lib/libc.a bos.rte.libc Symlink
    /usr/lib/threads/libc.a bos.rte.libc Symlink

    And, bos.rte.libc cannot be uninstalled:

    SELECTED FILESETS: The following is a list of filesets that you asked to
    remove. They cannot be removed until all of their dependent filesets
    are also removed. See subsequent lists for details of dependents.

     bos.rte.libc 5.3.7.0                      # Base Level Fileset
    

    NON-DEINSTALLABLE DEPENDENTS: The following filesets depend upon one or
    more of the selected filesets listed above. These dependents should
    be removed before the selected filesets. Deinstallability checks
    indicate
    that these dependents should not be removed from the system;
    therefore, the
    selected filesets cannot be removed.

     bos.mp64 5.3.7.0                          # Base Operating System 
    

    64-bit...
    bos.rte 5.3.7.0 # Base Level Fileset
    devices.chrp.IBM.lhea.rte 5.3.7.0 # Host Ethernet Adapter
    (HEA) ...
    devices.chrp.base.rte 5.3.7.0 # RISC PC Base System
    Device S...
    devices.chrp.vdevice.rte 5.3.7.0 # Virtual I/O Bus Support
    devices.vdevice.IBM.v-scsi.rte 5.3.7.0 # Virtual SCSI Client Support
    ifor_ls.base.cli 5.3.7.0 # License Use Management
    Runti...

    I gave up counting the number of dependents that "officially" are
    dependents on bos.rte.libc after the count went above 200 - on a vanilla
    system install.

    I'll make a different test (skip if not aix) that find_library("c") must
    succeed. The load has already been tested - with libc_name already
    hard-coded.

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue28276\>


    @aixtools
    Copy link
    Contributor Author

    aixtools commented Feb 2, 2020

    This is something from long long ago - time to get it completed.

    The (remaining) issue is: "c" and "m" may not be shared libraries - so nothing is ever found and the test is "skipped" but reports itself as PASSED.

    The original issue (fixed for AIX in Python3.7) would be if find_library itself is broken and always returns NULL/None.

    Again, the tests says "PASSED" when actually it was skipped.

    Following this - Martin's suggestion seems a viable solution.

    @aixtools aixtools added type-bug An unexpected behavior, bug, or error 3.8 only security fixes 3.9 only security fixes labels Feb 2, 2020
    @serhiy-storchaka
    Copy link
    Member

    Concur. Also you can add "ssl" to the list of tested names if both "c" and "m" fail on AIX.

    @aixtools
    Copy link
    Contributor Author

    Sure. Probably have to rebase first.

    @iritkatriel iritkatriel added 3.10 only security fixes 3.11 only security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Dec 9, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 29, 2024
    …skipped (pythonGH-18312)
    
    (cherry picked from commit 04d1000)
    
    Co-authored-by: Michael Felt <aixtools@users.noreply.github.com>
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 29, 2024
    …skipped (pythonGH-18312)
    
    (cherry picked from commit 04d1000)
    
    Co-authored-by: Michael Felt <aixtools@users.noreply.github.com>
    serhiy-storchaka pushed a commit that referenced this issue Feb 29, 2024
    … skipped (GH-18312) (GH-116136)
    
    (cherry picked from commit 04d1000)
    
    Co-authored-by: Michael Felt <aixtools@users.noreply.github.com>
    serhiy-storchaka pushed a commit that referenced this issue Feb 29, 2024
    … skipped (GH-18312) (GH-116137)
    
    (cherry picked from commit 04d1000)
    
    Co-authored-by: Michael Felt <aixtools@users.noreply.github.com>
    woodruffw pushed a commit to woodruffw-forks/cpython that referenced this issue Mar 4, 2024
    adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
    diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants