Skip to content

Commit d60f658

Browse files
csabellacsabella
authored
bpo-23205: IDLE: Add tests and refactor grep's findfiles (GH-12203)
* Add tests for grep findfiles. * Move findfiles to module function. * Change findfiles to use os.walk. Based on a patch by Al Sweigart.
1 parent 6d5ee97 commit d60f658

3 files changed

Lines changed: 109 additions & 45 deletions

File tree

‎Lib/idlelib/grep.py‎

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,27 @@ def grep(text, io=None, flist=None):
4040
dialog.open(text, searchphrase, io)
4141

4242

43+
def walk_error(msg):
44+
"Handle os.walk error."
45+
print(msg)
46+
47+
48+
def findfiles(folder, pattern, recursive):
49+
"""Generate file names in dir that match pattern.
50+
51+
Args:
52+
folder: Root directory to search.
53+
pattern: File pattern to match.
54+
recursive: True to include subdirectories.
55+
"""
56+
for dirpath, _, filenames in os.walk(folder, onerror=walk_error):
57+
yield from (os.path.join(dirpath, name)
58+
for name in filenames
59+
if fnmatch.fnmatch(name, pattern))
60+
if not recursive:
61+
break
62+
63+
4364
class GrepDialog(SearchDialogBase):
4465
"Dialog for searching multiple files."
4566

@@ -140,15 +161,16 @@ def grep_it(self, prog, path):
140161
prog: The compiled, cooked search pattern.
141162
path: String containing the search path.
142163
"""
143-
dir, base = os.path.split(path)
144-
list = self.findfiles(dir, base, self.recvar.get())
145-
list.sort()
164+
folder, filepat = os.path.split(path)
165+
if not folder:
166+
folder = os.curdir
167+
filelist = sorted(findfiles(folder, filepat, self.recvar.get()))
146168
self.close()
147169
pat = self.engine.getpat()
148170
print(f"Searching {pat!r} in {path} ...")
149171
hits = 0
150172
try:
151-
for fn in list:
173+
for fn in filelist:
152174
try:
153175
with open(fn, errors='replace') as f:
154176
for lineno, line in enumerate(f, 1):
@@ -166,36 +188,6 @@ def grep_it(self, prog, path):
166188
# so in OW.write, OW.text.insert fails.
167189
pass
168190

169-
def findfiles(self, dir, base, rec):
170-
"""Return list of files in the dir that match the base pattern.
171-
172-
Use the current directory if dir has no value.
173-
If rec is True, recursively iterate through subdirectories.
174-
175-
Args:
176-
dir: Directory path to search.
177-
base: File search pattern.
178-
rec: Boolean for recursive search through subdirectories.
179-
"""
180-
try:
181-
names = os.listdir(dir or os.curdir)
182-
except OSError as msg:
183-
print(msg)
184-
return []
185-
list = []
186-
subdirs = []
187-
for name in names:
188-
fn = os.path.join(dir, name)
189-
if os.path.isdir(fn):
190-
subdirs.append(fn)
191-
else:
192-
if fnmatch.fnmatch(name, base):
193-
list.append(fn)
194-
if rec:
195-
for subdir in subdirs:
196-
list.extend(self.findfiles(subdir, base, rec))
197-
return list
198-
199191

200192
def _grep_dialog(parent): # htest #
201193
from tkinter import Toplevel, Text, SEL, END

‎Lib/idlelib/idle_test/test_grep.py‎

Lines changed: 81 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
Otherwise, tests are mostly independent.
66
Currently only test grep_it, coverage 51%.
77
"""
8-
from idlelib.grep import GrepDialog
8+
from idlelib import grep
99
import unittest
1010
from test.support import captured_stdout
1111
from idlelib.idle_test.mock_tk import Var
12+
import os
1213
import re
1314

1415

@@ -26,23 +27,92 @@ def getpat(self):
2627
class Dummy_grep:
2728
# Methods tested
2829
#default_command = GrepDialog.default_command
29-
grep_it = GrepDialog.grep_it
30-
findfiles = GrepDialog.findfiles
30+
grep_it = grep.GrepDialog.grep_it
3131
# Other stuff needed
3232
recvar = Var(False)
3333
engine = searchengine
3434
def close(self): # gui method
3535
pass
3636

37-
grep = Dummy_grep()
37+
_grep = Dummy_grep()
3838

3939

4040
class FindfilesTest(unittest.TestCase):
41-
# findfiles is really a function, not a method, could be iterator
42-
# test that filename return filename
43-
# test that idlelib has many .py files
44-
# test that recursive flag adds idle_test .py files
45-
pass
41+
42+
@classmethod
43+
def setUpClass(cls):
44+
cls.realpath = os.path.realpath(__file__)
45+
cls.path = os.path.dirname(cls.realpath)
46+
47+
@classmethod
48+
def tearDownClass(cls):
49+
del cls.realpath, cls.path
50+
51+
def test_invaliddir(self):
52+
with captured_stdout() as s:
53+
filelist = list(grep.findfiles('invaliddir', '*.*', False))
54+
self.assertEqual(filelist, [])
55+
self.assertIn('invalid', s.getvalue())
56+
57+
def test_curdir(self):
58+
# Test os.curdir.
59+
ff = grep.findfiles
60+
save_cwd = os.getcwd()
61+
os.chdir(self.path)
62+
filename = 'test_grep.py'
63+
filelist = list(ff(os.curdir, filename, False))
64+
self.assertIn(os.path.join(os.curdir, filename), filelist)
65+
os.chdir(save_cwd)
66+
67+
def test_base(self):
68+
ff = grep.findfiles
69+
readme = os.path.join(self.path, 'README.txt')
70+
71+
# Check for Python files in path where this file lives.
72+
filelist = list(ff(self.path, '*.py', False))
73+
# This directory has many Python files.
74+
self.assertGreater(len(filelist), 10)
75+
self.assertIn(self.realpath, filelist)
76+
self.assertNotIn(readme, filelist)
77+
78+
# Look for .txt files in path where this file lives.
79+
filelist = list(ff(self.path, '*.txt', False))
80+
self.assertNotEqual(len(filelist), 0)
81+
self.assertNotIn(self.realpath, filelist)
82+
self.assertIn(readme, filelist)
83+
84+
# Look for non-matching pattern.
85+
filelist = list(ff(self.path, 'grep.*', False))
86+
self.assertEqual(len(filelist), 0)
87+
self.assertNotIn(self.realpath, filelist)
88+
89+
def test_recurse(self):
90+
ff = grep.findfiles
91+
parent = os.path.dirname(self.path)
92+
grepfile = os.path.join(parent, 'grep.py')
93+
pat = '*.py'
94+
95+
# Get Python files only in parent directory.
96+
filelist = list(ff(parent, pat, False))
97+
parent_size = len(filelist)
98+
# Lots of Python files in idlelib.
99+
self.assertGreater(parent_size, 20)
100+
self.assertIn(grepfile, filelist)
101+
# Without subdirectories, this file isn't returned.
102+
self.assertNotIn(self.realpath, filelist)
103+
104+
# Include subdirectories.
105+
filelist = list(ff(parent, pat, True))
106+
# More files found now.
107+
self.assertGreater(len(filelist), parent_size)
108+
self.assertIn(grepfile, filelist)
109+
# This file exists in list now.
110+
self.assertIn(self.realpath, filelist)
111+
112+
# Check another level up the tree.
113+
parent = os.path.dirname(parent)
114+
filelist = list(ff(parent, '*.py', True))
115+
self.assertIn(self.realpath, filelist)
46116

47117

48118
class Grep_itTest(unittest.TestCase):
@@ -51,9 +121,9 @@ class Grep_itTest(unittest.TestCase):
51121
# from incomplete replacement, so 'later'.
52122

53123
def report(self, pat):
54-
grep.engine._pat = pat
124+
_grep.engine._pat = pat
55125
with captured_stdout() as s:
56-
grep.grep_it(re.compile(pat), __file__)
126+
_grep.grep_it(re.compile(pat), __file__)
57127
lines = s.getvalue().split('\n')
58128
lines.pop() # remove bogus '' after last \n
59129
return lines

‎Misc/NEWS.d/next/IDLE/2019-03-06-14-47-57.bpo-23205.Vv0gfH.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
For the grep module, add tests for findfiles, refactor findfiles to be a
2+
module-level function, and refactor findfiles to use os.walk.

0 commit comments

Comments
 (0)