From 69062abe0540c0afa0a398a6802b369a7637238c Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Wed, 6 Mar 2019 10:53:51 -0500 Subject: [PATCH 1/5] Add tests for grep findfiles. * Based on a patch by Al Sweigart. --- Lib/idlelib/idle_test/test_grep.py | 82 ++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/Lib/idlelib/idle_test/test_grep.py b/Lib/idlelib/idle_test/test_grep.py index ab0d7860f78953..d522b456154e5d 100644 --- a/Lib/idlelib/idle_test/test_grep.py +++ b/Lib/idlelib/idle_test/test_grep.py @@ -9,6 +9,7 @@ import unittest from test.support import captured_stdout from idlelib.idle_test.mock_tk import Var +import os.path import re @@ -38,11 +39,82 @@ def close(self): # gui method class FindfilesTest(unittest.TestCase): - # findfiles is really a function, not a method, could be iterator - # test that filename return filename - # test that idlelib has many .py files - # test that recursive flag adds idle_test .py files - pass + + @classmethod + def setUpClass(cls): + cls.realpath = os.path.realpath(__file__) + cls.path = os.path.dirname(cls.realpath) + + @classmethod + def tearDownClass(cls): + del cls.realpath, cls.path + + def test_invaliddir(self): + with captured_stdout() as s: + filelist = grep.findfiles('invaliddir', '*.*', False) + self.assertEqual(filelist, []) + self.assertIn('invalid', s.getvalue()) + + def test_default_dir(self): + ff = grep.findfiles + save_cwd = os.getcwd() + os.chdir(self.path) + filename = 'test_grep.py' + + # No value sent for directory defaults to os.curdir. + filelist = ff('', filename, False) + self.assertIn(filename, filelist) + os.chdir(save_cwd) + + def test_base(self): + ff = grep.findfiles + readme = os.path.join(self.path, 'README.txt') + + # Check for Python files in path where this file lives. + filelist = ff(self.path, '*.py', False) + # This directory has many Python files. + self.assertGreater(len(filelist), 10) + self.assertIn(self.realpath, filelist) + self.assertNotIn(readme, filelist) + + # Look for .txt files in path where this file lives. + filelist = ff(self.path, '*.txt', False) + self.assertNotEqual(len(filelist), 0) + self.assertNotIn(self.realpath, filelist) + self.assertIn(readme, filelist) + + # Look for non-matching pattern. + filelist = ff(self.path, 'grep.*', False) + self.assertEqual(len(filelist), 0) + self.assertNotIn(self.realpath, filelist) + + def test_recurse(self): + ff = grep.findfiles + parent = os.path.dirname(self.path) + grepfile = os.path.join(parent, 'grep.py') + pat = '*.py' + + # Get Python files only in parent directory. + filelist = ff(parent, pat, False) + parent_size = len(filelist) + # Lots of Python files in idlelib. + self.assertGreater(parent_size, 20) + self.assertIn(grepfile, filelist) + # Without subdirectories, this file isn't returned. + self.assertNotIn(self.realpath, filelist) + + # Include subdirectories. + filelist = ff(parent, pat, True) + # More files found now. + self.assertGreater(len(filelist), parent_size) + self.assertIn(grepfile, filelist) + # This file exists in list now. + self.assertIn(self.realpath, filelist) + + # Check another level up the tree. + parent = os.path.dirname(parent) + filelist = ff(parent, '*.py', True) + self.assertIn(self.realpath, filelist) class Grep_itTest(unittest.TestCase): From f8a5cb79b6c21d436c275214904326c33d508ce6 Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Wed, 6 Mar 2019 11:08:47 -0500 Subject: [PATCH 2/5] Move findfiles to module function. --- Lib/idlelib/grep.py | 51 +++++++++++++++--------------- Lib/idlelib/idle_test/test_grep.py | 11 +++---- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Lib/idlelib/grep.py b/Lib/idlelib/grep.py index 873233ec15439c..9d3250411d2864 100644 --- a/Lib/idlelib/grep.py +++ b/Lib/idlelib/grep.py @@ -36,6 +36,31 @@ def grep(text, io=None, flist=None): dialog.open(text, searchphrase, io) +def findfiles(dir, base, rec): + """Return list of files in the dir that match the base pattern. + + If rec is True, recursively iterate through subdirectories. + """ + try: + names = os.listdir(dir or os.curdir) + except OSError as msg: + print(msg) + return [] + list = [] + subdirs = [] + for name in names: + fn = os.path.join(dir, name) + if os.path.isdir(fn): + subdirs.append(fn) + else: + if fnmatch.fnmatch(name, base): + list.append(fn) + if rec: + for subdir in subdirs: + list.extend(findfiles(subdir, base, rec)) + return list + + class GrepDialog(SearchDialogBase): "Dialog for searching multiple files." @@ -121,7 +146,7 @@ def grep_it(self, prog, path): is an OutputWindow). """ dir, base = os.path.split(path) - list = self.findfiles(dir, base, self.recvar.get()) + list = findfiles(dir, base, self.recvar.get()) list.sort() self.close() pat = self.engine.getpat() @@ -146,30 +171,6 @@ def grep_it(self, prog, path): # so in OW.write, OW.text.insert fails. pass - def findfiles(self, dir, base, rec): - """Return list of files in the dir that match the base pattern. - - If rec is True, recursively iterate through subdirectories. - """ - try: - names = os.listdir(dir or os.curdir) - except OSError as msg: - print(msg) - return [] - list = [] - subdirs = [] - for name in names: - fn = os.path.join(dir, name) - if os.path.isdir(fn): - subdirs.append(fn) - else: - if fnmatch.fnmatch(name, base): - list.append(fn) - if rec: - for subdir in subdirs: - list.extend(self.findfiles(subdir, base, rec)) - return list - def _grep_dialog(parent): # htest # from tkinter import Toplevel, Text, SEL, END diff --git a/Lib/idlelib/idle_test/test_grep.py b/Lib/idlelib/idle_test/test_grep.py index d522b456154e5d..b20cc7a7d0a2b4 100644 --- a/Lib/idlelib/idle_test/test_grep.py +++ b/Lib/idlelib/idle_test/test_grep.py @@ -5,7 +5,7 @@ Otherwise, tests are mostly independent. Currently only test grep_it, coverage 51%. """ -from idlelib.grep import GrepDialog +from idlelib import grep import unittest from test.support import captured_stdout from idlelib.idle_test.mock_tk import Var @@ -27,15 +27,14 @@ def getpat(self): class Dummy_grep: # Methods tested #default_command = GrepDialog.default_command - grep_it = GrepDialog.grep_it - findfiles = GrepDialog.findfiles + grep_it = grep.GrepDialog.grep_it # Other stuff needed recvar = Var(False) engine = searchengine def close(self): # gui method pass -grep = Dummy_grep() +_grep = Dummy_grep() class FindfilesTest(unittest.TestCase): @@ -123,9 +122,9 @@ class Grep_itTest(unittest.TestCase): # from incomplete replacement, so 'later'. def report(self, pat): - grep.engine._pat = pat + _grep.engine._pat = pat with captured_stdout() as s: - grep.grep_it(re.compile(pat), __file__) + _grep.grep_it(re.compile(pat), __file__) lines = s.getvalue().split('\n') lines.pop() # remove bogus '' after last \n return lines From e69fb53f7d7449661564e57f36e79db5ef0f7739 Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Wed, 6 Mar 2019 13:48:01 -0500 Subject: [PATCH 3/5] Change findfile to use os.walk --- Lib/idlelib/grep.py | 47 ++++++++++++++---------------- Lib/idlelib/idle_test/test_grep.py | 25 ++++++++-------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/Lib/idlelib/grep.py b/Lib/idlelib/grep.py index 9d3250411d2864..9c778aa8867b60 100644 --- a/Lib/idlelib/grep.py +++ b/Lib/idlelib/grep.py @@ -36,29 +36,25 @@ def grep(text, io=None, flist=None): dialog.open(text, searchphrase, io) -def findfiles(dir, base, rec): - """Return list of files in the dir that match the base pattern. +def walk_error(msg): + "Handle os.walk error." + print(msg) - If rec is True, recursively iterate through subdirectories. + +def findfiles(folder, pattern, recursive): + """Generate file names in dir that match pattern. + + Args: + folder: Root directory to search. + pattern: File pattern to match. + recursive: True to include subdirectories. """ - try: - names = os.listdir(dir or os.curdir) - except OSError as msg: - print(msg) - return [] - list = [] - subdirs = [] - for name in names: - fn = os.path.join(dir, name) - if os.path.isdir(fn): - subdirs.append(fn) - else: - if fnmatch.fnmatch(name, base): - list.append(fn) - if rec: - for subdir in subdirs: - list.extend(findfiles(subdir, base, rec)) - return list + for dirpath, _, filenames in os.walk(folder, onerror=walk_error): + yield from (os.path.join(dirpath, name) + for name in filenames + if fnmatch.fnmatch(name, pattern)) + if not recursive: + break class GrepDialog(SearchDialogBase): @@ -145,15 +141,16 @@ def grep_it(self, prog, path): found, write the file and line information to stdout (which is an OutputWindow). """ - dir, base = os.path.split(path) - list = findfiles(dir, base, self.recvar.get()) - list.sort() + folder, filepat = os.path.split(path) + if not folder: + folder = os.curdir + filelist = sorted(findfiles(folder, filepat, self.recvar.get())) self.close() pat = self.engine.getpat() print(f"Searching {pat!r} in {path} ...") hits = 0 try: - for fn in list: + for fn in filelist: try: with open(fn, errors='replace') as f: for lineno, line in enumerate(f, 1): diff --git a/Lib/idlelib/idle_test/test_grep.py b/Lib/idlelib/idle_test/test_grep.py index b20cc7a7d0a2b4..a0b5b69171879c 100644 --- a/Lib/idlelib/idle_test/test_grep.py +++ b/Lib/idlelib/idle_test/test_grep.py @@ -9,7 +9,7 @@ import unittest from test.support import captured_stdout from idlelib.idle_test.mock_tk import Var -import os.path +import os import re @@ -50,19 +50,18 @@ def tearDownClass(cls): def test_invaliddir(self): with captured_stdout() as s: - filelist = grep.findfiles('invaliddir', '*.*', False) + filelist = list(grep.findfiles('invaliddir', '*.*', False)) self.assertEqual(filelist, []) self.assertIn('invalid', s.getvalue()) - def test_default_dir(self): + def test_curdir(self): + # Test os.curdir. ff = grep.findfiles save_cwd = os.getcwd() os.chdir(self.path) filename = 'test_grep.py' - - # No value sent for directory defaults to os.curdir. - filelist = ff('', filename, False) - self.assertIn(filename, filelist) + filelist = list(ff(os.curdir, filename, False)) + self.assertIn(os.path.join(os.curdir, filename), filelist) os.chdir(save_cwd) def test_base(self): @@ -70,20 +69,20 @@ def test_base(self): readme = os.path.join(self.path, 'README.txt') # Check for Python files in path where this file lives. - filelist = ff(self.path, '*.py', False) + filelist = list(ff(self.path, '*.py', False)) # This directory has many Python files. self.assertGreater(len(filelist), 10) self.assertIn(self.realpath, filelist) self.assertNotIn(readme, filelist) # Look for .txt files in path where this file lives. - filelist = ff(self.path, '*.txt', False) + filelist = list(ff(self.path, '*.txt', False)) self.assertNotEqual(len(filelist), 0) self.assertNotIn(self.realpath, filelist) self.assertIn(readme, filelist) # Look for non-matching pattern. - filelist = ff(self.path, 'grep.*', False) + filelist = list(ff(self.path, 'grep.*', False)) self.assertEqual(len(filelist), 0) self.assertNotIn(self.realpath, filelist) @@ -94,7 +93,7 @@ def test_recurse(self): pat = '*.py' # Get Python files only in parent directory. - filelist = ff(parent, pat, False) + filelist = list(ff(parent, pat, False)) parent_size = len(filelist) # Lots of Python files in idlelib. self.assertGreater(parent_size, 20) @@ -103,7 +102,7 @@ def test_recurse(self): self.assertNotIn(self.realpath, filelist) # Include subdirectories. - filelist = ff(parent, pat, True) + filelist = list(ff(parent, pat, True)) # More files found now. self.assertGreater(len(filelist), parent_size) self.assertIn(grepfile, filelist) @@ -112,7 +111,7 @@ def test_recurse(self): # Check another level up the tree. parent = os.path.dirname(parent) - filelist = ff(parent, '*.py', True) + filelist = list(ff(parent, '*.py', True)) self.assertIn(self.realpath, filelist) From 77a07bb13ef2146ecfc2d2c5dd3c9ee8f279d0c7 Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Wed, 6 Mar 2019 14:48:09 -0500 Subject: [PATCH 4/5] Add blurb --- Misc/NEWS.d/next/IDLE/2019-03-06-14-47-57.bpo-23205.Vv0gfH.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/IDLE/2019-03-06-14-47-57.bpo-23205.Vv0gfH.rst diff --git a/Misc/NEWS.d/next/IDLE/2019-03-06-14-47-57.bpo-23205.Vv0gfH.rst b/Misc/NEWS.d/next/IDLE/2019-03-06-14-47-57.bpo-23205.Vv0gfH.rst new file mode 100644 index 00000000000000..9e7c222ffc456c --- /dev/null +++ b/Misc/NEWS.d/next/IDLE/2019-03-06-14-47-57.bpo-23205.Vv0gfH.rst @@ -0,0 +1,2 @@ +For the grep module, add tests for findfiles, refactor findfiles to be a +module-level function, and refactor findfiles to use os.walk. From 1955a64040897a4b2788e0bf30c05c1f7dec2c3c Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Sat, 23 Mar 2019 07:11:47 -0400 Subject: [PATCH 5/5] Fix indentation of docstring. --- Lib/idlelib/grep.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/idlelib/grep.py b/Lib/idlelib/grep.py index a82f446f90c7b3..12513594b76f8f 100644 --- a/Lib/idlelib/grep.py +++ b/Lib/idlelib/grep.py @@ -49,9 +49,9 @@ def findfiles(folder, pattern, recursive): """Generate file names in dir that match pattern. Args: - folder: Root directory to search. - pattern: File pattern to match. - recursive: True to include subdirectories. + folder: Root directory to search. + pattern: File pattern to match. + recursive: True to include subdirectories. """ for dirpath, _, filenames in os.walk(folder, onerror=walk_error): yield from (os.path.join(dirpath, name)