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

Fix test discovery for test_largefile.py #62466

Closed
zware opened this issue Jun 19, 2013 · 5 comments
Closed

Fix test discovery for test_largefile.py #62466

zware opened this issue Jun 19, 2013 · 5 comments
Assignees
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@zware
Copy link
Member

zware commented Jun 19, 2013

BPO 18266
Nosy @brettcannon, @facundobatista, @giampaolo, @ezio-melotti, @zware, @serhiy-storchaka
Files
  • test_largefile_discovery.diff: test_largefile.py fix, version 1
  • test_largefile_discovery.v2-3.3.diff: Version 2, with minor refactoring
  • test_largefile_discovery.v3-3.3.diff: Version 3, faster on Windows
  • 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/serhiy-storchaka'
    closed_at = <Date 2013-07-17.11:39:33.010>
    created_at = <Date 2013-06-19.21:00:54.777>
    labels = ['type-bug', 'tests']
    title = 'Fix test discovery for test_largefile.py'
    updated_at = <Date 2013-07-17.11:39:33.009>
    user = 'https://github.com/zware'

    bugs.python.org fields:

    activity = <Date 2013-07-17.11:39:33.009>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2013-07-17.11:39:33.010>
    closer = 'serhiy.storchaka'
    components = ['Tests']
    creation = <Date 2013-06-19.21:00:54.777>
    creator = 'zach.ware'
    dependencies = []
    files = ['30651', '30764', '30780']
    hgrepos = []
    issue_num = 18266
    keywords = ['patch']
    message_count = 5.0
    messages = ['191494', '192033', '192271', '192346', '193222']
    nosy_count = 7.0
    nosy_names = ['brett.cannon', 'facundobatista', 'giampaolo.rodola', 'ezio.melotti', 'python-dev', 'zach.ware', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18266'
    versions = ['Python 3.3', 'Python 3.4']

    @zware
    Copy link
    Member Author

    zware commented Jun 19, 2013

    This one is another inheritance issue. The patch removes unittest.TestCase as a base for LargeFileTest and converts test_main into setUpModule, load_tests, and tearDownModule. This seems like the way to go due to the fact that the order of test execution matters: test_seek must be first and test_truncate must be last. The only way around that restriction would be to create a new copy of the file for test_truncate, which would make the test take even longer and use even more resources.

    Also, the programmatic creation of the C and Py variants of the test has been changed from creating an empty subclass of LargeFileTest which is then renamed and assigned an 'open' attr, to using the three-arg form of type to create a subclass of LargeFileTest and unittest.TestCase with name set and an 'open' attr pointing to the correct function.

    Lastly, the superfluous check on whether f.truncate is available that had been in test_main is now removed so that the test is properly marked as skipped if skipped.

    @zware zware added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Jun 19, 2013
    @serhiy-storchaka
    Copy link
    Member

    There is other problem with test_largefile. It not allows running only selected tests. I.e.

    ./python -m test.regrtest -v -m test_lseek test_largefile

    Looks as test_largefile was suboptimal converted to unittest.

    @zware
    Copy link
    Member Author

    zware commented Jul 4, 2013

    Here's a stab at fixing that issue. It works, but I don't know that it's the best solution.

    This patch converts test_seek into setUp, and attempts to make sure the test file is in the same state at the beginning of each test, with a new tearDownClass removing the file after each implementation has been tested. setUp also tries not to recreate the file from scratch each time, as doing so would make the test take an unreasonably long time on Windows (currently the test takes around 5 minutes on my Windows 7 machine, creating the file twice. I have not yet tested this patch on Windows, but should be able to before Saturday).

    With this change, load_tests has been removed, and the Py/C test subclasses are created manually.

    Since this has moved from strictly test discovery fixes to a bit more of a refactor, I made a few other minor changes as well. I removed the verbose prints and wrapped a couple of long lines, changed the filesystem test (which is now in setUpModule) to remove its test file, and fixed a couple of comments. Also, I removed 'oldhandler' from the bpo-490453 fix (since it was unused and only served to confuse me until I did some research on it), though I suppose if preferred, it could be saved to reset the signal handler at the end of the run. Either way, that whole fix was also moved into setUpModule with other setup code.

    This patch is against 3.3; it does not apply cleanly to default due to a conflict between IOError and OSError in two places. It should merge forward fine, though.

    @zware
    Copy link
    Member Author

    zware commented Jul 5, 2013

    After testing on Windows, here's version 3. This version changes tearDownClass to truncate the file rather than unlink it, and adds a check to make sure the file is properly truncated to 0 length. Unlinking the file makes the test take an extra 40 seconds on my machine, I suspect due to Windows trying to simultaneously deallocate one 2GB file and build another. To go with that change, tearDownModule is added, which does unlink TESTFN.

    Also changed in this version, I've removed the "use_resources = ['largefile',]" assignment I had added to the "if __name__ == '__main__'" stanza; I've since learned that it has no effect (support.requires lets anything pass if the test module's __name__ is
    "__main__").

    @serhiy-storchaka serhiy-storchaka self-assigned this Jul 17, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 17, 2013

    New changeset dd75dbed1135 by Serhiy Storchaka in branch '3.3':
    Issue bpo-18266: test_largefile now works with unittest test discovery and
    http://hg.python.org/cpython/rev/dd75dbed1135

    New changeset 2d8573e12591 by Serhiy Storchaka in branch 'default':
    Issue bpo-18266: test_largefile now works with unittest test discovery and
    http://hg.python.org/cpython/rev/2d8573e12591

    @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

    2 participants