From a240460fc78fd6cb17bcb61a427ff08ec0c69dd9 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk <21087696+oleksandr-pavlyk@users.noreply.github.com> Date: Mon, 29 Jun 2026 16:34:45 -0500 Subject: [PATCH] Fix unexpected AttributeError when --no-color is used Implemented robo-review feedback suggesting to expand coverage of lazy loading helper. --- python/scripts/nvbench_compare.py | 25 +++++--- python/test/test_nvbench_compare.py | 6 +- python/test/test_nvbench_tooling_deps.py | 80 ++++++++++++++++++++---- 3 files changed, 90 insertions(+), 21 deletions(-) diff --git a/python/scripts/nvbench_compare.py b/python/scripts/nvbench_compare.py index 593c85c..054a564 100644 --- a/python/scripts/nvbench_compare.py +++ b/python/scripts/nvbench_compare.py @@ -2767,14 +2767,23 @@ def is_usable_noise(noise): def colorize_comparison_status(status, no_color): if status == ComparisonStatus.UNKNOWN: - return colorize(status.value, Fore.YELLOW, Emoji.YELLOW, no_color) - if status == ComparisonStatus.UNDECIDED: - return colorize(status.value, Fore.LIGHTBLACK_EX, Emoji.SHRUG, no_color) - if status == ComparisonStatus.SAME: - return colorize(status.value, Fore.BLUE, Emoji.BLUE, no_color) - if status == ComparisonStatus.FAST: - return colorize(status.value, Fore.GREEN, Emoji.GREEN, no_color) - return colorize(status.value, Fore.RED, Emoji.RED, no_color) + fore_name = "YELLOW" + emoji = Emoji.YELLOW + elif status == ComparisonStatus.UNDECIDED: + fore_name = "LIGHTBLACK_EX" + emoji = Emoji.SHRUG + elif status == ComparisonStatus.SAME: + fore_name = "BLUE" + emoji = Emoji.BLUE + elif status == ComparisonStatus.FAST: + fore_name = "GREEN" + emoji = Emoji.GREEN + else: + fore_name = "RED" + emoji = Emoji.RED + + fore = "" if no_color else getattr(Fore, fore_name) + return colorize(status.value, fore, emoji, no_color) def format_axis_values(axis_values, axes, axis_filters=None): diff --git a/python/test/test_nvbench_compare.py b/python/test/test_nvbench_compare.py index d2304e4..b47f53d 100644 --- a/python/test/test_nvbench_compare.py +++ b/python/test/test_nvbench_compare.py @@ -1494,7 +1494,11 @@ def test_format_change_only_reports_fast_and_slow_rows(nvbench_compare): assert nvbench_compare.format_change(undecided) == "" -def test_ambiguous_status_uses_shrug_marker(nvbench_compare): +def test_ambiguous_status_uses_shrug_marker_without_colorama( + monkeypatch, nvbench_compare +): + monkeypatch.setattr(nvbench_compare, "Fore", None) + assert ( nvbench_compare.colorize_comparison_status( nvbench_compare.ComparisonStatus.UNDECIDED, no_color=True diff --git a/python/test/test_nvbench_tooling_deps.py b/python/test/test_nvbench_tooling_deps.py index f839ab6..05ea2a1 100644 --- a/python/test/test_nvbench_tooling_deps.py +++ b/python/test/test_nvbench_tooling_deps.py @@ -17,25 +17,57 @@ def tooling_deps(monkeypatch): return importlib.import_module("nvbench_tooling_deps") -def test_tooling_deps_imports_from_packaged_script_path(tmp_path, monkeypatch): +def make_packaged_scripts_tree(tmp_path: Path) -> Path: scripts_dir = Path(__file__).resolve().parents[1] / "scripts" package_dir = tmp_path / "cuda" / "bench" / "scripts" + results_dir = tmp_path / "cuda" / "bench" / "results" package_dir.mkdir(parents=True) - for package in [tmp_path / "cuda", tmp_path / "cuda" / "bench", package_dir]: + results_dir.mkdir() + for package in [ + tmp_path / "cuda", + tmp_path / "cuda" / "bench", + package_dir, + results_dir, + ]: (package / "__init__.py").write_text("", encoding="utf-8") - shutil.copy( - scripts_dir / "nvbench_tooling_deps.py", - package_dir / "nvbench_tooling_deps.py", + for filename in [ + "nvbench_compare.py", + "nvbench_histogram.py", + "nvbench_json_summary.py", + "nvbench_tooling_deps.py", + "nvbench_walltime.py", + ]: + shutil.copy(scripts_dir / filename, package_dir / filename) + shutil.copytree( + scripts_dir / "nvbench_json", + package_dir / "nvbench_json", + ignore=shutil.ignore_patterns("__pycache__"), ) + (results_dir / "__init__.py").write_text( + "class BenchmarkResult:\n" + " pass\n" + "class BenchmarkResultSummary:\n" + " pass\n" + "class SubBenchmarkResult:\n" + " pass\n" + "class SubBenchmarkState:\n" + " pass\n", + encoding="utf-8", + ) + return package_dir + + +def clear_packaged_cuda_modules(monkeypatch): + for module_name in list(sys.modules): + if module_name == "cuda" or module_name.startswith("cuda."): + monkeypatch.delitem(sys.modules, module_name, raising=False) + + +def test_tooling_deps_imports_from_packaged_script_path(tmp_path, monkeypatch): + package_dir = make_packaged_scripts_tree(tmp_path) monkeypatch.syspath_prepend(str(tmp_path)) - for module_name in [ - "cuda", - "cuda.bench", - "cuda.bench.scripts", - "cuda.bench.scripts.nvbench_tooling_deps", - ]: - monkeypatch.delitem(sys.modules, module_name, raising=False) + clear_packaged_cuda_modules(monkeypatch) module = importlib.import_module("cuda.bench.scripts.nvbench_tooling_deps") @@ -43,6 +75,30 @@ def test_tooling_deps_imports_from_packaged_script_path(tmp_path, monkeypatch): assert module.ToolingDependency("math", "math", "testing").extra == "tools" +@pytest.mark.parametrize( + ("module_name", "expected_entry"), + [ + ("cuda.bench.scripts.nvbench_compare", "main"), + ("cuda.bench.scripts.nvbench_histogram", "main"), + ("cuda.bench.scripts.nvbench_json_summary", "main"), + ("cuda.bench.scripts.nvbench_walltime", "main"), + ], +) +def test_console_script_modules_import_from_packaged_paths( + tmp_path, monkeypatch, module_name, expected_entry +): + package_dir = make_packaged_scripts_tree(tmp_path) + leaf_module = module_name.rsplit(".", 1)[-1] + + monkeypatch.syspath_prepend(str(tmp_path)) + clear_packaged_cuda_modules(monkeypatch) + + module = importlib.import_module(module_name) + + assert Path(module.__file__) == package_dir / f"{leaf_module}.py" + assert callable(getattr(module, expected_entry)) + + def test_require_tooling_dependency_returns_loaded_module(tooling_deps): module = tooling_deps.require_tooling_dependency( tooling_deps.ToolingDependency("math", "math", "testing"),