diff --git a/.python-version b/.python-version index 3a4f41ef..24ee5b1b 100644 --- a/.python-version +++ b/.python-version @@ -1 +1 @@ -3.13 \ No newline at end of file +3.13 diff --git a/main.py b/main.py index 4b766144..be361dd9 100644 --- a/main.py +++ b/main.py @@ -228,6 +228,26 @@ def sanitize_for_log(text: Any) -> str: return safe +def format_duration(seconds: float) -> str: + """Formats duration in a human-readable way (e.g., 2m 05s). + + We first round to the nearest whole second to avoid surprising + outputs around boundaries (e.g., 59.95s -> 1m 00s instead of + 60.0s) and then derive minutes/seconds from that integer. + """ + # Round once to whole seconds so behavior is consistent around the 60s + # boundary and we don't under-report longer durations due to truncation. + total_seconds = int(round(seconds)) + + if total_seconds < 60: + # For sub-minute durations, show whole seconds for clarity and to + # match the rounded value used for longer durations. + return f"{total_seconds}s" + + minutes, rem_seconds = divmod(total_seconds, 60) + return f"{minutes}m {rem_seconds:02d}s" + + def print_plan_details(plan_entry: Dict[str, Any]) -> None: """Pretty-print the folder-level breakdown during a dry-run.""" profile = sanitize_for_log(plan_entry.get("profile", "unknown")) @@ -267,12 +287,14 @@ def countdown_timer(seconds: int, message: str = "Waiting") -> None: progress = (seconds - remaining + 1) / seconds filled = int(width * progress) bar = "█" * filled + "░" * (width - filled) + # Clear line (\033[K) to prevent trailing characters when digits shrink sys.stderr.write( - f"\r{Colors.CYAN}⏳ {message}: [{bar}] {remaining}s...{Colors.ENDC}" + f"\r\033[K{Colors.CYAN}⏳ {message}: [{bar}] {remaining}s...{Colors.ENDC}" ) sys.stderr.flush() time.sleep(1) + # Clear the line one final time before showing completion message sys.stderr.write(f"\r\033[K{Colors.GREEN}✅ {message}: Done!{Colors.ENDC}\n") sys.stderr.flush() @@ -609,7 +631,8 @@ def _retry_request(request_func, max_retries=MAX_RETRIES, delay=RETRY_DELAY): f"Request failed (attempt {attempt + 1}/{max_retries}): " f"{sanitize_for_log(e)}. Retrying in {wait_time}s..." ) - time.sleep(wait_time) + # Use countdown timer for user feedback during retries (when interactive) + countdown_timer(int(wait_time), "Retrying") def _gh_get(url: str) -> Dict: @@ -836,7 +859,7 @@ def verify_access_and_get_folders( MAX_RETRIES, wait_time, ) - time.sleep(wait_time) + countdown_timer(int(wait_time), "Retrying") def get_all_existing_rules( @@ -1044,7 +1067,8 @@ def create_folder( log.info( f"Folder '{sanitize_for_log(name)}' not found yet. Retrying in {wait_time}s..." ) - time.sleep(wait_time) + # Use countdown timer for consistent UX with other retry operations + countdown_timer(int(wait_time), "Waiting for folder") log.error( f"Folder {sanitize_for_log(name)} was not found after creation and retries." @@ -1651,7 +1675,7 @@ def validate_profile_input(value: str) -> bool: f"{res['profile']:<{profile_col_width}} | " f"{res['folders']:>10} | " f"{res['rules']:>10,} | " - f"{res['duration']:>9.1f}s | " + f"{format_duration(res['duration']):>10} | " f"{status_color}{res['status_label']:<15}{Colors.ENDC}" ) total_folders += res["folders"] @@ -1682,7 +1706,7 @@ def validate_profile_input(value: str) -> bool: f"{'TOTAL':<{profile_col_width}} | " f"{total_folders:>10} | " f"{total_rules:>10,} | " - f"{total_duration:>9.1f}s | " + f"{format_duration(total_duration):>10} | " f"{total_status_color}{total_status_text:<15}{Colors.ENDC}" ) print("=" * table_width + "\n") diff --git a/tests/test_format_duration.py b/tests/test_format_duration.py new file mode 100644 index 00000000..57e5824d --- /dev/null +++ b/tests/test_format_duration.py @@ -0,0 +1,70 @@ +"""Tests for the format_duration function.""" + +import main + + +def test_format_duration_sub_minute(): + """Test format_duration with durations less than 60 seconds.""" + # Exact values + assert main.format_duration(0) == "0s" + assert main.format_duration(5) == "5s" + assert main.format_duration(42) == "42s" + assert main.format_duration(59) == "59s" + + # Rounding behavior for sub-minute values + assert main.format_duration(5.4) == "5s" # Rounds down + assert main.format_duration(5.5) == "6s" # Rounds up (banker's rounding to even) + assert main.format_duration(59.4) == "59s" # Rounds down + assert main.format_duration(59.5) == "1m 00s" # Rounds to 60 -> shows as 1m 00s + assert main.format_duration(59.95) == "1m 00s" # Edge case: rounds to 60 -> 1m 00s + + +def test_format_duration_exact_minutes(): + """Test format_duration with exact minute values.""" + assert main.format_duration(60) == "1m 00s" + assert main.format_duration(120) == "2m 00s" + assert main.format_duration(300) == "5m 00s" + assert main.format_duration(3600) == "60m 00s" + + +def test_format_duration_mixed(): + """Test format_duration with minutes and seconds.""" + assert main.format_duration(65) == "1m 05s" + assert main.format_duration(125) == "2m 05s" + assert main.format_duration(185) == "3m 05s" + assert main.format_duration(305.5) == "5m 06s" # Rounds to 306 seconds = 5m 06s + + +def test_format_duration_rounding_boundaries(): + """Test format_duration rounding behavior at boundaries. + + These boundary tests protect against the issue mentioned in the PR review + where 59.95s would show as "60.0s" instead of "1m 00s" due to truncation. + By rounding first, we get consistent behavior: values that round to 60+ + seconds are displayed in minutes format for clarity. + """ + # Just under a minute: should round down and stay in seconds + assert main.format_duration(59.4) == "59s" + + # Halfway to next second at boundary: rounds to 60 -> shown as minutes + assert main.format_duration(59.5) == "1m 00s" + + # Very close to a minute: rounds to 60 -> shown as 1m 00s (clearer than "60s") + assert main.format_duration(59.95) == "1m 00s" + + # Just over a minute: should be in minutes format + assert main.format_duration(60.1) == "1m 00s" + assert main.format_duration(60.5) == "1m 00s" # Banker's rounding: rounds to 60 (even) + assert main.format_duration(61.5) == "1m 02s" # Banker's rounding: rounds to 62 (even) + + # Edge cases around 2 minutes + assert main.format_duration(119.4) == "1m 59s" # Rounds down + assert main.format_duration(119.5) == "2m 00s" # Rounds up + assert main.format_duration(125.9) == "2m 06s" # Example from PR review + + +def test_format_duration_large_values(): + """Test format_duration with large durations.""" + assert main.format_duration(3661) == "61m 01s" + assert main.format_duration(7200) == "120m 00s" + assert main.format_duration(7325.7) == "122m 06s" # 7326 seconds = 122m 06s diff --git a/tests/test_plan_details.py b/tests/test_plan_details.py index 12cacb2c..9c46478c 100644 --- a/tests/test_plan_details.py +++ b/tests/test_plan_details.py @@ -1,21 +1,19 @@ """Tests for the print_plan_details dry-run output function.""" -from unittest.mock import patch - import main -def test_print_plan_details_no_colors(capsys): +def test_print_plan_details_no_colors(capsys, monkeypatch): """Test print_plan_details output when colors are disabled.""" - with patch("main.USE_COLORS", False): - plan_entry = { - "profile": "test_profile", - "folders": [ - {"name": "Folder B", "rules": 5}, - {"name": "Folder A", "rules": 10}, - ], - } - main.print_plan_details(plan_entry) + monkeypatch.setattr(main, "USE_COLORS", False) + plan_entry = { + "profile": "test_profile", + "folders": [ + {"name": "Folder B", "rules": 5}, + {"name": "Folder A", "rules": 10}, + ], + } + main.print_plan_details(plan_entry) captured = capsys.readouterr() output = captured.out @@ -27,11 +25,11 @@ def test_print_plan_details_no_colors(capsys): assert output.index("Folder A") < output.index("Folder B") -def test_print_plan_details_empty_folders(capsys): +def test_print_plan_details_empty_folders(capsys, monkeypatch): """Test print_plan_details with no folders.""" - with patch("main.USE_COLORS", False): - plan_entry = {"profile": "test_profile", "folders": []} - main.print_plan_details(plan_entry) + monkeypatch.setattr(main, "USE_COLORS", False) + plan_entry = {"profile": "test_profile", "folders": []} + main.print_plan_details(plan_entry) captured = capsys.readouterr() output = captured.out @@ -40,14 +38,14 @@ def test_print_plan_details_empty_folders(capsys): assert "No folders to sync." in output -def test_print_plan_details_with_colors(capsys): +def test_print_plan_details_with_colors(capsys, monkeypatch): """Test print_plan_details output when colors are enabled.""" - with patch("main.USE_COLORS", True): - plan_entry = { - "profile": "test_profile", - "folders": [{"name": "Folder A", "rules": 10}], - } - main.print_plan_details(plan_entry) + monkeypatch.setattr(main, "USE_COLORS", True) + plan_entry = { + "profile": "test_profile", + "folders": [{"name": "Folder A", "rules": 10}], + } + main.print_plan_details(plan_entry) captured = capsys.readouterr() output = captured.out