bpo-32100: IDLE: Fix pathbrowser errors; improve tests. (#4484)

Patch mostly by Cheryl Sabella

Lib/idlelib/browser.py

@@ -79,9 +79,6 @@ class ModuleBrowser:
                 creating ModuleBrowserTreeItem as the rootnode for
                 the tree and subsequently in the children.
         """
-        global file_open
-        if not (_htest or _utest):
-            file_open = pyshell.flist.open
         self.master = master
         self.path = path
         self._htest = _htest
@@ -95,9 +92,13 @@ class ModuleBrowser:
 
     def init(self):
         "Create browser tkinter widgets, including the tree."
+        global file_open
         root = self.master
-        # reset pyclbr
+        flist = (pyshell.flist if not (self._htest or self._utest)
+                 else pyshell.PyShellFileList(root))
+        file_open = flist.open
         pyclbr._modules.clear()
+
         # create top
         self.top = top = ListedToplevel(root)
         top.protocol("WM_DELETE_WINDOW", self.close)
@@ -107,6 +108,7 @@ class ModuleBrowser:
                 (root.winfo_rootx(), root.winfo_rooty() + 200))
         self.settitle()
         top.focus_set()
+
         # create scrolled canvas
         theme = idleConf.CurrentTheme()
         background = idleConf.GetHighlight(theme, 'normal')['background']
@@ -236,8 +238,6 @@ def _module_browser(parent): # htest #
             def nested_in_class(): pass
         def closure():
             class Nested_in_closure: pass
-    global file_open
-    file_open = pyshell.PyShellFileList(parent).open
     ModuleBrowser(parent, file, _htest=True)
 
 if __name__ == "__main__":

Lib/idlelib/editor.py

@@ -668,7 +668,7 @@ class EditorWindow(object):
 
     def open_path_browser(self, event=None):
         from idlelib import pathbrowser
-        pathbrowser.PathBrowser(self.flist)
+        pathbrowser.PathBrowser(self.root)
         return "break"
 
     def open_turtle_demo(self, event = None):

Lib/idlelib/idle_test/test_browser.py

@@ -4,17 +4,19 @@ Coverage: 88%
 (Higher, because should exclude 3 lines that .coveragerc won't exclude.)
 """
 
-import os.path
-import unittest
-import pyclbr
-
-from idlelib import browser, filelist
-from idlelib.tree import TreeNode
-from test.support import requires
-from unittest import mock
-from tkinter import Tk
-from idlelib.idle_test.mock_idle import Func
 from collections import deque
+import os.path
+import pyclbr
+from tkinter import Tk
+
+from test.support import requires
+import unittest
+from unittest import mock
+from idlelib.idle_test.mock_idle import Func
+
+from idlelib import browser
+from idlelib import filelist
+from idlelib.tree import TreeNode
 
 
 class ModuleBrowserTest(unittest.TestCase):
@@ -29,6 +31,7 @@ class ModuleBrowserTest(unittest.TestCase):
     @classmethod
     def tearDownClass(cls):
         cls.mb.close()
+        cls.root.update_idletasks()
         cls.root.destroy()
         del cls.root, cls.mb
 
@@ -38,6 +41,7 @@ class ModuleBrowserTest(unittest.TestCase):
         eq(mb.path, __file__)
         eq(pyclbr._modules, {})
         self.assertIsInstance(mb.node, TreeNode)
+        self.assertIsNotNone(browser.file_open)
 
     def test_settitle(self):
         mb = self.mb
@@ -151,10 +155,9 @@ class ModuleBrowserTreeItemTest(unittest.TestCase):
         self.assertEqual(sub0.name, 'f0')
         self.assertEqual(sub1.name, 'C0(base)')
 
-
-    def test_ondoubleclick(self):
+    @mock.patch('idlelib.browser.file_open')
+    def test_ondoubleclick(self, fopen):
         mbt = self.mbt
-        fopen = browser.file_open = mock.Mock()
 
         with mock.patch('os.path.exists', return_value=False):
             mbt.OnDoubleClick()
@@ -165,8 +168,6 @@ class ModuleBrowserTreeItemTest(unittest.TestCase):
             fopen.assert_called()
             fopen.called_with(fname)
 
-        del browser.file_open
-
 
 class ChildBrowserTreeItemTest(unittest.TestCase):
 
@@ -212,14 +213,13 @@ class ChildBrowserTreeItemTest(unittest.TestCase):
 
         eq(self.cbt_F1.GetSubList(), [])
 
-    def test_ondoubleclick(self):
-        fopen = browser.file_open = mock.Mock()
+    @mock.patch('idlelib.browser.file_open')
+    def test_ondoubleclick(self, fopen):
         goto = fopen.return_value.gotoline = mock.Mock()
         self.cbt_F1.OnDoubleClick()
         fopen.assert_called()
         goto.assert_called()
         goto.assert_called_with(self.cbt_F1.obj.lineno)
-        del browser.file_open
         # Failure test would have to raise OSError or AttributeError.
 
 

Lib/idlelib/idle_test/test_pathbrowser.py

@@ -1,11 +1,68 @@
+""" Test idlelib.pathbrowser.
+"""
+
+
+import os.path
+import pyclbr  # for _modules
+import sys  # for sys.path
+from tkinter import Tk
+
+from test.support import requires
 import unittest
-import os
-import sys
-import idlelib
+from idlelib.idle_test.mock_idle import Func
+
+import idlelib  # for __file__
+from idlelib import browser
 from idlelib import pathbrowser
+from idlelib.tree import TreeNode
+
 
 class PathBrowserTest(unittest.TestCase):
 
+    @classmethod
+    def setUpClass(cls):
+        requires('gui')
+        cls.root = Tk()
+        cls.root.withdraw()
+        cls.pb = pathbrowser.PathBrowser(cls.root, _utest=True)
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.pb.close()
+        cls.root.update_idletasks()
+        cls.root.destroy()
+        del cls.root, cls.pb
+
+    def test_init(self):
+        pb = self.pb
+        eq = self.assertEqual
+        eq(pb.master, self.root)
+        eq(pyclbr._modules, {})
+        self.assertIsInstance(pb.node, TreeNode)
+        self.assertIsNotNone(browser.file_open)
+
+    def test_settitle(self):
+        pb = self.pb
+        self.assertEqual(pb.top.title(), 'Path Browser')
+        self.assertEqual(pb.top.iconname(), 'Path Browser')
+
+    def test_rootnode(self):
+        pb = self.pb
+        rn = pb.rootnode()
+        self.assertIsInstance(rn, pathbrowser.PathBrowserTreeItem)
+
+    def test_close(self):
+        pb = self.pb
+        pb.top.destroy = Func()
+        pb.node.destroy = Func()
+        pb.close()
+        self.assertTrue(pb.top.destroy.called)
+        self.assertTrue(pb.node.destroy.called)
+        del pb.top.destroy, pb.node.destroy
+
+
+class DirBrowserTreeItemTest(unittest.TestCase):
+
     def test_DirBrowserTreeItem(self):
         # Issue16226 - make sure that getting a sublist works
         d = pathbrowser.DirBrowserTreeItem('')
@@ -16,6 +73,9 @@ class PathBrowserTest(unittest.TestCase):
         self.assertEqual(d.ispackagedir(dir), True)
         self.assertEqual(d.ispackagedir(dir + '/Icons'), False)
 
+
+class PathBrowserTreeItemTest(unittest.TestCase):
+
     def test_PathBrowserTreeItem(self):
         p = pathbrowser.PathBrowserTreeItem()
         self.assertEqual(p.GetText(), 'sys.path')
@@ -23,5 +83,6 @@ class PathBrowserTest(unittest.TestCase):
         self.assertEqual(len(sub), len(sys.path))
         self.assertEqual(type(sub[0]), pathbrowser.DirBrowserTreeItem)
 
+
 if __name__ == '__main__':
     unittest.main(verbosity=2, exit=False)

Lib/idlelib/pathbrowser.py

@@ -9,13 +9,14 @@ from idlelib.tree import TreeItem
 
 class PathBrowser(ModuleBrowser):
 
-    def __init__(self, flist, *, _htest=False, _utest=False):
+    def __init__(self, master, *, _htest=False, _utest=False):
         """
         _htest - bool, change box location when running htest
         """
+        self.master = master
         self._htest = _htest
         self._utest = _utest
-        self.init(flist)
+        self.init()
 
     def settitle(self):
         "Set window titles."
@@ -100,8 +101,7 @@ class DirBrowserTreeItem(TreeItem):
 
 
 def _path_browser(parent):  # htest #
-    flist = PyShellFileList(parent)
-    PathBrowser(flist, _htest=True)
+    PathBrowser(parent, _htest=True)
     parent.mainloop()
 
 if __name__ == "__main__":

Misc/NEWS.d/next/IDLE/2017-11-21-08-26-08.bpo-32100.P43qx2.rst

@@ -0,0 +1,2 @@
+IDLE: Fix old and new bugs in pathbrowser; improve tests.
+Patch mostly by Cheryl Sabella.