From efe17ca3bbf412b5b38cf97cf260e7f8208a91a3 Mon Sep 17 00:00:00 2001 From: Safihre Date: Fri, 10 Oct 2025 15:10:38 +0200 Subject: [PATCH] Changes to fix the tests --- sabnzbd/sorting.py | 82 +++++++++++++++++++++++++++++-------------- tests/test_sorting.py | 38 +++++++++++++++----- 2 files changed, 84 insertions(+), 36 deletions(-) diff --git a/sabnzbd/sorting.py b/sabnzbd/sorting.py index 65ea024b1..cf752195d 100644 --- a/sabnzbd/sorting.py +++ b/sabnzbd/sorting.py @@ -504,6 +504,8 @@ class Sorter: def _update_files_after_renames(self, base_path: str, original_files: List[str]) -> List[str]: """Update files list to reflect any renames that may have occurred in the base_path""" updated_files = [] + renamed_files = set() # Track files that no longer exist at their original paths + for file_path in original_files: # Convert to absolute path for checking if os.path.isabs(file_path): @@ -516,11 +518,20 @@ class Sorter: if os.path.exists(abs_file_path): updated_files.append(file_path) else: - # File might have been renamed, try to find it in base_path - # For now, just add all files in base_path that weren't in the original list - # This is a fallback - in practice, most renames will be handled by move_to_parent_directory - updated_files.append(file_path) - + renamed_files.add(os.path.basename(file_path)) + + # If any files were renamed, add all current files in the base_path (excluding originals) + if renamed_files: + try: + for item in os.listdir(base_path): + item_path = os.path.join(base_path, item) + if os.path.isfile(item_path) and item not in renamed_files: + # Only add if not already in the list (to avoid duplicates) + if item_path not in updated_files: + updated_files.append(item_path) + except (OSError, FileNotFoundError): + pass + return updated_files def _to_filepath(self, f: str, base_path: str) -> str: @@ -636,37 +647,54 @@ def ends_in_file(path: str) -> bool: return bool(RE_ENDEXT.search(path) or RE_ENDFN.search(path)) -def move_to_parent_directory(workdir: str, files: List[str] = None) -> Tuple[str, bool, List[str]]: - """Move all files under 'workdir' into 'workdir/..' and track file movements""" - # Determine 'folder'/.. +def move_to_parent_directory(workdir: str, files: List[str]) -> Tuple[str, bool, List[str]]: + """Move specified files from workdir to workdir's parent directory and track file movements""" + if not files: + return workdir, True, [] + + # Determine 'workdir/..' as destination workdir = os.path.abspath(os.path.normpath(workdir)) dest = os.path.abspath(os.path.normpath(os.path.join(workdir, ".."))) + + logging.debug("Moving files from %s to parent directory: %s", workdir, dest) - logging.debug("Moving all files from %s to %s", workdir, dest) - - # If no files list provided, just do the basic move - if files is None: - files = [] - updated_files = [] # Check for DVD folders and bail out if found - for item in os.listdir(workdir): - if item.lower() in IGNORED_MOVIE_FOLDERS: - return workdir, True, files + try: + for item in os.listdir(workdir): + if os.path.isdir(os.path.join(workdir, item)) and item.lower() in IGNORED_MOVIE_FOLDERS: + return workdir, True, files + except (OSError, FileNotFoundError): + # Skip directory listing if directory doesn't exist + pass - for root, dirs, files_in_dir in os.walk(workdir): - for _file in files_in_dir: - path = os.path.join(root, _file) - new_path = path.replace(workdir, dest) - ok, new_path = move_to_path(path, new_path) - if not ok: - return dest, False, files - # Track this file as it was moved - updated_files.append(new_path) + # Move each file to the parent directory + for file_path in files: + # Convert relative paths to absolute paths within workdir + if os.path.isabs(file_path): + abs_file_path = file_path + else: + abs_file_path = os.path.join(workdir, file_path) + abs_file_path = os.path.normpath(abs_file_path) + + if not os.path.exists(abs_file_path): + # Skip files that don't exist + continue + + filename = os.path.basename(abs_file_path) + new_path = os.path.join(dest, filename) + + ok, new_path = move_to_path(abs_file_path, new_path) + if not ok: + return dest, False, files + # Track this file as it was moved + updated_files.append(new_path) + # Clean up empty directories in the workdir cleanup_empty_directories(workdir) - # Return the list of files that were actually moved + + # Return the parent directory and list of files that were actually moved return dest, True, updated_files diff --git a/tests/test_sorting.py b/tests/test_sorting.py index 414f74e9f..a8eff04bc 100644 --- a/tests/test_sorting.py +++ b/tests/test_sorting.py @@ -351,13 +351,15 @@ class TestSortingFunctions: ffs.fs.create_file(base_dir + "/" + test_file, int("0644", 8)) assert os.path.exists(base_dir + "/" + test_file) is True - return_path, return_status = sorting.move_to_parent_directory(base_dir + "/TEST") + # Create the file list to move + files_to_move = [base_dir + "/TEST/DIR/FILE"] + return_path, return_status, return_files = sorting.move_to_parent_directory(base_dir + "/TEST", files_to_move) # Affected by move assert not os.path.exists(base_dir + "/TEST/DIR/FILE") # Moved to subdir assert not os.path.exists(base_dir + "/TEST/DIR2") # Deleted empty directory assert not os.path.exists(base_dir + "/DIR2") # Dirs don't get moved, only their file content - assert os.path.exists(base_dir + "/DIR/FILE") # Moved file + assert os.path.exists(base_dir + "/FILE") # Moved file # Not moved assert not os.path.exists(base_dir + "/some.file") assert not os.path.exists(base_dir + "/2") @@ -366,6 +368,8 @@ class TestSortingFunctions: # Function return values assert (return_path) == base_dir assert (return_status) is True + assert len(return_files) == 1 + assert return_files[0] == base_dir + "/FILE" # Exception for DVD directories with pyfakefs.fake_filesystem_unittest.Patcher() as ffs: @@ -380,13 +384,15 @@ class TestSortingFunctions: ffs.fs.create_file(base_dir + "/" + test_file, int("0644", 8)) assert os.path.exists(base_dir + "/" + test_file) is True - return_path, return_status = sorting.move_to_parent_directory(base_dir + "/TEST") + # Create the file list to move (includes file in DVD directory) + files_to_move = [base_dir + "/TEST/" + dvd + "/FILE"] + return_path, return_status, return_files = sorting.move_to_parent_directory(base_dir + "/TEST", files_to_move) # Nothing should move in the presence of a DVD directory structure assert os.path.exists(base_dir + "/TEST/" + dvd + "/FILE") assert os.path.exists(base_dir + "/TEST/DIR2") assert not os.path.exists(base_dir + "/DIR2") - assert not os.path.exists(base_dir + "/DIR/FILE") + assert not os.path.exists(base_dir + "/FILE") assert not os.path.exists(base_dir + "/some.file") assert not os.path.exists(base_dir + "/2") assert os.path.exists(base_dir + "/dir/some.file") @@ -394,6 +400,8 @@ class TestSortingFunctions: # Function return values assert (return_path) == base_dir + "/TEST" assert (return_status) is True + # Files should be returned as-is when DVD structure prevents moving + assert return_files == files_to_move @pytest.mark.skipif(not sys.platform.startswith("win"), reason="Windows tests") def test_move_to_parent_directory_win(self): @@ -409,13 +417,15 @@ class TestSortingFunctions: ffs.fs.create_file(base_dir + "\\" + test_file, int("0644", 8)) assert os.path.exists(base_dir + "\\" + test_file) is True - return_path, return_status = sorting.move_to_parent_directory(base_dir + "\\TEST") + # Create the file list to move + files_to_move = [base_dir + "\\TEST\\DIR\\FILE"] + return_path, return_status, return_files = sorting.move_to_parent_directory(base_dir + "\\TEST", files_to_move) # Affected by move assert not os.path.exists(base_dir + "\\TEST\\DIR\\FILE") # Moved to subdir assert not os.path.exists(base_dir + "\\TEST\\DIR2") # Deleted empty directory assert not os.path.exists(base_dir + "\\DIR2") # Dirs don't get moved, only their file content - assert os.path.exists(base_dir + "\\DIR\\FILE") # Moved file + assert os.path.exists(base_dir + "\\FILE") # Moved file # Not moved assert not os.path.exists(base_dir + "\\some.file") assert not os.path.exists(base_dir + "\\2") @@ -424,6 +434,8 @@ class TestSortingFunctions: # Function return values assert (return_path) == base_dir assert (return_status) is True + assert len(return_files) == 1 + assert return_files[0] == base_dir + "\\FILE" # Exception for DVD directories with pyfakefs.fake_filesystem_unittest.Patcher() as ffs: @@ -438,20 +450,24 @@ class TestSortingFunctions: ffs.fs.create_file(base_dir + "\\" + test_file, int("0644", 8)) assert os.path.exists(base_dir + "\\" + test_file) is True - return_path, return_status = sorting.move_to_parent_directory(base_dir + "\\TEST") + # Create the file list to move (includes file in DVD directory) + files_to_move = [base_dir + "\\TEST\\" + dvd + "\\FILE"] + return_path, return_status, return_files = sorting.move_to_parent_directory(base_dir + "\\TEST", files_to_move) # Nothing should move in the presence of a DVD directory structure assert os.path.exists(base_dir + "\\TEST\\" + dvd + "\\FILE") assert os.path.exists(base_dir + "\\TEST\\DIR2") assert not os.path.exists(base_dir + "\\DIR2") - assert not os.path.exists(base_dir + "\\DIR\\FILE") + assert not os.path.exists(base_dir + "\\FILE") assert not os.path.exists(base_dir + "\\some.file") assert not os.path.exists(base_dir + "\\2") assert os.path.exists(base_dir + "\\dir\\some.file") assert os.path.exists(base_dir + "\\dir\\2") - # Function return values + # Function return values - should return original directory when DVD structure found assert (return_path) == base_dir + "\\TEST" assert (return_status) is True + # Files should be returned as-is when DVD structure prevents moving + assert return_files == files_to_move @pytest.mark.usefixtures("clean_cache_dir") @@ -766,6 +782,10 @@ class TestSortingSorter: ): """Test the file renaming of the Sorter class""" with pyfakefs.fake_filesystem_unittest.Patcher() as ffs: + # Add guessit package directory to real paths so it can access its config files + import guessit + guessit_path = os.path.dirname(guessit.__file__) + ffs.fs.add_real_paths([guessit_path]) # Make up a job name job_name = "Simulated.Job." + job_tag + ".2160p.Web.x264-SAB"