Skip to content

Commit eef2e46

Browse files
cpyle0819cpyle0819
committed
Address PR feedback: use fully qualified imports, absolute paths, and add nested gitignore test
- Use fully qualified imports in file_utils.py and file_utils_test.py - Convert all test paths to absolute paths as requested - Add comprehensive nested gitignore test case - Improve RecordFs.list() method to handle intermediate directories properly
1 parent cb136d4 commit eef2e46

File tree

3 files changed

+94
-48
lines changed

3 files changed

+94
-48
lines changed

‎aws_doc_sdk_examples_tools/file_utils.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from shutil import rmtree
99

1010
from pathspec import GitIgnoreSpec
11-
from .fs import Fs, PathFs
11+
from aws_doc_sdk_examples_tools.fs import Fs, PathFs
1212

1313

1414
def match_path_to_specs(path: Path, specs: List[GitIgnoreSpec]) -> bool:

‎aws_doc_sdk_examples_tools/file_utils_test.py‎

Lines changed: 74 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77

88
from pathlib import Path
99

10-
from .fs import RecordFs
11-
from .file_utils import walk_with_gitignore, get_files
10+
from aws_doc_sdk_examples_tools.fs import RecordFs
11+
from aws_doc_sdk_examples_tools.file_utils import walk_with_gitignore, get_files
1212

1313

1414
class TestWalkWithGitignore:
@@ -18,79 +18,108 @@ def test_basic_directory_traversal(self):
1818
"""Test basic directory traversal without gitignore."""
1919
fs = RecordFs(
2020
{
21-
Path("root/file1.py"): "print('file1')",
22-
Path("root/file2.js"): "console.log('file2')",
21+
Path("/root/file1.py"): "print('file1')",
22+
Path("/root/file2.js"): "console.log('file2')",
2323
}
2424
)
2525

26-
files = list(walk_with_gitignore(Path("root"), fs=fs))
26+
files = list(walk_with_gitignore(Path("/root"), fs=fs))
2727

2828
expected = [
29-
Path("root/file1.py"),
30-
Path("root/file2.js"),
29+
Path("/root/file1.py"),
30+
Path("/root/file2.js"),
3131
]
3232
assert sorted(files) == sorted(expected)
3333

3434
def test_gitignore_filtering(self):
3535
"""Test that gitignore rules are applied correctly."""
3636
fs = RecordFs(
3737
{
38-
Path("root/.gitignore"): "*.tmp\n*.log\n",
39-
Path("root/keep.py"): "print('keep')",
40-
Path("root/ignore.tmp"): "temporary",
41-
Path("root/keep.js"): "console.log('keep')",
42-
Path("root/debug.log"): "log content",
38+
Path("/root/.gitignore"): "*.tmp\n*.log\n",
39+
Path("/root/keep.py"): "print('keep')",
40+
Path("/root/ignore.tmp"): "temporary",
41+
Path("/root/keep.js"): "console.log('keep')",
42+
Path("/root/debug.log"): "log content",
4343
}
4444
)
4545

46-
files = list(walk_with_gitignore(Path("root"), fs=fs))
46+
files = list(walk_with_gitignore(Path("/root"), fs=fs))
4747

4848
# .gitignore files should not be included in results
4949
expected = [
50-
Path("root/keep.py"),
51-
Path("root/keep.js"),
50+
Path("/root/keep.py"),
51+
Path("/root/keep.js"),
5252
]
5353
assert sorted(files) == sorted(expected)
5454

5555
def test_no_gitignore_file(self):
5656
"""Test directory traversal when no .gitignore exists."""
5757
fs = RecordFs(
5858
{
59-
Path("root/file1.py"): "print('file1')",
60-
Path("root/file2.js"): "console.log('file2')",
61-
Path("root/file3.txt"): "text content",
59+
Path("/root/file1.py"): "print('file1')",
60+
Path("/root/file2.js"): "console.log('file2')",
61+
Path("/root/file3.txt"): "text content",
6262
}
6363
)
6464

65-
files = list(walk_with_gitignore(Path("root"), fs=fs))
65+
files = list(walk_with_gitignore(Path("/root"), fs=fs))
6666

6767
expected = [
68-
Path("root/file1.py"),
69-
Path("root/file2.js"),
70-
Path("root/file3.txt"),
68+
Path("/root/file1.py"),
69+
Path("/root/file2.js"),
70+
Path("/root/file3.txt"),
7171
]
7272
assert sorted(files) == sorted(expected)
7373

7474
def test_empty_directory(self):
7575
"""Test walking an empty directory."""
7676
fs = RecordFs({})
7777

78-
files = list(walk_with_gitignore(Path("empty"), fs=fs))
78+
files = list(walk_with_gitignore(Path("/empty"), fs=fs))
7979

8080
assert files == []
8181

8282
def test_directory_with_only_gitignore(self):
8383
"""Test directory that only contains .gitignore file."""
8484
fs = RecordFs(
8585
{
86-
Path("root/.gitignore"): "*.tmp\n",
86+
Path("/root/.gitignore"): "*.tmp\n",
8787
}
8888
)
8989

90-
files = list(walk_with_gitignore(Path("root"), fs=fs))
90+
files = list(walk_with_gitignore(Path("/root"), fs=fs))
9191

9292
assert files == []
9393

94+
def test_nested_gitignores(self):
95+
"""Test nested gitignore files with different rules."""
96+
fs = RecordFs(
97+
{
98+
# Root level gitignore ignores *.log files
99+
Path("/root/.gitignore"): "*.log\n",
100+
Path("/root/keep.py"): "print('keep')",
101+
Path("/root/debug.log"): "root log", # Should be ignored
102+
# Nested directory with its own gitignore ignoring *.tmp files
103+
Path("/root/subdir/.gitignore"): "*.tmp\n",
104+
Path("/root/subdir/keep.js"): "console.log('keep')",
105+
Path(
106+
"/root/subdir/ignore.tmp"
107+
): "temporary", # Should be ignored by subdir gitignore
108+
Path(
109+
"/root/subdir/keep.log"
110+
): "nested log", # Should be ignored by root gitignore
111+
}
112+
)
113+
114+
files = list(walk_with_gitignore(Path("/root"), fs=fs))
115+
116+
# Only files that don't match any gitignore pattern should be returned
117+
expected = [
118+
Path("/root/keep.py"),
119+
Path("/root/subdir/keep.js"),
120+
]
121+
assert sorted(files) == sorted(expected)
122+
94123

95124
class TestGetFiles:
96125
"""Test get_files with RecordFs."""
@@ -99,60 +128,60 @@ def test_get_files_basic(self):
99128
"""Test basic get_files functionality."""
100129
fs = RecordFs(
101130
{
102-
Path("root/file1.py"): "print('file1')",
103-
Path("root/file2.js"): "console.log('file2')",
131+
Path("/root/file1.py"): "print('file1')",
132+
Path("/root/file2.js"): "console.log('file2')",
104133
}
105134
)
106135

107-
files = list(get_files(Path("root"), fs=fs))
136+
files = list(get_files(Path("/root"), fs=fs))
108137

109138
expected = [
110-
Path("root/file1.py"),
111-
Path("root/file2.js"),
139+
Path("/root/file1.py"),
140+
Path("/root/file2.js"),
112141
]
113142
assert sorted(files) == sorted(expected)
114143

115144
def test_get_files_with_skip_function(self):
116145
"""Test get_files with skip function."""
117146
fs = RecordFs(
118147
{
119-
Path("root/keep.py"): "print('keep')",
120-
Path("root/skip.py"): "print('skip')",
121-
Path("root/keep.js"): "console.log('keep')",
122-
Path("root/skip.js"): "console.log('skip')",
148+
Path("/root/keep.py"): "print('keep')",
149+
Path("/root/skip.py"): "print('skip')",
150+
Path("/root/keep.js"): "console.log('keep')",
151+
Path("/root/skip.js"): "console.log('skip')",
123152
}
124153
)
125154

126155
def skip_function(path: Path) -> bool:
127156
return "skip" in path.name
128157

129-
files = list(get_files(Path("root"), skip=skip_function, fs=fs))
158+
files = list(get_files(Path("/root"), skip=skip_function, fs=fs))
130159

131160
expected = [
132-
Path("root/keep.py"),
133-
Path("root/keep.js"),
161+
Path("/root/keep.py"),
162+
Path("/root/keep.js"),
134163
]
135164
assert sorted(files) == sorted(expected)
136165

137166
def test_get_files_with_gitignore_and_skip(self):
138167
"""Test get_files with both gitignore and skip function."""
139168
fs = RecordFs(
140169
{
141-
Path("root/.gitignore"): "*.tmp\n",
142-
Path("root/keep.py"): "print('keep')",
143-
Path("root/skip.py"): "print('skip')",
144-
Path("root/ignore.tmp"): "temporary",
145-
Path("root/keep.js"): "console.log('keep')",
170+
Path("/root/.gitignore"): "*.tmp\n",
171+
Path("/root/keep.py"): "print('keep')",
172+
Path("/root/skip.py"): "print('skip')",
173+
Path("/root/ignore.tmp"): "temporary",
174+
Path("/root/keep.js"): "console.log('keep')",
146175
}
147176
)
148177

149178
def skip_function(path: Path) -> bool:
150179
return "skip" in path.name
151180

152-
files = list(get_files(Path("root"), skip=skip_function, fs=fs))
181+
files = list(get_files(Path("/root"), skip=skip_function, fs=fs))
153182

154183
expected = [
155-
Path("root/keep.py"),
156-
Path("root/keep.js"),
184+
Path("/root/keep.py"),
185+
Path("/root/keep.js"),
157186
]
158187
assert sorted(files) == sorted(expected)

‎aws_doc_sdk_examples_tools/fs.py‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def glob(self, path: Path, glob: str) -> Generator[Path, None, None]:
5353
return path.glob(glob)
5454

5555
def read(self, path: Path) -> str:
56-
with path.open("r") as file:
56+
with path.open("r", encoding="utf-8") as file:
5757
return file.read()
5858

5959
def readlines(self, path: Path) -> List[str]:
@@ -118,7 +118,24 @@ def mkdir(self, path: Path):
118118
self.fs.setdefault(path, "")
119119

120120
def list(self, path: Path) -> List[Path]:
121-
return [item for item in self.fs.keys() if item.parent == path]
121+
# If it's a file, return an empty list
122+
if self.stat(path).is_file:
123+
return []
124+
125+
# Gather all entries that are immediate children of `path`
126+
prefix = str(path).rstrip("/") + "/"
127+
children = set()
128+
129+
for item in self.fs.keys():
130+
item_s = str(item)
131+
if item_s.startswith(prefix):
132+
# Determine the remainder path after the prefix
133+
remainder = item_s[len(prefix) :]
134+
# Split off the first component
135+
first_part = remainder.split("/", 1)[0]
136+
children.add(Path(prefix + first_part))
137+
138+
return sorted(children)
122139

123140

124141
fs = PathFs()

0 commit comments

Comments
 (0)