Continuing the IDLE unittest framework initiated in http://bugs.python.org/issue15392.

A small unit test for IDLE FormatParagraph.py. This patch introduces a test module named test_format_paragraph.py inside Lib/idlelib/idle_test, considering the guidance in README file from idle_test. I should have a patch uploaded by Monday 6/17/2013 night.

Here is a uncompleted patch but works for the most part. I thought I would post just in case somebody wanted to provide me comments on the general direction of the patch. The naming might have to change but this follows Terry Reedy's model of monkey patching.

The test passes as written, so I am not sure what 'mostly works' means.

I said elsewhere, and revised README to match, that I changed my mind about test case names starting with 'Test'. Ending with 'Test' is much more common in /test and I decided from experience that it works better in verbose listings.

A complete test of FormatParagraph might have these test cases:
Is_Get_Test (with methods for is_ and get_ functions)
FindTest
ReformatFunctionTest
ReformatClassTest

I am assuming that the latter 3 will each need multiple tests. You can add blank subclasses, possibly with comments on what to do, as I did in test_grep.

(While reviewing reformat_paragraph, you might consider whether you think the 'xxx Should' comments are valid.)

Guido just reminded pydev readers that a proper docstring is a single summary line, with period, optionally followed by a blank line and more explanation. For multiple lines, the closing ''' should be on its own line.

Idle is deficient in this regard, either omitting docstrings where needed or writing docstrings as comments or mis-formatting multiple lines. Writing tests is a good time to add or revise them because one has to understand the function to test it. See comments on review.

I think putting a mock text widget and mock editor class together is one file. They should eventually be filled out, but starting with pass is ok. But see next message.

'Monkey-patching' is importing a module and changing a binding within the module. For Idle testing, a reason to do this is to avoid using tkinter. For example, test_config_name imports configSectionNameDialog and rebinds tkMessageBox to a mock_tk.Mbox*. This is necessary because even though methods are rebound as attributes of a dummy class in the test module, their read-only .__globals__ attribute still points to the module where they are defined.

Monkey patching is only needed for global names used within the method tested. All self.xyx attribute references are resolved with the dummy instance and class. Hence mock Var is used in the dummy class but not monkey-patched into the imported module.

This test does not monkey patch and does not need too. The only imported glogals are re and idleConf, and the latter does not (I presume) involve tkinter. Neither do any of the objects defined in the module. It is really handy for testing that FormatParagraph is initialized with a passed-in editwin argument, so we can simply pass in a non-gui substitute for testing.

• This is done is a setup function so it can and will be undone in a matching teardown function.

The patch models Text.data as a single string. But a tkinter Text contains a sequence of strings. While the single sequence works for the one test of the ...event method, a list of strings is needed for .get and many other methods with position parameters to actually work. And .get is needed for testing find_paragraph and, I am sure, other methods of other classes.

I am not sure of the correct initialization, but it mostly will not matter.

More troublesome is that tkinter lines start at 1, not 0. Possible responses are 1) add a dummy first line to self.data, 2) added all line number by -1 before indexing, or 3) ignore the difference as it will not matter for most tests. I like 3). Any test that cares can prepend an extra \n to the beginning of the text is loads into the editor.

Choosing 3, the methods would be

def __init__(self):
    self.data = ['']  # I think

def setData(self, text)
    self.data = text.split('\n)

def getData(self):
    return '\n'.join(self.data)

# easy so far ;-)

def _decode(self, position):  # private helper
    line, col = position.split('.')
    line = int(line)
    col = len(self.data[line]) if col == '0 lineend' else int(col)
    return line, col

Doc string for Text.get(self, index1, index2=None) is
"Return the text from INDEX1 to INDEX2 (not included)."
Interpreting this gives

def get(self, start, end=None):
    line, col = self._decode(start)
    if end is None:
        return self.data[line][col]
    else:
        endline, endcol = self._decode(end)
        if line == endline:
            return self.data[line][col:endcol]
        else:
            lines = [self.data[line][col:]]
            for i in range(line+1, endline):
                lines.append(self.data[i])
            lines.append(self.data[endline][:endcol])
            return '\n'.join(lines)

This .get code can be used or adapted for .count, .dump, .delete, .replace, and even .insert. At this point, we need a test for the mock Text class. Maybe we can extract something from tkinter.Text tests.

I am not sure how far to go with this; at some point (use of marks or tags?), we say "Use tkinter.Text and make it a gui test.". But handling as least basic indexing and pairs of indexes seems essential to me.

For gui methods like .see and .scan*, the docstring should be something short like "Gui method, do nothing."

I'm not sure if this is worth pursuing, but I made a mock Text Widget that behaves more like an actual Text Widget. I've attached my modified mock_tk.py which I used to create a mock editor window. This works for the test I made in bpo-18279, but I am also working on a version similar to Todd's for that issue.

Terry,
Thank you for the feedback this helps me alot! I will work with Phil Webster and will use his Text Widget and EditorWindow classes. Hopefully this will help us converge on a strong unit test for FormatParagraph.py. Thanks for the reminder about triple quoted strings I think your idea to make sure those strings are in place now is excellent.

Phil (and Todd): I do not know what you mean by "behaves like more like an actual Text Widget" and whether you are talking about internal or external behavior. But I know that tk does not use an immutable string to hold the text. There is no such thing in C. Tk might use one array of 2-byte words with a gap at the cursor and an auxiliary line pointer array, or it might use an array of line structure similar to what I did (but again with arrays rather than immutable strings for each line), or possibly some other structure, However we could only imitate the internals of tk.Text by using (deprecated) PyUNICODE arrays (see the array doc) and doing much more programming effort.

Re-splitting a single line over and over is too inefficient to consider for a repository patch. I believe your particular indexToInt with getLine makes the conversion O(n*n) instead of 'just' O(n), whereas keeping the line in split form makes it O(1).

Side note, as far as I know, "re.split('\n', text)" (the r prefix is not needed here) does the same thing as text.split('\n'). But it is slower and harder to type. We don't use re when a string method does the same thing.

I made slight modifications to Todd's initial patch to test the mock Text widget from bpo-18365.

Thank you for the helpful feedback Terry!

Added tests for FormatParagraph using single/multiline comment blocks and single/multiline strings in the mock editor window.

Here is a summary of the changes:

FormatParagraph.py

• description for format_paragraph_event
• modified so that selecting a long comment line will comment the new wrapped line
• if __name__ == __main__"...

mock_tk.py

• FormatParagraph uses the 'insert' tag, so I implemented tag_add using a dict (not sure if this is the best option...)
• index returns '' if the index refers to selection that doesn't exist (this replicates the real Text widget). It does this by excepting a TypeError and I'm guessing there is a better way to do it than this.
• tags (i.e. 'sel.first') can be set to 'end' or other indexes which would not work with a regular Text widget, but it may be useful here.

mock_idle.py

• added undo start/stop for the mock text widget
• used 'sel.first' and 'sel.last' for get_selection_indices()

test_rstrip.py

• This test used the 'insert' tag, so I set 'insert' to an index to keep the tests passing.

test_text.py

• Same issue with 'insert' as test_rstrip

Should I be submitting multiple patches for these changes or is it ok to combine them? I was also wondering if it would be a good idea to split up the FormatParagraph tests into separate tests.

After reading Terry's comments on the initial patch I turned FormatParagraph's initial comments into a docstring and made sure that all lines were < 80 characters.

What I think still needs to be done.

• FormatParagraph.py:

As near as I can tell from the patch, the comments are correct except that one is needed for reformat_paragraph.

I gather that the substantive code change allows reformatting of a comment block when it is properly selected (with the beginning at the beginning of the line). The proof will be in the testing.

The middle of format_paragraph_event currently looks like

    if comment_header:
        <bunch of no-gui code>
    else:
        newdata = reformat_paragraph(data, maxformatwidth)

where reformat_paragraph is no-gui code. The comment formatting code should be pulled out into a no-gui reformat_comment function (with docstring) and replaced with in its current location with
newdata = reformat_comment(data, maxformatwidth, comment_header).

• test_formatparagraph.py:

As I hinted before, the test file should begin with the simplest functions and those that do not require widgets (or mocks thereof). In the initial idle test issue bpo-15392, Nick recommended "As much as possible, push logic testing down into the non-GUI tests." This is easiet when non-gui logic and widget manipulation are isolated in different function. When logic has already been pushed into non-gui functions, they should normally be tested first.

Is_Get_Test (with methods for is_ and get_ functions)
ReformatCommentTest
ReformatParagraphTest
would be straightforward text test cases.

FindTest
FormatEventTest (split into multiple test cases)
require widgets.

If the helper functions do not work, the functions that use them will not work. So FormatEventTest does not really test the method unless all the other tests pass.

For this file, there is the additional factor that the latter test cases (or groups of test cases) require Text methods that do not currently exist and will not be added immediately. See my message on bpo-18425, as well as below. So they should first be implemented using the tk Text widget and requires('gui').

An advantage of starting with real widgets is that it separates problems with the tested and test files from problems with the mocks. Remember that mocks are only needed for the buildbots, not our desktops with graphics screens.

I have not yet looked at the individual tests, but please do a patch for only these two files.

I am dubious that the current code will work with tkinter. Marks name slice positions. Tags name 0 to many pairs of slice positions, with 'sel' predefined as corresponding to a mouse selection and limited to one pair. According to my tests, tag_add('sel.first', None) adds a *new* tag that has nothing to do with the 'sel' selection tag. Tag.first and tag.last, if not used themselves as tag names, refer to the first and second slice positions of the first pair. Idle only uses 'sel.first' and 'sel.last', which simplifies what we need in Text._decode.

Experiments indicate that the two sel marks can be set with mark_set, in the sense that the internal structures are set even though the editor screen is not affected:

>>> t.mark_set('sel.first', 1.0)
>>> t.mark_set('sel.last', 1.3)
>>> e. get_selection_indices()
('1.0', '1.3')

'Find in files' indicates that Idle never does this. Except for two instances, the only three marks set are 'insert', 'iomark', and 'my_anchor', which again simplifies mock Text.

Once tests are running with Tk, we can comment out the tk use code (see test_rstrip.py) and add to the mocks what is either needed, or appears reusable for other tests, to make these tests pass again.

For instance, we could subclass mock_tk.Test to add test-specific methods with either 'pass' or minimal specialized code needed.

*mock_idle.Editor:

To use the subclass of Test, one option would be a subclass of mock_idle.Editor that uses the subclass of Text.

Editor.__init__.

Or we could add a parameter text_class to init that defaults to mock_tk.Text, so we can inject a subclass thereof without subclassing Editor.

The two undo_block lines are ok but irrelevant until we can use mock test. You might have mentioned that they are copied from EditorWindow. (The reason they work is that EditorWindow().text is not tkinter.Text() but a Python wrapper thereof.)

The purpose of get_selection_indices is to report the endpoints of the user-selected slice of text. Normal users do so with the mouse. We will have to do so by directly setting the return values. If I did not want to be able to run the rests with tk, the following would be sufficient.

    _selection = ('', '')
    def get_selection_indices(self):
        return self._selection
# with test code setting editor._selection = ('u.v', 'x.y')

However, since that will not work with tk, which as near as I can tell, requires text.mark_set.

mock_tk.Text:

Any (non-trivial) changes to this and its tests should be a separate issue or at least a separate patch.

In any case, the patch is wrong as it is. So are the ones for test_rstrip.py and test_text.py.

I added and rewrote tests to use the Text widget (when necessary). The only changes made to FormatParagraph are documentation/comment formatting and the if __main__ function. I will work on the comment header behavior separately and can post the patch in this issue. Thanks for your guidance Terry!

This patch moves the comment_header code to reformat_comment, adds tests for reformat_comment, and implements the fix that Todd mentioned in bpo-18429. In addition, I changed the get_comment_header and get_indent regular expressions to only look for spaces and tabs, rather than any whitespace ('\s'). These functions should only ever get single lines, but I don't think characters like newline would ever need to be part of the comment header.

Some comments on patch 6:

1. tearDownClass was omitted, resulting in boxes left on the screen, at least when running in Idle with win7. I added this to the README.

I am also getting messages like "warning: callback failed in WindowList <class '_tkinter.TclError'> : invalid command name ".57777664.windows" in the Idle Shell window. The number of warnings is not deterministics.

I started idle with 'import idlelib.idle' in the console interpreter, I have also gotten messages in the console window but no more at the moment.

When I got these with test_text, they disappeared, as I remember, when I added root.destroy(). I do not know what is different this time.

I do not see the messages when I run in a console window.
python_d -m idlelib.idle_test.test_formatparagraph

2). The only code that needs to go in setUpClass is code that must only be executed when the tests are run rather than when the class is defined. All the string constants should be normal class code.
test_string = (
" \"\"\"this is a test of a reformat for a triple "
...

3. Only looking for space and tab for indent is correct. I was thinking that perhaps we should only include the first # in comment headers, in case someone does something like
   ########################################...
   # real comment line
   but after looking at the implementation, I decided that anyone with such a block had better select the normal comment lines and omit the all #s line.

4. Formatting comments by stripping the header and reusing the format_paragraph is nice -- assuming that the latter can handle imbedded blank lines. The end of format_comments is flaky, but something that passes a test is way better than nothing. Format_paragraph looks like it could use improvement also, but I think we have improved FormatParagraph.py enough for now.

5. Like many, I prefer to use map with pre-existing functions and comprehensions for new expressions.

I am polishing the tests now and might commit later today.

Patch attached with current version. I noted in a comment that we should probably replace some of format_paragraph with textwrap.wrap.

The find_paragraph test works fine with the current text mock. I split one test method and will probably do more.

It could be considered a bug that find_paragraph included the hidden terminal \n when it finds the final line of the text and that line has no \n. Since that is a fairly unrealistic test case, we might do better to add \n on all test snippets. Certainly, adding \n\n is a bug.

I have the impression from what I have looked at so far is that triple quoted strings are not handled as they should be. I opened bpo-18583 for enhancing the module beyond what we have done here.

Added tests to get 100% coverage (not counting in __name__ block).
More test cleanups. Some of the extra newlines come from getting to 'end' instead of 'insert'. Others are avoided by adding newline to end of test strings, which is more realistic anyway. I commented out tests that I think test bad behavior that should be changed.

I got rid of the shutdown warning by replacing EditorWindow with a simple mock, as explained in the test file. Doing so also reduced the running time from about .40 seconds to .25 (it was 1.0+ before I reduced the redundancy of the tests). All the asserts passed after the substitution except for the last. For some reason, the 2nd line of the comment block looses its \n so that lines 2 and 3 get pasted together. I am baffled since the block is almost the same as the previous one. For the moment, I just commented out that test.

New changeset 453b4f89a2b4 by Terry Jan Reedy in branch '2.7':
Issue bpo-18226: Add docstrings and unittests for idlelib/FormatParagraph.py.
http://hg.python.org/cpython/rev/453b4f89a2b4

New changeset bfdb687ca485 by Terry Jan Reedy in branch '3.3':
Issue bpo-18226: Add docstrings and unittests for idlelib/FormatParagraph.py.
http://hg.python.org/cpython/rev/bfdb687ca485

Unless there is a buildbot problem, further enhancements of either file will be part of bpo-18583

New changeset 47307e7c80e1 by Terry Jan Reedy in branch '2.7':
Issue bpo-18226: Fix ImportError and subsequent TypeError in 2.7 backport.
http://hg.python.org/cpython/rev/47307e7c80e1

test.regrtest is semi-useless, at least for idle, in 2.7 -- see commit message. First run single file directly or python -m test.test_idle.