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: improve idle_test.htest #65676

Closed
terryjreedy opened this issue May 12, 2014 · 24 comments
Closed

Idle: improve idle_test.htest #65676

terryjreedy opened this issue May 12, 2014 · 24 comments
Assignees
Labels
topic-IDLE type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 21477
Nosy @terryjreedy, @ned-deily
Superseder
  • bpo-21624: Idle: Improve htests
  • Files
  • 21477-htest.txt: renamed and copied from 18104
  • week1.diff
  • run-runall.diff
  • run-runall-34.diff
  • run-runall-27.diff
  • htest-25052014.diff
  • htest-25052014-34.diff
  • htest-25052014-27.diff: Corresponding patch for 2.7 for test-25052014-34.diff
  • htest-26052014-34.diff
  • htest-26052014-27.diff
  • htest-28052014-34.diff
  • htest-28052014-27.diff
  • htest-docstring-34.diff: Docstring change to reflect change in code.
  • htest-docstring-27.diff: Docstring change to reflect change in code.
  • 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 2014-06-01.04:44:08.199>
    created_at = <Date 2014-05-12.04:08:37.972>
    labels = ['expert-IDLE', 'type-feature']
    title = 'Idle: improve idle_test.htest'
    updated_at = <Date 2019-03-23.20:08:32.581>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2019-03-23.20:08:32.581>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2014-06-01.04:44:08.199>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2014-05-12.04:08:37.972>
    creator = 'terry.reedy'
    dependencies = []
    files = ['35221', '35334', '35335', '35342', '35343', '35352', '35356', '35364', '35368', '35369', '35386', '35387', '35411', '35412']
    hgrepos = []
    issue_num = 21477
    keywords = ['patch']
    message_count = 24.0
    messages = ['218312', '218830', '218839', '219021', '219055', '219060', '219061', '219066', '219096', '219107', '219108', '219109', '219161', '219190', '219205', '219207', '219209', '219210', '219292', '219335', '219336', '219484', '219487', '219535']
    nosy_count = 5.0
    nosy_names = ['terry.reedy', 'ned.deily', 'jesstess', 'python-dev', 'Saimadhav.Heblikar']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = '21624'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21477'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @terryjreedy
    Copy link
    Member Author

    bpo-18104 created idle_test/htest.py. Besides adding more tests in other issues, some possible improvements to htest itself follow. Many depend on not reusing run, as it now it, in the runall loop. What is left in common to both might be factored out.

    • For runall, reuse the same master box so the user does not have to click away each master box to see another (which also appears in a different position). Instead, rewrite the message and if still used, the button.

    • Make sure tested widgets do not cover the master box. For configSectionNameDialog, an htest parameter was added to the class to change the usual positioning (which was intentionally centered over the parent).

    • For most tests, eliminate the button and simply display the widget. The button is needed for testing message boxes when one wants to test different entries (the section name dialog). 'button=True' could be added to the spec where needed.

    • If there is no button to restart a test, then closing the widget should, if possible, close the master. The editor window does this, but that would have to be changed if runall ran in one root.

    • The pre-existing editor window test hides (withdraws) the master (root) window. This is not necessary and should be changed.

    @terryjreedy terryjreedy self-assigned this May 12, 2014
    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label May 12, 2014
    @terryjreedy terryjreedy changed the title Idle: improve idle_test,htest Idle: improve idle_test.htest May 16, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 20, 2014

    New changeset 33a39dfc239e by Terry Jan Reedy in branch '2.7':
    Issue bpo-21477: idle htests - lower case function names, other cleanups.
    http://hg.python.org/cpython/rev/33a39dfc239e

    New changeset 7c70198ec48e by Terry Jan Reedy in branch '3.4':
    Issue bpo-21477: idle htests - lower case function names, other cleanups.
    http://hg.python.org/cpython/rev/7c70198ec48e

    @SaimadhavHeblikar
    Copy link
    Mannequin

    SaimadhavHeblikar mannequin commented May 20, 2014

    Adds spec dicts for aboutDialog, ClassBrowser, PathBrowser, textView and configHelpSourceEdit and modifies the related modules appropriately.
    The spec dicts(and the tests relating to) for editorwindow and help dialog have not been modified in this patch.

    The tests work OK when run individually from respective modules. The widgets are placed below parent. Some widgets close the parent when they are closed, some do not.(I feel the parent should be manually closed by the user, because the user might want to re-run a test. This would also help in reusing root, when running all the tests together).

    The runall() seems shaky at the moment. For eg, as mentioned above, some widgets close the parent when they are closed, some do not. This is causes build-up of parent dialogs. I did not try to fix it, because things will change once we agree upon factoring out run() and runall(), about reusing the same root window for runall(). If things dont change, I will work on fixing the buildup of dialogs.

    If this patch seems "incomplete" because of the above issue, please provide feedback about the tests itself, when run individually.

    @SaimadhavHeblikar
    Copy link
    Mannequin

    SaimadhavHeblikar mannequin commented May 24, 2014

    Posting a cumulative patch of all htest written so far.

    They include IOBinding, Tooltips, MultiStatusbar, tabbedpages, objectbrowser,
    scrolledlist, dynOptionWidget, treewidget, widgetredirector,
    colordelegator, calltip and multicall, besides aboutDialog, ClassBrowser, PathBrowser, textView and configHelpSourceEdit already posted before.
    This patch is named week1.diff

    *Improving run/runall - (run-runall.diff)
    This patch concerns more with the way run/runall are executed. The only modification for this occurs in the file htest.py
    I have streamlined run/runall into a single function. The behavior is same as the current htest.py, except for the reuse of a single root window and the addition of a "next" button.

    The logic behind it is contained in line 206 to line 223 of htest.py when run-runall.diff is applied.
    In short, a list of tuple containing(spec, kwds and callable object) is used to store the required information. Global variables maintain the above information only for the current test. Once a user finishes a test, the values of the global variables are updated to reflect the values for the next test. The "next" button is disabled for the case when a) it is the last test 2) when run() is called from a module

    If this approach is desired, please say so. I will work on improving the responsiveness of the root window(Constant size of root dialog, ensuring buttons, labels etc stay in the "same place" and dont "jump" around).

    @terryjreedy
    Copy link
    Member Author

    run-runall.diff import cleanly to 3.4. Running all the tests works well enough that I am applying this, with minor changes, to be a base for further patches. Good job.

    The few problems I fixed:

    1. AutoCompleteWindow.py had \n added at the beginning. I just deleted it.
    2. 2 files had an unused 'import tkinter'. I deleted those lines. (Any new imports, that are used, should be 'import tkinter as tk' so that is the only place where the Tkinter/tkinter change shows up in the Idle file.)
    3. patchcheck found 7 files with extra whitespace to be deleted.

    Things to do starting from this.

    1. See review comments on htest.run().

    2. See review comments on text changes.

    3. See review comments on individual tests. There will be more of these, but run changes and 4) below are the most important now.

    4. Many wrappers create a separate root=Tk() (and mainloop) ins4tead of reusing the parent arg. The result is that clicking [Next] does not delete the test window, as it is not a child of the main window. The test window has to be closed separately. So unless the test *requires* a new root and mainloop, please delete them. (Modal dialogs have to be closed explicitly - message should say so.)

    5. When I changed root = tk.Tk() to root = parent in _editor_window, the test seemed to continue to work fine. Why did you comment it out? Please restore it and make any change necessary to _editor_window to work with the revised run().

    @terryjreedy
    Copy link
    Member Author

    Since I wanted to get this large patch applied as a basis for further work, I went ahead with the 2.7 backport. I am listing the issues in converting run-runall-34.diff to run-runall-27.diff to help Saimadhav do this in the future.

    1. /tkinter/Tkinter/ 7 places (easy).

    2. /print(xxx)/print xxx/ 3 places in 2 idlelib files,
      but leave print(widget.result) alone in htest

    3. /importlib.machinery/imp/ in Pathbrowser.py

    4. In textView.py, the 2.7 line
      text = file(filename, 'r').read()
      was changed in 3.x to two lines:
      with open(filename, 'r') as f:
      text = f.read()
      Since inserting or deleting a line in a diff file (as opposed to the within-line edits above) is tricky, and since this line is part of a chunk being deleted, I left this alone in the -27.diff resulting from the changes above. The diff uploaded included the handpatch changes.

    (Note: some time ago, I converted all open statements to 'with open' in 2.7 and 3.x. I did not think of 'file(' as a synonym for 'open('. Any other uses of 'file(' should be converted also.)

    1. The surprise I had forgotten about. 'nonlocal', used in the new version of run(), is new in 3.0! Fortunately, I remember the ugly hack we used in 2.x. It does mean, however, that the run code is different in 2.7. Future patches to run should only patch run().

    2. ColorDelegator: the code in the 3.x test is displayed for me as
      �ó
      ��チSc
      I replaced source with a string literal. I think we should use a revised version of this, with everything that should be colored, in 3.x also and skip the file read.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 24, 2014

    New changeset ece24bcd1a6f by Terry Jan Reedy in branch '2.7':
    Issue bpo-21477: Idle htest: merge and modify run and runall; add many tests.
    http://hg.python.org/cpython/rev/ece24bcd1a6f

    New changeset 038cbbef4539 by Terry Jan Reedy in branch '3.4':
    Issue bpo-21477: Idle htest: merge and modify run and runall; add many tests.
    http://hg.python.org/cpython/rev/038cbbef4539

    @terryjreedy
    Copy link
    Member Author

    I just discovered today that Rietveld can do diffs between diffs, so to speak. For example,
    http://bugs.python.org/review/21477/diff2/11941:11942/Lib/idlelib/idle_test/htest.py
    showed the changes between patched htest in 3.4 and 2.7.

    @SaimadhavHeblikar
    Copy link
    Mannequin

    SaimadhavHeblikar mannequin commented May 25, 2014

    Modifications in htest-25052014.diff

    1. ClassBrowser, PathBrowser, EditorWindow no longer close parent when closed

    2. Sample code in _color_delegator changed to string, instead of reading from file.

    3. String text change for Tooltip.

    4. Adds htest for FormatParagraph, Percolator, EditorWindow(rather uncomment's spec), StackViewer, KeyBinding

    5. Modification to run based on review comments at http://bugs.python.org/review/21477/diff/11937/Lib/idlelib/idle_test/htest.py

    6. Other cosmetic changes to spec string 'msg' text.

    When this diff(subject to passing review and feedback) is pushed, I will make a corresponding patch for 2.7

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 25, 2014

    New changeset d7eea8f608c2 by Terry Jan Reedy in branch '3.4':
    Issue bpo-21477: Idle htest: modify run; add more tests.
    http://hg.python.org/cpython/rev/d7eea8f608c2

    @terryjreedy
    Copy link
    Member Author

    1. I pushed a slight revision of the patch. See 2) for the main change. In htest, I also did 3), corrected some line spacing issues, and changed 'focussing' to the US spelling 'focusing'. A tested 2.7 version of the attached, htest-25052014-34.py, is top priority.

    2. parent.mainloop() - ClassBrower, EditorWindow, PathBrowser: since mainloop is already running, this should have no effect. I had no problem with closing of child window closing parent. Perhaps there is an OS difference, perhaps related to the focus difference. Or this might even be a tk or tkinter bug in not masking OS differences. If the line is necessary for #nix, it seems harmless on Windows.

    3. Let us remember that the purpose of htest is to test what cannot be sensibly tested with automated unittests. That includes the visual look of widgets and some behaviors that either cannot be automatically tested or that we currently do not know how to test.

    The format paragragh methods and functions are well tested, better than a human will, in the current test_formatgraph unittest. I removed this part of the patch. (But see 5. below about moving it.)

    When I wrote the notes that became htest.txt, a year ago, I had not written any tests other than for calltips. I listed almost all the files that had existig tests. So don't add new tests not mentioned in htest.txt without discussion. And question the existing and proposed tests listed there.

    For instance, I presume we can use the Text.tag_methods to write an automated ColorDelegator test that checks that tagged words are as expected and that colors match the configured values. We can leave the current test (with patch), until replaced.

    1. Class/PathBrowser: not only does double clicking 'not work', it causes an exception to be printed to console or shell, but which seems to be ignored as test continues. The exception is either like this
    Exception in Tkinter callback
    Traceback (most recent call last):
      File "F:\Python\dev\4\py34\lib\idlelib\run.py", line 121, in main
        seq, request = rpc.request_queue.get(block=True, timeout=0.05)
      File "F:\Python\dev\4\py34\lib\queue.py", line 175, in get
        raise Empty
    queue.Empty
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "F:\Python\dev\4\py34\lib\tkinter\__init__.py", line 1487, in __call__
        return self.func(*args)
      File "F:\Python\dev\4\py34\lib\idlelib\TreeWidget.py", line 122, in flip
        self.item.OnDoubleClick()
      File "F:\Python\dev\4\py34\Lib\idlelib\ClassBrowser.py", line 174, in OnDoubleClick
        edit = PyShell.flist.open(self.file)
    AttributeError: 'module' object has no attribute 'flist'

    or with just the AttributeError. Unless and until we can suppress the traceback, the message should be changed from 'not work' to 'will print a traceback for an exception that is ignored'. [done]

    1. Please comment on point 4 in msg219055.

    2. Possible followup idea: By reviewing and running tests for classes I have not previously paid attention to, I learned something about them. After htest is done, polish the test explanations as appropriate to explain each class to someone not familiar with it. Revise run() to start as follows:

    def run(*tests, *, test_list=None):
        if test_list is None: test_list = []

    Add htour.py (for instance) with a callable and spec for non-htested classes (and behaviors). It could begin like this
    ---

    from idlelib.idle_test.htest import run
    
    def _format_paragraph(parent): ...(such as in current patch)

    _format_paragraph_spec = ...(such as in current patch)

    tour_list = [(_format_paragraph, _format_paragraph_spec),]
    # automate by scanning globals for callables and grabbing matching specs
    
    run(test_list=tour_list)

    This should be a separate issue that follows on this one. If you feel like expanding the above to a preliminary patch, go ahead.

    1. For me, many of the htest geometry increments are too small. Example:
      root.geometry("+%d+%d"%(x, y + 150) # percolator

    @terryjreedy
    Copy link
    Member Author

    Yes, remove unscrollable (test2) tree widget (review response).

    @SaimadhavHeblikar
    Copy link
    Mannequin

    SaimadhavHeblikar mannequin commented May 26, 2014

    Summary for htest-26052014-34.diff and htest-26052014-27.diff

    1. Adds htest for ReplaceDialog and SearchDialog
    2. Removes the two canvases in TreeWidget as per code review comment. Now there is only a single ScrollableCanvas
    3. Some text changes in spec dict messages

    The corresponding 27 patch is to be applied on top of htest-25052014-27.diff.

    Apart,

    1. Wrt point 4 in msg219055, I think wrapper functions are required along with creating a new root.
      It ensures the parent dialog is not destroyed, even 'accidently' and across different OS.
      About the concern that clicking '[Next]' does not destroy the child:
      We could have the following line in wrapper function(which creates a new root)-

      parent.child = root

    and, in the "next()" in run():
    try:
    root.child.destroy()
    except AttributeError:
    pass

    To make things more readable, we could perhaps change the 'root' in run() to 'parent' and ensure consistency.
    With this, clicking '[Next]' destroys the child, before moving onto the next htest.
    A drawback with this approach would be that, we would need to create a wrapper function for those tests which currently dont have one(See: configNameDialog, configHelpSource, GetKeysDialog etc.)

    1. Htest's for GrepDialog, outputwindow, configDialog and Filelist are not progressing because of assert statements in macosxsupport.py.
      I do not know the reasoning behind the assert statement either. Neither do I know how to 'ignore' it.
      One way I found out is using the -O flag, but it is not pragmatic.
      Another way is to have a macosxsupport._tk_type = "other" in the respective wrapper function.
      But I do not know the effect of such a statement in a OSX environment.
      If someone with a OSX environment wants to volunteer, please try running Lib/idlelib/configDialog.py with and without the insertion
      macosxsupport._tk_type = "other". With the insertion, the configDialog works on a Linux gnome environment and doesn't without it.
      Funnily enough, the configDialog works as-is in Win7 which I tried on a VM.
      One way would be to insert the above statment only if its a *nix environment.

    @ned-deily
    Copy link
    Member

    "2. Htest's for GrepDialog, outputwindow, configDialog and Filelist are not progressing because of assert statements in macosxsupport.py."

    The asserts are to ensure that none of the Tk-variant tests (isCarbonTk(), isCocoaTk(), et al) are called before _initializeTkVariantTests(root) has been called. And _initializeTkVariantTests can only be called after the initial calls to Tk have been made to create an initial window environment; it's only at that point that we can call Tk to enquire what variant of Tk we are running under (macosxSupport.py:28 and 33). _initializeTkVariantTests is called from macosxSupport.setupApp which is called from PyShell.main. This is fine for normal IDLE startup but it doesn't take into account the module level tests. Ideally, you do want to have run _initializeTkVariantTests before running GUI tests so that the behavior is properly conditioned on Tk variant type, so stubbing out the Tk type with macosxsupport._tk_type = "other' is not desirable. Perhaps the best way to deal with it is to create a common GUI test initialization function that is called by all of the GUI tests; the initialization function would be responsible for creating the Tk() root, calling _initializeTkVariantTests, and any other common steps that might arise.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 27, 2014

    New changeset 72a8a107eed1 by Terry Jan Reedy in branch '2.7':
    Issue bpo-21477: Idle htest: modify run; add more tests.
    http://hg.python.org/cpython/rev/72a8a107eed1

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 27, 2014

    New changeset e770d8c4291c by Terry Jan Reedy in branch '2.7':
    Issue bpo-21477: Add htests for Search and Replace dialogs.
    http://hg.python.org/cpython/rev/e770d8c4291c

    New changeset b8e4bb1e1090 by Terry Jan Reedy in branch '3.4':
    Issue bpo-21477: Add htests for Search and Replace dialogs.
    http://hg.python.org/cpython/rev/b8e4bb1e1090

    @terryjreedy
    Copy link
    Member Author

    I think it time to make running through all or just some of the tests more pleasant. If we reversed the order [next], [test name], test instruction, then [next] would not just up and down. If the window were set to a constant width, then it would not jump back and forth either. The width should be just wide enough for the longest lines we use in htest.py, or can we just allow them to wrap?

    To select just a one or a few tests (like just Search, Replace, and Tree for the 2605.. patch), a drop down list box would be nice. It could be beside the next box. With a constant width, there would be room.

    Doing the test reminded me that Search (find) and Replace have a severe bug on Windows of not highlighting the the found text while the box is still open. There was an issue where this was supposedly fixed, but it is not. Find is usable by closing the box and using cntl-G for find next. But this does not work for replace, making replace useless on windows unless one is really sure about replace all.

    @terryjreedy
    Copy link
    Member Author

    Ned, I recently saw that some of the builtin extensions call macosxsupport. I though then that it would be better if such calls were somehow handled automatically. I have no idea how and when to make them and so I don't know if the existing calls are needed or if other calls should be added. So I like your idea. It seems like it should be a separate issue.

    @SaimadhavHeblikar
    Copy link
    Mannequin

    SaimadhavHeblikar mannequin commented May 28, 2014

    Summary for
    htest-28052014-34.diff and htest-28052014-27.diff

    1. Add htest for GrepDialog,UndoDelegator and configDialog
    2. Makes changes to the way the help string is displayed. The label has been replaced by a text widget made to look like a label, and also scrollable. The result is that the htest root dialog stays in the same place and same size throughout.(See the code for more information).
    3. Some minor change to spec dict strings and in ObjectBrowser for child dialog placement.

    With this, it leaves AutoCompleteWindow, TkMessageBoxes, Debugger(from 21477-htest.txt) to be tested.

    OutputWindow - already being tested from GrepDialog. There is nothing extra to be tested separately.

    FileList - Is it already being tested through EditorWindow?
    In case there is a need to test them, please say so. The code is ready.

    RemoteDebugger and RemoteObjectBrowser - I need some input on how to begin with them.

    Next: Apart from the above htest, a way to destroy the child when user clicks 'next', for certain modules like ClassBrowser, which don't work by the method of http://bugs.python.org/msg219161

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 29, 2014

    New changeset 90dab7696e89 by Terry Jan Reedy in branch '2.7':
    Issue bpo-21477: Add htests for GrepDialog, UndoDelegator, and configDialog.
    http://hg.python.org/cpython/rev/90dab7696e89

    New changeset d90905960803 by Terry Jan Reedy in branch '3.4':
    Issue bpo-21477: Add htests for GrepDialog, UndoDelegator, and configDialog.
    http://hg.python.org/cpython/rev/d90905960803

    @terryjreedy
    Copy link
    Member Author

    Ned, this patch includes a call to macosxSupport. _initializeTkVariantTests(root) in htest.run. Does this work on mac or is something else needed (like doing the same for individual tests that create another root)?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 1, 2014

    New changeset 334b6725b2f7 by Terry Jan Reedy in branch '2.7':
    Issue bpo-21477: Update htest docstring and remove extraneous differences between
    http://hg.python.org/cpython/rev/334b6725b2f7

    New changeset e56c3585ea80 by Terry Jan Reedy in branch '3.4':
    Issue bpo-21477: Update htest docstring and remove extraneous differences between
    http://hg.python.org/cpython/rev/e56c3585ea80

    @terryjreedy
    Copy link
    Member Author

    I regard the goal of this issue as having been accomplished. I opened bpo-21624 for any further work on htests.

    @ned-deily
    Copy link
    Member

    "this patch includes a call to macosxSupport. _initializeTkVariantTests(root) in htest.run. Does this work on mac or is something else needed (like doing the same for individual tests that create another root)?"

    That looks good and, in a quick spot check running a few tests, seems to work fine.

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

    No branches or pull requests

    2 participants