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

Erroneous zipfile test failure if the string 'bad' appears in pwd #65719

Closed
larryhastings opened this issue May 18, 2014 · 8 comments
Closed
Assignees
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@larryhastings
Copy link
Contributor

BPO 21520
Nosy @birkenfeld, @larryhastings, @ned-deily, @serhiy-storchaka
Files
  • larry.bad.zipfile.1.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/larryhastings'
    closed_at = <Date 2015-05-08.14:00:43.310>
    created_at = <Date 2014-05-18.06:46:00.596>
    labels = ['type-bug', 'tests']
    title = "Erroneous zipfile test failure if the string 'bad' appears in pwd"
    updated_at = <Date 2015-05-08.14:00:43.309>
    user = 'https://github.com/larryhastings'

    bugs.python.org fields:

    activity = <Date 2015-05-08.14:00:43.309>
    actor = 'larry'
    assignee = 'larry'
    closed = True
    closed_date = <Date 2015-05-08.14:00:43.310>
    closer = 'larry'
    components = ['Tests']
    creation = <Date 2014-05-18.06:46:00.596>
    creator = 'larry'
    dependencies = []
    files = ['35567']
    hgrepos = []
    issue_num = 21520
    keywords = ['patch']
    message_count = 8.0
    messages = ['218735', '218746', '218751', '220258', '220304', '225462', '242762', '242763']
    nosy_count = 6.0
    nosy_names = ['georg.brandl', 'larry', 'ned.deily', 'python-dev', 'serhiy.storchaka', 'ismail.badawi']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue21520'
    versions = ['Python 3.4', 'Python 3.5']

    @larryhastings
    Copy link
    Contributor Author

    If you extract current Python (3.4 or trunk) into a directory, and anywhere in the name of the directory is the string "bad", such as

    /tmp/badtest
    /home/baddison/src/python

    then test_write_filtered_python_package() in Lib/test/test_zipfile.py will fail. The reason is, the third subtest uses a "filterfunc" to ignore certain files, and its test to ignore files is effectively

    "bad" in fn

    ("fn" is an ill-conceived abbreviation for "filename")

    This is overbroad. Changing it to

    "Lib/test/bad" in fn

    prevents this error.

    I'm creating this issue just to remind myself to fix it. 3.4.1 is tagged and I didn't want to re-cut the release, but I didn't feel like pushing it while 3.4.1 hadn't landed in trunk yet.

    @larryhastings larryhastings self-assigned this May 18, 2014
    @larryhastings larryhastings added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels May 18, 2014
    @larryhastings
    Copy link
    Contributor Author

    Here's an eye-wateringly-thorough description of the bug for the sake of posterity.

    The test code in question is test_write_filtered_python_package() in Lib/test/test_zipfile.py. This function uses PyZipFile to build a zipfile from the contents of the "Lib/test" directory. PyZipFile scans for .py files, then compiles them into .pyc or .pyo files and adds the compiled result.

    The test code actually reuses the PyZipFile object three times:

    The first try succeeds, but raises some warnings because of some deliberately troublesome files in that directory that upset the compiler. These files all contain the substring "bad" in their name, like "Lib/test/bad_coding.py". The warnings are written to stdout; the test captures stdout and scans for the errors. When this function is done, the zipfile contains .pyc files of all the files in Lib/test except for the ones with the substring "bad" in their name.

    The second try succeeds, but ignores every file because of a "filterfunc" passed in that always returns False. It's effectively a no-op--no files are added to the zipfile. The test then scans the output to make sure no warnings were issued.

    The third try succeeds. It uses the "filterfunc" parameter to selectively skip the "bad" files, then scans stdout to ensure that no warnings were issued there. However, since it's re-adding all the other files to the zipfile, this does issue a zillion UserWarning assert warnings. The code suppresses these with a "self.assertWarns(UserWarning)" context manager.

    So here's the bug. If you untarred Python into "/tmp/goodidea", then the test works as expected. But if you untar Python into "/tmp/badidea", then the filterfunc in the third test ignores *every* file, because *every* file contains the substring "bad". Therefore it never adds a single file. And therefore it never fires the UserWarning about redundantly adding a file. Since UserWarning is never issued, and the test is supposed to issue it, the assertWarns context manager flags the test as a failure.

    The easy fix: change the filterfunc to be far more selective, only filtering out paths containing the substring "Lib/test/bad". This would still fail if you untarred Python to "/tmp/Lib/test/bad/", but hopefully nobody will do *that*.

    Perhaps a still-better approach would be
    lambda path: os.path.basename(path).startswith("bad")

    @ned-deily
    Copy link
    Member

    Testing for "Lib/test/bad" isn't correct either since the test will fail when the tests are run from an installed Python rather than just from a build directory.

    @larryhastings
    Copy link
    Contributor Author

    With this patch applied the test passes. (Patch is against 3.4 branch.) Look good?

    @ned-deily
    Copy link
    Member

    Yep

    @serhiy-storchaka
    Copy link
    Member

    The line is too long. It would be better to extract a filter as regular function. Otherwise LGTM.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 8, 2015

    New changeset bff966aed6a3 by Larry Hastings in branch '3.4':
    Issue bpo-21520: test_zipfile no longer fails if the word 'bad' appears
    https://hg.python.org/cpython/rev/bff966aed6a3

    @larryhastings
    Copy link
    Contributor Author

    Checked in, with the filter function on a separate line, to 3.4. Also merged into 3.5.

    @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

    3 participants