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

IDLE: Add docstrings and unittests to outwin.py #74802

Closed
csabella opened this issue Jun 9, 2017 · 6 comments
Closed

IDLE: Add docstrings and unittests to outwin.py #74802

csabella opened this issue Jun 9, 2017 · 6 comments
Assignees
Labels
3.7 (EOL) end of life topic-IDLE type-feature A feature request or enhancement

Comments

@csabella
Copy link
Contributor

csabella commented Jun 9, 2017

BPO 30617
Nosy @terryjreedy, @csabella
PRs
  • bpo-30617: IDLE: docstrings and unittest for outwin.py #2046
  • bpo-31284: IDLE: Fix WindowList warning message when running tests #3212
  • [3.6] bpo-30617: IDLE: docstrings and unittest for outwin.py (GH-2046) #3223
  • 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/terryjreedy'
    closed_at = <Date 2017-08-27.22:31:38.498>
    created_at = <Date 2017-06-09.21:55:30.612>
    labels = ['expert-IDLE', 'type-feature', '3.7']
    title = 'IDLE: Add docstrings and unittests to outwin.py'
    updated_at = <Date 2017-08-27.22:31:38.497>
    user = 'https://github.com/csabella'

    bugs.python.org fields:

    activity = <Date 2017-08-27.22:31:38.497>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2017-08-27.22:31:38.498>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2017-06-09.21:55:30.612>
    creator = 'cheryl.sabella'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 30617
    keywords = []
    message_count = 6.0
    messages = ['295586', '295587', '295598', '295790', '300929', '300931']
    nosy_count = 2.0
    nosy_names = ['terry.reedy', 'cheryl.sabella']
    pr_nums = ['2046', '3212', '3223']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue30617'
    versions = ['Python 3.6', 'Python 3.7']

    @csabella csabella added the 3.7 (EOL) end of life label Jun 9, 2017
    @csabella csabella added topic-IDLE type-feature A feature request or enhancement labels Jun 9, 2017
    @csabella
    Copy link
    Contributor Author

    csabella commented Jun 9, 2017

    For bpo-30422.

    @csabella
    Copy link
    Contributor Author

    csabella commented Jun 9, 2017

    Questions about outwin.py:

    1. Should a main() be added for htest and unittest?

    2. In _file_line_helper, to test if the file exists, the file is opened, then closed. Would os.path.isfile work here?

    Questions about tests:

    1. OutputWindow is an EditorWindow and EditorWindow has a superclass of Object. I didn't know how to make tests to test the text binding for goto-file-line. Or how to test the other functions. I tried emulating the dialogs, but couldn't get it to work. Is there any existing test for another module I can study and copy? EditorWindow didn't really have any tests for this yet, but if I learn how to do it here, I can apply it there.

    2. The goto_line_file function compiles the class attribute for the matching patterns file_line_pats which is then used by _file_line_helper. I had trouble writing a test just for the helper without copying this pattern compile code from the other function. I wasn't sure how to get around that. Are the patterns at the class level so that they are cahced for all the calls? How should I change the test? Can the patterns be compiled inside _file_line_helper?

    3. Is the test of _file_line_helper the right way to go with the tests? Should examples of each pattern be included? I didn't add a test for the TypeError because the pattens didn't seem like they would match on anything but digits. Should I add a pattern for testing just for code coverage?

    Thanks!

    @terryjreedy
    Copy link
    Member

    A1: yes, add
    if __name__ == '__main__':
    import unittest
    unittest.main('idlelib.idle_test.test_outwin', verbosity=2, exit=False)

    I will have to think about whether and how to add an htest. The grep (find in files) test actually displays greps results in an outwin, and suggests rt clicking within the result window, so it really tests both grep and outwin modules. Maybe this htest should be split.

    A2. The current code tests whether the file exists *and* can be read. It must have been read to appear in grep output. But I am not sure this is true for names in tracebacks.

    B1. This is first test of an editor window. Having the least code, it it probably the best place to start and learn ;-).

    B2. The class level patterns are compiled to an instance array for each instance in which a user tries to go to file/line. That should be most. This looks to me like an attempted optimization (don't compile unless needed) that is a pessimization. I think either A) the compiled patterns should be stored back on the class or B) raw and compiled patterns should be stored at module level. They are not instance attributes and not really class attributes either. Also, the code blocks that create file_line_progs and use it really belong in module level functions. So lets try B.

    Move stuff from class to module scope:

    file_line_pats = [
       ...]
    file_line_progs = None
    
    def compile_progs():
        global file_line_progs
        file_line_progs = [re.compile(pat, re.IGNORECASE)
                           for pat in file_line_pats]
    
    def find_file_line(line_of_text):
        <body of current _file_line_helper, with 'self.' deleted>
    
    # and simply method
       def goto_file_line(self, event=None): # in class
            if not file_line_progs:
                compile_progs()

    Test for compile_progs (:
    for pat, regex in zip(file_line_pats, file_line_progs):
    assertEqual(regex.pattern, pat)

    Then feed find_file_line lines that should and should not match. Try to find at least 1 true-to-life line for each pattern.

    B3. In re, \d "matches any Unicode decimal digit (that is, any character in Unicode character category [Nd])". Builtin int (at least) accepts strings consisting of string literal, which is restricted to ascii digit. msg90878 says "int and float currently accept characters in category 'Nd'". I am not sure if that mean *all* Nd chars. We could test. If the exception is impossible, it should not be caught. (Dead code should be removed, but we must be sure it is dead ;-)

    @csabella
    Copy link
    Contributor Author

    Thanks!

    I've added more unittests. They are passing, but I'm getting a callback error that I don't understand. The same _utest flag that's on the textview dialog didn't seem to work and I was just blindly trying things without success. Also, the window is briefly opening and closing on the screen. Any suggestions on where I should look next?

    B3 below. Thanks for the link to that other message. Combining that with some comments on SO, I tried to find unicode that would be selected by \d and would fail with int(), but couldn't find any. However, I also realized that the try/except is checking for a TypeError and not a ValueError. I went and looked at the change history for OutputWindow, but couldn't see anything that would show why there might be a TypeError check here. If the match group is None, it wouldn't get this far in the code.

    Additionally,

    1. I removed 'from tkinter import *' because it wasn't being used.
    2. I changed 'import tkinter.messagebox as tkMessageBox' per bpo-30422.
    3. There were two lines:
      edit = self.flist.open(filename)
      edit.gotoline(lineno)
      but flist had a gotofileline() that did the same thing. In the historical code, the 'edit = ' line had and 'or', so maybe that's why it was separated.
    4. *args on the __init__. I've seen *args used for passing config options and things like that, but I don't understand why it would be preferable here instead of listing the 4 arguments explicitly.

    Thanks!

    @terryjreedy
    Copy link
    Member

    New changeset 998f496 by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
    bpo-30617: IDLE: docstrings and unittest for outwin.py (bpo-2046)
    998f496

    @terryjreedy
    Copy link
    Member

    New changeset 5c89c2f by Terry Jan Reedy in branch '3.6':
    [3.6] bpo-30617: IDLE: docstrings and unittest for outwin.py (GH-2046) (bpo-3223)
    5c89c2f

    @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.7 (EOL) end of life topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants