Summary
Two code quality issues were identified by Copilot during review of PR #356 (Skip crawling of excluded directories during path resolution) that should be addressed as follow-up work.
Issues to Address
1. resolve_ingress_paths() silently discards an explicitly passed empty list
File: src/pds/ingress/util/path_util.py
Problem: The initialization guard resolved_paths = resolved_paths or list() treats an explicitly passed empty list [] as falsy and allocates a new list, silently discarding the caller's reference.
Fix:
# Before
resolved_paths = resolved_paths or list()
# After
if resolved_paths is None:
resolved_paths = list()
2. get_path_progress_bar() caches PATH_BAR globally regardless of call arguments
File: src/pds/ingress/util/progress_util.py
Problem: After PR #356 is merged, get_path_progress_bar() accepts includes, excludes, and follow_symlinks parameters to compute the correct total. However, subsequent calls with different parameters still return the cached PATH_BAR initialized from the first call, potentially producing incorrect totals (progress bar overrunning 100%) and causing test bleed between test cases that call the function with different filter settings.
Fix: Guard the cached value so it is only reused when called with the same parameters, or enforce and document single-call semantics with a clear reset mechanism.
Acceptance Criteria
Context
Identified by GitHub Copilot during review of PR #356.
Summary
Two code quality issues were identified by Copilot during review of PR #356 (Skip crawling of excluded directories during path resolution) that should be addressed as follow-up work.
Issues to Address
1.
resolve_ingress_paths()silently discards an explicitly passed empty listFile:
src/pds/ingress/util/path_util.pyProblem: The initialization guard
resolved_paths = resolved_paths or list()treats an explicitly passed empty list[]as falsy and allocates a new list, silently discarding the caller's reference.Fix:
2.
get_path_progress_bar()cachesPATH_BARglobally regardless of call argumentsFile:
src/pds/ingress/util/progress_util.pyProblem: After PR #356 is merged,
get_path_progress_bar()acceptsincludes,excludes, andfollow_symlinksparameters to compute the correct total. However, subsequent calls with different parameters still return the cachedPATH_BARinitialized from the first call, potentially producing incorrect totals (progress bar overrunning 100%) and causing test bleed between test cases that call the function with different filter settings.Fix: Guard the cached value so it is only reused when called with the same parameters, or enforce and document single-call semantics with a clear reset mechanism.
Acceptance Criteria
resolve_ingress_paths()initializesresolved_pathsonly whenNoneis passed, not when[]is passedget_path_progress_bar()computes the correct total when called with different filter parameters, or raises a clear error if called more than once with conflicting settingspytest tests/)Context
Identified by GitHub Copilot during review of PR #356.