Skip to content

Commit 19c1f7d

Browse files
jordanpadamsclaude
andcommitted
Fix two Copilot code quality findings from PR #356
- resolve_ingress_paths(): replace falsy-check `or list()` with explicit `is None` guard so a caller-supplied empty list is never silently discarded - get_path_progress_bar(): track parameters used to build the cached PATH_BAR and recompute when called with different includes/excludes/follow_symlinks, preventing incorrect totals and test bleed - Add unit tests for both fixes (issue #361) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d15b3fd commit 19c1f7d

3 files changed

Lines changed: 41 additions & 6 deletions

File tree

src/pds/ingress/util/path_util.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ def resolve_ingress_paths(user_paths, includes, excludes, pbar, resolved_paths=N
7676
# Use a logger with no console output to avoid interfering with tqdm output
7777
logger = get_logger("resove_ingress_paths", console=False)
7878

79-
# Initialize the list of resolved paths if necessary
80-
resolved_paths = resolved_paths or list()
79+
if resolved_paths is None:
80+
resolved_paths = list()
8181

8282
for ingress_path in PathUtil._iter_resolvable_ingress_paths(
8383
user_paths, includes, excludes, follow_symlinks, logger

src/pds/ingress/util/progress_util.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from tqdm.asyncio import tqdm_asyncio
1515

1616
PATH_BAR = None
17+
_PATH_BAR_PARAMS = None # (user_paths_tuple, includes_tuple, excludes_tuple, follow_symlinks) used to build PATH_BAR
1718
MANIFEST_BAR = None
1819
TOTAL_INGRESS_BAR = None
1920
BATCH_BARS = []
@@ -47,11 +48,15 @@ def get_path_progress_bar(user_paths, includes=None, excludes=None, follow_symli
4748
The initialized global instance of the Path Resolution progress bar.
4849
4950
"""
50-
global PATH_BAR
51+
global PATH_BAR, _PATH_BAR_PARAMS
5152

52-
if PATH_BAR is None:
53-
includes = includes or []
54-
excludes = excludes or []
53+
includes = includes or []
54+
excludes = excludes or []
55+
call_params = (tuple(user_paths), tuple(includes), tuple(excludes), follow_symlinks)
56+
57+
if PATH_BAR is None or _PATH_BAR_PARAMS != call_params:
58+
if PATH_BAR is not None:
59+
PATH_BAR.close()
5560
total_files = PathUtil.count_resolvable_ingress_paths(user_paths, includes, excludes, follow_symlinks)
5661

5762
PATH_BAR = tqdm(
@@ -62,6 +67,7 @@ def get_path_progress_bar(user_paths, includes=None, excludes=None, follow_symli
6267
colour=LIGHT_GREEN,
6368
bar_format="{l_bar}{bar:60}| {n_fmt}/{total_fmt} Files",
6469
)
70+
_PATH_BAR_PARAMS = call_params
6571

6672
return PATH_BAR
6773

tests/pds/ingress/util/test_path_util.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,35 @@ def test_nested_include_and_exclude_filters_share_count_and_resolution_semantics
230230
self.assertNotIn(abspath(join(included_dir, "skip.dat")), resolved_ingress_paths)
231231
self.assertNotIn(abspath(join(excluded_dir, "drop.xml")), resolved_ingress_paths)
232232

233+
def test_resolve_ingress_paths_preserves_explicitly_passed_empty_list(self):
234+
"""Test that resolve_ingress_paths() does not replace an explicitly passed [] with a new list"""
235+
Path(self.working_dir.name, "file.txt").touch()
236+
237+
caller_list = []
238+
with get_path_progress_bar([self.working_dir.name]) as pbar:
239+
result = PathUtil.resolve_ingress_paths([self.working_dir.name], [], [], pbar, resolved_paths=caller_list)
240+
241+
self.assertIs(result, caller_list)
242+
self.assertEqual(len(result), 1)
243+
244+
def test_get_path_progress_bar_recomputes_total_for_different_params(self):
245+
"""Test that get_path_progress_bar() recomputes total when called with different filter parameters"""
246+
Path(self.working_dir.name, "keep.txt").touch()
247+
Path(self.working_dir.name, "drop.xml").touch()
248+
249+
with get_path_progress_bar([self.working_dir.name], [], []) as first_bar:
250+
first_total = first_bar.total
251+
252+
progress_util.PATH_BAR = None
253+
progress_util._PATH_BAR_PARAMS = None
254+
255+
excludes = [join(abspath(self.working_dir.name), "*.xml")]
256+
with get_path_progress_bar([self.working_dir.name], [], excludes) as second_bar:
257+
second_total = second_bar.total
258+
259+
self.assertEqual(first_total, 2)
260+
self.assertEqual(second_total, 1)
261+
233262
def test_trim_ingress_path(self):
234263
"""Test the trim_ingress_path() function"""
235264
ingress_paths = [

0 commit comments

Comments
 (0)