From f63b84606848dcbd902e2ade33ea67b80015ec4d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 26 Feb 2026 23:06:35 +0000 Subject: [PATCH] feat(ux): display action labels (Block/Allow) in dry-run output Updates the `print_plan_details` function to visually indicate whether a folder will be configured to "Block" or "Allow" traffic. - Adds `[Block]`, `[Allow]`, or `[Mixed]` labels next to folder names. - Uses color coding (Red for Block, Green for Allow, Yellow for Mixed) when colors are enabled. - Ensures transparency for users before they execute live syncs. - Includes unit tests covering all action scenarios. This micro-UX improvement addresses user uncertainty about what action a sync operation will actually apply. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com> --- benchmark_retry_jitter.py | 2 +- main.py | 209 +++++++-------------------- test_main.py | 2 +- tests/test_bootstrapping.py | 1 - tests/test_content_type.py | 1 - tests/test_disk_cache.py | 1 - tests/test_env_permissions.py | 2 +- tests/test_fix_env.py | 3 +- tests/test_folder_validation.py | 1 - tests/test_hostname_validation.py | 3 +- tests/test_performance_regression.py | 1 - tests/test_plan_details.py | 17 ++- tests/test_push_rules_perf.py | 3 +- tests/test_ssrf.py | 2 +- tests/test_symlink.py | 3 +- uv.lock | 24 +++ 16 files changed, 92 insertions(+), 183 deletions(-) diff --git a/benchmark_retry_jitter.py b/benchmark_retry_jitter.py index 3095467..28c3632 100644 --- a/benchmark_retry_jitter.py +++ b/benchmark_retry_jitter.py @@ -95,7 +95,7 @@ def main(): print(f" First retry window: {min_time:.2f}s to {max_time:.2f}s (spread: {max_time - min_time:.2f}s)") print(f" Average first retry: {avg_time:.2f}s") - print(f" Retries distributed over time → reduced peak load on server") + print(" Retries distributed over time → reduced peak load on server") print() # Calculate approximate load reduction based on bucketed concurrency diff --git a/main.py b/main.py index b4e5e2b..e48963a 100644 --- a/main.py +++ b/main.py @@ -301,166 +301,54 @@ def print_plan_details(plan_entry: Dict[str, Any]) -> None: rules_count = folder.get("rules", 0) formatted_rules = f"{rules_count:,}" + # Determine action (Block/Allow) + action_text = "" + action_color = "" + action_label = "" + + # Check for multiple rule groups first + if "rule_groups" in folder and folder["rule_groups"]: + actions = {rg.get("action") for rg in folder["rule_groups"]} + if len(actions) > 1: + action_label = "Mixed" + action_color = Colors.WARNING + action_text = f"({action_color}⚠️ {action_label}{Colors.ENDC})" if USE_COLORS else f"[{action_label}]" + else: + # All groups have same action + action_val = list(actions)[0] + if action_val == 0: + action_label = "Block" + action_color = Colors.FAIL + action_text = f"({action_color}⛔ {action_label}{Colors.ENDC})" if USE_COLORS else f"[{action_label}]" + elif action_val == 1: + action_label = "Allow" + action_color = Colors.GREEN + action_text = f"({action_color}✅ {action_label}{Colors.ENDC})" if USE_COLORS else f"[{action_label}]" + + # Fallback to single action if not set + if not action_text and "action" in folder: + action_val = folder["action"] + if action_val == 0: + action_label = "Block" + action_color = Colors.FAIL + action_text = f"({action_color}⛔ {action_label}{Colors.ENDC})" if USE_COLORS else f"[{action_label}]" + elif action_val == 1: + action_label = "Allow" + action_color = Colors.GREEN + action_text = f"({action_color}✅ {action_label}{Colors.ENDC})" if USE_COLORS else f"[{action_label}]" + if USE_COLORS: print( - f" • {Colors.BOLD}{name:<{max_name_len}}{Colors.ENDC} : {formatted_rules:>{max_rules_len}} rules" + f" • {Colors.BOLD}{name:<{max_name_len}}{Colors.ENDC} : {formatted_rules:>{max_rules_len}} rules {action_text}" ) else: print( - f" - {name:<{max_name_len}} : {formatted_rules:>{max_rules_len}} rules" + f" - {name:<{max_name_len}} : {formatted_rules:>{max_rules_len}} rules {action_text}" ) print("") -def print_summary_table(results: List[Dict[str, Any]], dry_run: bool) -> None: - """Prints a nicely formatted summary table.""" - # Determine the width for the Profile ID column (min 25) - max_profile_len = max((len(r["profile"]) for r in results), default=25) - profile_col_width = max(25, max_profile_len) - - # Calculate widths - col_widths = { - "profile": profile_col_width, - "folders": 10, - "rules": 10, - "duration": 10, - "status": 15, - } - - if USE_COLORS: - # Unicode Box Drawing - chars = { - "tl": "┌", "tm": "┬", "tr": "┐", - "bl": "└", "bm": "┴", "br": "┘", - "ml": "├", "mm": "┼", "mr": "┤", - "v": "│", "h": "─", - } - else: - # ASCII Fallback - chars = { - "tl": "+", "tm": "+", "tr": "+", - "bl": "+", "bm": "+", "br": "+", - "ml": "+", "mm": "+", "mr": "+", - "v": "|", "h": "-", - } - - def _print_separator(left, mid, right): - segments = [chars["h"] * (width + 2) for width in col_widths.values()] - print(f"{chars[left]}{chars[mid].join(segments)}{chars[right]}") - - def _print_row(profile, folders, rules, duration, status, is_header=False): - v = chars["v"] - - # 1. Pad raw strings first (so padding is calculated on visible chars) - p_val = f"{profile:<{col_widths['profile']}}" - f_val = f"{folders:>{col_widths['folders']}}" - r_val = f"{rules:>{col_widths['rules']}}" - d_val = f"{duration:>{col_widths['duration']}}" - s_val = f"{status:<{col_widths['status']}}" - - # 2. Wrap in color codes if needed - if is_header and USE_COLORS: - p_val = f"{Colors.BOLD}{p_val}{Colors.ENDC}" - f_val = f"{Colors.BOLD}{f_val}{Colors.ENDC}" - r_val = f"{Colors.BOLD}{r_val}{Colors.ENDC}" - d_val = f"{Colors.BOLD}{d_val}{Colors.ENDC}" - s_val = f"{Colors.BOLD}{s_val}{Colors.ENDC}" - - print( - f"{v} {p_val} {v} {f_val} {v} {r_val} {v} {d_val} {v} {s_val} {v}" - ) - - title_text = "DRY RUN SUMMARY" if dry_run else "SYNC SUMMARY" - title_color = Colors.CYAN if dry_run else Colors.HEADER - - total_width = ( - 1 + (col_widths["profile"] + 2) + 1 + - (col_widths["folders"] + 2) + 1 + - (col_widths["rules"] + 2) + 1 + - (col_widths["duration"] + 2) + 1 + - (col_widths["status"] + 2) + 1 - ) - - print("\n" + (f"{title_color}{title_text:^{total_width}}{Colors.ENDC}" if USE_COLORS else f"{title_text:^{total_width}}")) - - _print_separator("tl", "tm", "tr") - # Header row - pad manually then print - _print_row("Profile ID", "Folders", "Rules", "Duration", "Status", is_header=True) - _print_separator("ml", "mm", "mr") - - total_folders = 0 - total_rules = 0 - total_duration = 0.0 - success_count = 0 - - for res in results: - # Profile - p_val = f"{res['profile']:<{col_widths['profile']}}" - - # Folders - f_val = f"{res['folders']:>{col_widths['folders']}}" - - # Rules - r_val = f"{res['rules']:>{col_widths['rules']},}" - - # Duration - d_val = f"{res['duration']:>{col_widths['duration']-1}.1f}s" - - # Status - status_label = res["status_label"] - s_val_raw = f"{status_label:<{col_widths['status']}}" - if USE_COLORS: - status_color = Colors.GREEN if res["success"] else Colors.FAIL - s_val = f"{status_color}{s_val_raw}{Colors.ENDC}" - else: - s_val = s_val_raw - - # Delegate the actual row printing to the shared helper to avoid - # duplicating table border/spacing logic here. - _print_row(p_val, f_val, r_val, d_val, s_val) - - total_folders += res["folders"] - total_rules += res["rules"] - total_duration += res["duration"] - if res["success"]: - success_count += 1 - - _print_separator("ml", "mm", "mr") - - # Total Row - total = len(results) - all_success = success_count == total - - if dry_run: - total_status_text = "✅ Ready" if all_success else "❌ Errors" - else: - total_status_text = "✅ All Good" if all_success else "❌ Errors" - - p_val = f"{'TOTAL':<{col_widths['profile']}}" - if USE_COLORS: - p_val = f"{Colors.BOLD}{p_val}{Colors.ENDC}" - - f_val = f"{total_folders:>{col_widths['folders']}}" - r_val = f"{total_rules:>{col_widths['rules']},}" - d_val = f"{total_duration:>{col_widths['duration']-1}.1f}s" - - s_val_raw = f"{total_status_text:<{col_widths['status']}}" - if USE_COLORS: - status_color = Colors.GREEN if all_success else Colors.FAIL - s_val = f"{status_color}{s_val_raw}{Colors.ENDC}" - else: - s_val = s_val_raw - - print( - f"{chars['v']} {p_val} " - f"{chars['v']} {f_val} " - f"{chars['v']} {r_val} " - f"{chars['v']} {d_val} " - f"{chars['v']} {s_val} {chars['v']}" - ) - - _print_separator("bl", "bm", "br") def _get_progress_bar_width() -> int: @@ -1486,7 +1374,7 @@ def _gh_get(url: str) -> Dict: _cache_stats["misses"] += 1 - except httpx.HTTPStatusError as e: + except httpx.HTTPStatusError: # Re-raise with original exception (don't catch and re-raise) raise @@ -2458,21 +2346,24 @@ def print_summary_table( return # Unicode Table - def line(l, m, r): return f"{Colors.BOLD}{l}{m.join('─' * (x+2) for x in w)}{r}{Colors.ENDC}" - def row(c): return f"{Colors.BOLD}│{Colors.ENDC} {c[0]:<{w[0]}} {Colors.BOLD}│{Colors.ENDC} {c[1]:>{w[1]}} {Colors.BOLD}│{Colors.ENDC} {c[2]:>{w[2]}} {Colors.BOLD}│{Colors.ENDC} {c[3]:>{w[3]}} {Colors.BOLD}│{Colors.ENDC} {c[4]:<{w[4]}} {Colors.BOLD}│{Colors.ENDC}" + def print_line(left_char, mid_char, right_char): + return f"{Colors.BOLD}{left_char}{mid_char.join('─' * (x+2) for x in w)}{right_char}{Colors.ENDC}" + + def print_row(cols): + return f"{Colors.BOLD}│{Colors.ENDC} {cols[0]:<{w[0]}} {Colors.BOLD}│{Colors.ENDC} {cols[1]:>{w[1]}} {Colors.BOLD}│{Colors.ENDC} {cols[2]:>{w[2]}} {Colors.BOLD}│{Colors.ENDC} {cols[3]:>{w[3]}} {Colors.BOLD}│{Colors.ENDC} {cols[4]:<{w[4]}} {Colors.BOLD}│{Colors.ENDC}" - print(f"\n{line('┌', '─', '┐')}") + print(f"\n{print_line('┌', '─', '┐')}") title = f"{'DRY RUN' if dry_run else 'SYNC'} SUMMARY" print(f"{Colors.BOLD}│{Colors.CYAN if dry_run else Colors.HEADER}{title:^{sum(w) + 14}}{Colors.ENDC}{Colors.BOLD}│{Colors.ENDC}") - print(f"{line('├', '┬', '┤')}\n{row([f'{Colors.HEADER}Profile ID{Colors.ENDC}', f'{Colors.HEADER}Folders{Colors.ENDC}', f'{Colors.HEADER}Rules{Colors.ENDC}', f'{Colors.HEADER}Duration{Colors.ENDC}', f'{Colors.HEADER}Status{Colors.ENDC}'])}") - print(line("├", "┼", "┤")) + print(f"{print_line('├', '┬', '┤')}\n{print_row([f'{Colors.HEADER}Profile ID{Colors.ENDC}', f'{Colors.HEADER}Folders{Colors.ENDC}', f'{Colors.HEADER}Rules{Colors.ENDC}', f'{Colors.HEADER}Duration{Colors.ENDC}', f'{Colors.HEADER}Status{Colors.ENDC}'])}") + print(print_line("├", "┼", "┤")) for r in sync_results: sc = Colors.GREEN if r["success"] else Colors.FAIL - print(row([r["profile"], str(r["folders"]), f"{r['rules']:,}", f"{r['duration']:.1f}s", f"{sc}{r['status_label']}{Colors.ENDC}"])) + print(print_row([r["profile"], str(r["folders"]), f"{r['rules']:,}", f"{r['duration']:.1f}s", f"{sc}{r['status_label']}{Colors.ENDC}"])) - print(f"{line('├', '┼', '┤')}\n{row(['TOTAL', str(t_f), f'{t_r:,}', f'{t_d:.1f}s', f'{t_col}{t_status}{Colors.ENDC}'])}") - print(f"{line('└', '┴', '┘')}\n") + print(f"{print_line('├', '┼', '┤')}\n{print_row(['TOTAL', str(t_f), f'{t_r:,}', f'{t_d:.1f}s', f'{t_col}{t_status}{Colors.ENDC}'])}") + print(f"{print_line('└', '┴', '┘')}\n") def print_success_message(profile_ids: List[str]) -> None: diff --git a/test_main.py b/test_main.py index d585ef3..f6d9b76 100644 --- a/test_main.py +++ b/test_main.py @@ -1,7 +1,7 @@ import importlib import os import sys -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch import httpx import pytest diff --git a/tests/test_bootstrapping.py b/tests/test_bootstrapping.py index 0a40248..29a7dcd 100644 --- a/tests/test_bootstrapping.py +++ b/tests/test_bootstrapping.py @@ -1,5 +1,4 @@ import os -import sys from unittest.mock import MagicMock, patch import pytest diff --git a/tests/test_content_type.py b/tests/test_content_type.py index 30cddf8..ff00338 100644 --- a/tests/test_content_type.py +++ b/tests/test_content_type.py @@ -3,7 +3,6 @@ from unittest.mock import patch, MagicMock import sys import os -import json # Add root to path to import main sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) diff --git a/tests/test_disk_cache.py b/tests/test_disk_cache.py index 6dd0ec0..359684a 100644 --- a/tests/test_disk_cache.py +++ b/tests/test_disk_cache.py @@ -11,7 +11,6 @@ """ import json import os -import platform import tempfile import time import unittest diff --git a/tests/test_env_permissions.py b/tests/test_env_permissions.py index 9485bec..f4a7947 100644 --- a/tests/test_env_permissions.py +++ b/tests/test_env_permissions.py @@ -2,7 +2,7 @@ import os import stat -from unittest.mock import MagicMock, patch, call +from unittest.mock import MagicMock, patch # Set TOKEN before importing main to avoid issues with load_dotenv() diff --git a/tests/test_fix_env.py b/tests/test_fix_env.py index c371e80..d7b78c4 100644 --- a/tests/test_fix_env.py +++ b/tests/test_fix_env.py @@ -1,7 +1,6 @@ import os -import stat import pytest -from unittest.mock import MagicMock, patch +from unittest.mock import patch import fix_env def test_fix_env_skips_symlink(tmp_path): diff --git a/tests/test_folder_validation.py b/tests/test_folder_validation.py index 368b367..a35e12c 100644 --- a/tests/test_folder_validation.py +++ b/tests/test_folder_validation.py @@ -1,6 +1,5 @@ from unittest.mock import MagicMock -import pytest import main diff --git a/tests/test_hostname_validation.py b/tests/test_hostname_validation.py index 3e0cd70..ee1b88a 100644 --- a/tests/test_hostname_validation.py +++ b/tests/test_hostname_validation.py @@ -1,8 +1,7 @@ import socket -from unittest.mock import MagicMock, patch +from unittest.mock import patch -import pytest import main def test_validate_hostname_caching(): diff --git a/tests/test_performance_regression.py b/tests/test_performance_regression.py index a39e255..4bc4191 100644 --- a/tests/test_performance_regression.py +++ b/tests/test_performance_regression.py @@ -8,7 +8,6 @@ import sys import os -import pytest import httpx sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) diff --git a/tests/test_plan_details.py b/tests/test_plan_details.py index 4f87139..4140596 100644 --- a/tests/test_plan_details.py +++ b/tests/test_plan_details.py @@ -2,7 +2,6 @@ import importlib import os -import sys from unittest.mock import patch @@ -18,8 +17,9 @@ def test_print_plan_details_no_colors(capsys): plan_entry = { "profile": "test_profile", "folders": [ - {"name": "Folder B", "rules": 5}, - {"name": "Folder A", "rules": 10}, + {"name": "Folder B", "rules": 5, "action": 0}, + {"name": "Folder A", "rules": 10, "action": 1}, + {"name": "Folder C", "rules": 3, "rule_groups": [{"action": 0}, {"action": 1}]}, ], } m.print_plan_details(plan_entry) @@ -29,10 +29,12 @@ def test_print_plan_details_no_colors(capsys): assert "Plan Details for test_profile:" in output # Match exact output including alignment spaces - assert " - Folder A : 10 rules" in output - assert " - Folder B : 5 rules" in output - # Verify alphabetical ordering (A before B) + assert " - Folder A : 10 rules [Allow]" in output + assert " - Folder B : 5 rules [Block]" in output + assert " - Folder C : 3 rules [Mixed]" in output + # Verify alphabetical ordering (A before B before C) assert output.index("Folder A") < output.index("Folder B") + assert output.index("Folder B") < output.index("Folder C") def test_print_plan_details_empty_folders(capsys): @@ -65,7 +67,7 @@ def test_print_plan_details_with_colors(capsys): plan_entry = { "profile": "test_profile", - "folders": [{"name": "Folder A", "rules": 10}], + "folders": [{"name": "Folder A", "rules": 10, "action": 1}], } m.print_plan_details(plan_entry) @@ -75,3 +77,4 @@ def test_print_plan_details_with_colors(capsys): assert "📝 Plan Details for test_profile:" in output assert "Folder A" in output assert "10 rules" in output + assert "✅ Allow" in output diff --git a/tests/test_push_rules_perf.py b/tests/test_push_rules_perf.py index 9d42d68..bae1fce 100644 --- a/tests/test_push_rules_perf.py +++ b/tests/test_push_rules_perf.py @@ -1,12 +1,11 @@ import unittest -from unittest.mock import MagicMock, patch, ANY +from unittest.mock import MagicMock, patch import sys import os # Add root to path to import main sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -import main class TestPushRulesPerf(unittest.TestCase): def setUp(self): diff --git a/tests/test_ssrf.py b/tests/test_ssrf.py index 53221ec..f613b6a 100644 --- a/tests/test_ssrf.py +++ b/tests/test_ssrf.py @@ -1,5 +1,5 @@ import unittest -from unittest.mock import patch, MagicMock +from unittest.mock import patch import sys import os import socket diff --git a/tests/test_symlink.py b/tests/test_symlink.py index d66caa6..438c0de 100644 --- a/tests/test_symlink.py +++ b/tests/test_symlink.py @@ -1,7 +1,6 @@ import os -import stat import pytest -from unittest.mock import MagicMock, patch +from unittest.mock import patch import main def test_check_env_permissions_skips_symlink(tmp_path): diff --git a/uv.lock b/uv.lock index 53e9b38..ebe769c 100644 --- a/uv.lock +++ b/uv.lock @@ -45,6 +45,7 @@ dependencies = [ [package.optional-dependencies] dev = [ { name = "pytest" }, + { name = "pytest-benchmark" }, { name = "pytest-mock" }, { name = "pytest-xdist" }, ] @@ -53,6 +54,7 @@ dev = [ requires-dist = [ { name = "httpx", specifier = ">=0.28.1" }, { name = "pytest", marker = "extra == 'dev'", specifier = ">=7.0.0" }, + { name = "pytest-benchmark", marker = "extra == 'dev'", specifier = ">=4.0.0" }, { name = "pytest-mock", marker = "extra == 'dev'", specifier = ">=3.10.0" }, { name = "pytest-xdist", marker = "extra == 'dev'", specifier = ">=3.0.0" }, { name = "python-dotenv", specifier = ">=1.1.1" }, @@ -141,6 +143,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/54/20/4d324d65cc6d9205fabedc306948156824eb9f0ee1633355a8f7ec5c66bf/pluggy-1.6.0-py3-none-any.whl", hash = "sha256:e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746", size = 20538, upload-time = "2025-05-15T12:30:06.134Z" }, ] +[[package]] +name = "py-cpuinfo" +version = "9.0.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/37/a8/d832f7293ebb21690860d2e01d8115e5ff6f2ae8bbdc953f0eb0fa4bd2c7/py-cpuinfo-9.0.0.tar.gz", hash = "sha256:3cdbbf3fac90dc6f118bfd64384f309edeadd902d7c8fb17f02ffa1fc3f49690", size = 104716, upload-time = "2022-10-25T20:38:06.303Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/e0/a9/023730ba63db1e494a271cb018dcd361bd2c917ba7004c3e49d5daf795a2/py_cpuinfo-9.0.0-py3-none-any.whl", hash = "sha256:859625bc251f64e21f077d099d4162689c762b5d6a4c3c97553d56241c9674d5", size = 22335, upload-time = "2022-10-25T20:38:27.636Z" }, +] + [[package]] name = "pygments" version = "2.19.2" @@ -166,6 +177,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/3b/ab/b3226f0bd7cdcf710fbede2b3548584366da3b19b5021e74f5bde2a8fa3f/pytest-9.0.2-py3-none-any.whl", hash = "sha256:711ffd45bf766d5264d487b917733b453d917afd2b0ad65223959f59089f875b", size = 374801, upload-time = "2025-12-06T21:30:49.154Z" }, ] +[[package]] +name = "pytest-benchmark" +version = "5.2.3" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "py-cpuinfo" }, + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/24/34/9f732b76456d64faffbef6232f1f9dbec7a7c4999ff46282fa418bd1af66/pytest_benchmark-5.2.3.tar.gz", hash = "sha256:deb7317998a23c650fd4ff76e1230066a76cb45dcece0aca5607143c619e7779", size = 341340, upload-time = "2025-11-09T18:48:43.215Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/33/29/e756e715a48959f1c0045342088d7ca9762a2f509b945f362a316e9412b7/pytest_benchmark-5.2.3-py3-none-any.whl", hash = "sha256:bc839726ad20e99aaa0d11a127445457b4219bdb9e80a1afc4b51da7f96b0803", size = 45255, upload-time = "2025-11-09T18:48:39.765Z" }, +] + [[package]] name = "pytest-mock" version = "3.15.1"