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_zoneinfo fails when lzma module is unavailable #85543

Closed
doodspav mannequin opened this issue Jul 22, 2020 · 9 comments
Closed

test_zoneinfo fails when lzma module is unavailable #85543

doodspav mannequin opened this issue Jul 22, 2020 · 9 comments
Labels
3.9 only security fixes 3.10 only security fixes easy tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@doodspav
Copy link
Mannequin

doodspav mannequin commented Jul 22, 2020

BPO 41371
Nosy @pganssle, @miss-islington, @tirkarthi, @doodspav, @nmaynes
PRs
  • bpo-41371: Handle lzma lib import error in test_zoneinfo.py #21730
  • bpo-41371: Handle lzma lib import error in test_zoneinfo.py #21734
  • [3.9] bpo-41371: Handle lzma lib import error in test_zoneinfo.py (GH-21734) #21758
  • [3.9] bpo-41371: Handle lzma lib import error in test_zoneinfo.py (GH-21734) #22039
  • Files
  • test_output.txt: Output from running make test
  • 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 = <Date 2020-10-10.04:54:58.432>
    created_at = <Date 2020-07-22.20:17:30.357>
    labels = ['easy', '3.10', 'tests', '3.9', 'type-crash']
    title = 'test_zoneinfo fails when lzma module is unavailable'
    updated_at = <Date 2020-10-10.04:54:58.431>
    user = 'https://github.com/doodspav'

    bugs.python.org fields:

    activity = <Date 2020-10-10.04:54:58.431>
    actor = 'xtreak'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-10-10.04:54:58.432>
    closer = 'xtreak'
    components = ['Tests']
    creation = <Date 2020-07-22.20:17:30.357>
    creator = 'doodspav'
    dependencies = []
    files = ['49332']
    hgrepos = []
    issue_num = 41371
    keywords = ['patch', 'easy', 'newcomer friendly']
    message_count = 9.0
    messages = ['374106', '374113', '374186', '374801', '374822', '374842', '374967', '378363', '378364']
    nosy_count = 5.0
    nosy_names = ['p-ganssle', 'miss-islington', 'xtreak', 'doodspav', 'nmaynes']
    pr_nums = ['21730', '21734', '21758', '22039']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue41371'
    versions = ['Python 3.9', 'Python 3.10']

    @doodspav
    Copy link
    Mannequin Author

    doodspav mannequin commented Jul 22, 2020

    Issue:
    ======
    _lzma is not built because the required libraries are not available on my machine. test_zoneinfo assumes it is always available, leading it to crash on my machine.

    How I build and ran the tests:
    ==============================
    git clone https://github.com/python/cpython.git (bpo-41364)
    cd cpython
    mkdir build && cd build
    ../configure
    make -j8
    make test > test_output.txt

    Test traceback:
    ===============
    File "/home/doodspav/Jetbrains/CLionProjects/cpython/Lib/test/libregrtest/runtest.py", line 272, in _runtest_inner
    refleak = _runtest_inner2(ns, test_name)
    File "/home/doodspav/Jetbrains/CLionProjects/cpython/Lib/test/libregrtest/runtest.py", line 223, in _runtest_inner2
    the_module = importlib.import_module(abstest)
    File "/home/doodspav/Jetbrains/CLionProjects/cpython/Lib/importlib/init.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
    File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
    File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
    File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
    File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
    File "<frozen importlib._bootstrap_external>", line 790, in exec_module
    File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
    File "/home/doodspav/Jetbrains/CLionProjects/cpython/Lib/test/test_zoneinfo/init.py", line 1, in <module>
    from .test_zoneinfo import *
    File "/home/doodspav/Jetbrains/CLionProjects/cpython/Lib/test/test_zoneinfo/test_zoneinfo.py", line 9, in <module>
    import lzma
    File "/home/doodspav/Jetbrains/CLionProjects/cpython/Lib/lzma.py", line 27, in <module>
    from _lzma import *
    ModuleNotFoundError: No module named '_lzma'

    @doodspav doodspav mannequin added 3.10 only security fixes tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 22, 2020
    @doodspav
    Copy link
    Mannequin Author

    doodspav mannequin commented Jul 22, 2020

    Note:
    =====
    I marked the type as crash because during the test run, the first attempt at test_zoneinfo failed with a segfault

    @tirkarthi
    Copy link
    Member

    I got this error as well. Since lzma is needed to decode the test data the ImportError can be captured to skip the test in setUpModule [0] like other test module setup with similar approach for required cases. I am adding easy tag. Feel free to retriage this if the test data need to be encoded in a different format for the test to support platforms that don't have lzma.

    try:
    import lzma
    except ImportError:
    raise unittest.skipTest("lzma is needed")

    [0]

    @tirkarthi tirkarthi added easy 3.9 only security fixes labels Jul 24, 2020
    @tirkarthi tirkarthi changed the title test_zoneinfo fails test_zoneinfo fails when lzma module is unavailable Jul 24, 2020
    @tirkarthi tirkarthi changed the title test_zoneinfo fails test_zoneinfo fails when lzma module is unavailable Jul 24, 2020
    @nmaynes
    Copy link
    Mannequin

    nmaynes mannequin commented Aug 4, 2020

    I'm creating a pull request that implements the suggestion by xtreak.

    @pganssle
    Copy link
    Member

    pganssle commented Aug 4, 2020

    I think for now skipping the tests when lzma is missing is the easiest thing, though another option would be to drop the compression on the input test data so that the tests don't depend on lzma.

    Taking a look at the data files, it looks like we get around 50% compression using either lzma or gzip, but the uncompressed file is only 32k to start with:

        $ du -b tests/data/*
        31054   tests/data/zoneinfo_data.json
        15127   tests/data/zoneinfo_data.json.gz
        12895   tests/data/zoneinfo_data.json.lz

    We're also currently using the "fat" binaries that zic produces (which includes hard-coded transitions all the way until 2038). The new default for zic is to produce "slim" binaries, and the script to update test data does nothing to explicitly request fat binaries. If we were to switch over to "slim" binaries, the result would be more like this:

        $ du -b tests/data/*
        8297    tests/data/zoneinfo_data_slim.json.gz
        7750    tests/data/zoneinfo_data_slim.json.lz
        15551   tests/data/zoneinfo_data_unc_slim.json

    So we're still looking at ~2:1 compression for both gzip and lzma, but the overall file size is 50% of what it was to start with. The biggest downside to this is that the way the "slim" binaries work is that once a rule repeats indefinitely, zic stops producing explicit transitions for it, and falls back to a simple repeating rule, meaning that the current set of tests would take a different code path.

    I think we can go with the following course of action (3 or 4 different PRs):

    1. Start by skipping the tests when lzma is missing.
    2. Update the test suite so that it is testing more or less the same thing when the binaries are compiled with -b slim.
    3. Change [Lib/test/test_zoneinfo/data/update_test_data.py](https://github.com/python/cpython/blob/main/Lib/test/test_zoneinfo/data/update_test_data.py) so that it pulls the raw data from the tzdata module on PyPI (which is compiled with -b slim) instead of the user's machine.
    4. Change update_test_data.py to stop using lzma and change the tests so that they are able to process the new format of the JSON files.

    If we ever decide that we really want the compression again, I assume that gzip is found more commonly than lzma among systems that don't build the whole standard library, so it might be mildly preferable to switch to gzip.

    @nmaynes
    Copy link
    Mannequin

    nmaynes mannequin commented Aug 4, 2020

    Im still trying to get the hang of the PR workflow so my apologies in advance.

    I closed the first PR by accident. I made the mistake of including a commit for another issue as well as the commit for this issue. When trying to clean up, I reverted back too far and Github closed the PR. I have submitted another PR that imports the lzma library as follows:

    from test.support.import_helper import import_module
    
    lzma = import_module('lzma')

    Let me know if something still does not look right. I'll have some time this evening to work it out.

    @miss-islington
    Copy link
    Contributor

    New changeset 5f0769a by Nathan M in branch 'master':
    bpo-41371: Handle lzma lib import error in test_zoneinfo.py (GH-21734)
    5f0769a

    @tirkarthi
    Copy link
    Member

    New changeset 20bdeed by Karthikeyan Singaravelan in branch '3.9':
    [3.9] bpo-41371: Handle lzma lib import error in test_zoneinfo.py (GH-21734) (GH-22039)
    20bdeed

    @tirkarthi
    Copy link
    Member

    Thanks @doodspav for the report and Thanks nathan for the patch. This somehow missed 3.9.0 but will be available in 3.9.1. I am closing the issue. Feel free to reopen this or a new issue to implement suggestions by Paul in msg374822.

    @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
    3.9 only security fixes 3.10 only security fixes easy tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants