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

Improve Test Coverage Of Pydoc #61666

Closed
bachmann1234 mannequin opened this issue Mar 18, 2013 · 9 comments
Closed

Improve Test Coverage Of Pydoc #61666

bachmann1234 mannequin opened this issue Mar 18, 2013 · 9 comments
Labels
tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@bachmann1234
Copy link
Mannequin

bachmann1234 mannequin commented Mar 18, 2013

BPO 17464
Nosy @bitdancer, @Bachmann1234
Files
  • pydoctests.patch: Updates to test, adding myself to ACKS
  • pydoc_tests_v2.patch: Updates to previous patch
  • pydoc_tests_v3.patch: Updated patch file addressing comments
  • 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 2013-03-19.04:05:51.396>
    created_at = <Date 2013-03-18.18:37:26.747>
    labels = ['type-feature', 'tests']
    title = 'Improve Test Coverage Of Pydoc'
    updated_at = <Date 2013-03-19.04:05:51.375>
    user = 'https://github.com/bachmann1234'

    bugs.python.org fields:

    activity = <Date 2013-03-19.04:05:51.375>
    actor = 'r.david.murray'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-03-19.04:05:51.396>
    closer = 'r.david.murray'
    components = ['Tests']
    creation = <Date 2013-03-18.18:37:26.747>
    creator = 'Matt.Bachmann'
    dependencies = []
    files = ['29448', '29452', '29470']
    hgrepos = []
    issue_num = 17464
    keywords = ['patch']
    message_count = 9.0
    messages = ['184486', '184503', '184504', '184507', '184570', '184582', '184584', '184587', '184588']
    nosy_count = 4.0
    nosy_names = ['r.david.murray', 'python-dev', 'raduv', 'Matt.Bachmann']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17464'
    versions = ['Python 3.4']

    @bachmann1234
    Copy link
    Mannequin Author

    bachmann1234 mannequin commented Mar 18, 2013

    Adds some test coverage to pydoc.

    @bachmann1234 bachmann1234 mannequin added tests Tests in the Lib/test dir type-feature A feature request or enhancement labels Mar 18, 2013
    @raduv
    Copy link
    Mannequin

    raduv mannequin commented Mar 18, 2013

    Hi,

    Here are some small comments to your otherwise good to have patch:

    -- assertEquals has been deprecated in favor of assertEqual, and usually it's great to be consistent across the test suite
    -- likewise maxDiff should be max_diff mainly because of consistency and because of PEP-8's naming styles
    -- self.temp_dir is never cleaned up (on tearDown maybe?) and gettempdir() doesn't really look like a good option; maybe you wanted to create a new temporary directory rather than pointing to the filesystem's tmp dir ?
    -- the patch doesn't apply cleanly, at least for me - the ACKS hunk is failing for me

    @bachmann1234
    Copy link
    Mannequin Author

    bachmann1234 mannequin commented Mar 18, 2013

    Sure thing, ill make the improvements and upload a new patch. Thanks!

    @bachmann1234
    Copy link
    Mannequin Author

    bachmann1234 mannequin commented Mar 18, 2013

    --Changes assertequals to assertequal
    --Removes maxdiff as it should not really be there in the first place
    --Creates an explicit testdir, and cleans it up
    --Last patch did not actually apply cleanly... I reverted and applied this one and this seems to work

    @bitdancer
    Copy link
    Member

    Thanks for the patch.

    Rather than create and destroy a directory for every test (setUp/tearDown), it is possible to use the test.support.temp_cwd context manager to create and destroy one inside the single tests that need it.

    A nit: we prefer to keep the line length to <80 (that is, max 79).

    In test_getting_all_methods_from_class, why is 'method_returning_true' not in the list? I also worry that this test is a bit fragile, but I don't have a good suggestion for how to fix that, so we'll probably just have to live with it.

    @bachmann1234
    Copy link
    Mannequin Author

    bachmann1234 mannequin commented Mar 19, 2013

    Thanks for the feedback :-D

    Changes:

    Utilize test.support.temp_cwd()
    Removed imports that this made unnecessary
    Awesome trick!

    Cut line length down. Don't worry about nitpicking. Anything to get this to be as good as possible.

    I added an explanation to the test class explaining why test_getting_all_methods_from_class does not appear. I also agree this is fragile. I have an idea for how to fix this and will upload it as a separate patch (in this thread) because I am not sure what people will think.

    @bachmann1234
    Copy link
    Mannequin Author

    bachmann1234 mannequin commented Mar 19, 2013

    Eh, sorry... I tried to do a less fragile way of generating the configuration dict. Tried a few things but it very quickly got messy and even worse started to smell like testing the method with... the method being tested.

    I am open to ideas though.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 19, 2013

    New changeset 474f078ec958 by R David Murray in branch 'default':
    bpo-17464: improve pydoc test coverage.
    http://hg.python.org/cpython/rev/474f078ec958

    @bitdancer
    Copy link
    Member

    As discussed with Matt, I deleted the allmethods tests, since the current behavior is actually a bug. pydoc itself doesn't use allmethods, but I did find that numpy at least does so. A separate issue will be opened for that bug.

    @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-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant