From f24e53d10ce7ae84aa5b710c1238245dc83f10f0 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Tue, 11 Aug 2026 23:08:49 +0000 Subject: [PATCH 1/5] refactor(profiler): clarify module resolution filtering and variable names --- scripts/import_profiler/profiler.py | 70 ++++++++++++++++++++---- scripts/import_profiler/test_profiler.py | 36 ++++++++++++ 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/scripts/import_profiler/profiler.py b/scripts/import_profiler/profiler.py index 72b73e1a755e..844b0b8fd0f8 100644 --- a/scripts/import_profiler/profiler.py +++ b/scripts/import_profiler/profiler.py @@ -377,6 +377,53 @@ def validate_module_name(module_name): raise argparse.ArgumentTypeError(f"'{module_name}' is not a valid Python module identifier.") return module_name +PARENT_NAMESPACES = ("google", "google.cloud") +IGNORED_TOP_LEVEL_NAMES = { + "tests", "samples", "examples", "benchmark", "benchmarks", "third_party", + "testing", "test_utils", "docs", "build", "dist", "bin", "ci", "scripts", + "cloudbuild", "notebooks", "assets", "scratch", "specs" +} +IGNORED_NAME_PREFIXES = ( + "test_", "tests_", "sample_", "samples_", "bench_", "benchmarks_", + "example_", "examples_", "doc_", "docs_", "notebook_", "notebooks_" +) + + +def _should_ignore_namespace_package(top_level, full_package, target_pkg): + """Determines if a discovered package path should be excluded from module selection. + + Args: + top_level: Top-level directory component of the package (e.g. 'tests' or 'google'). + full_package: Full namespace package path (e.g. 'google.cloud.storage' or 'google'). + target_pkg: The distribution package being profiled (e.g. 'google-cloud-storage' or 'google-cloud-testutils'). + + Returns: + True if the namespace package should be ignored, False otherwise. + """ + # Parent namespace containers (e.g. 'google', 'google.cloud') are non-leaf packages, not concrete library modules. + if full_package in PARENT_NAMESPACES: + return True + + # Check if the top-level directory matches non-library folders (exact match) or prefixes (e.g. test_*) + is_non_library_dir = ( + top_level in IGNORED_TOP_LEVEL_NAMES + or top_level.startswith(IGNORED_NAME_PREFIXES) + ) + + if not is_non_library_dir: + return False + + # Exception: If the top-level folder name is part of the target package name + # (e.g. top_level='test_utils' when target_pkg='google-cloud-testutils'), do not ignore it. + normalized_target_pkg = target_pkg.replace("-", "").replace("_", "").lower() + normalized_top_level = top_level.replace("-", "").replace("_", "").lower() + + if normalized_top_level in normalized_target_pkg: + return False + + return True + + def find_module_from_package(pkg): import importlib.metadata import importlib.util @@ -385,10 +432,17 @@ def find_module_from_package(pkg): try: files = importlib.metadata.files(pkg) if files: - ignored_parts = {'tests', 'testing', 'samples', 'examples', 'benchmark', 'benchmarks', 'third_party', 'test_utils', 'docs', 'build', 'dist', 'bin', 'ci', 'scripts', 'cloudbuild', 'notebooks', 'assets', 'scratch', 'specs'} - if pkg == "google-cloud-testutils": - ignored_parts.discard('test_utils') - init_files = [str(f) for f in files if str(f).endswith('__init__.py') and '__pycache__' not in str(f) and not any(part in ignored_parts for part in str(f).replace('\\', '/').split('/'))] + pkg_norm = pkg.replace("-", "").replace("_", "").lower() + init_files = [ + str(f) for f in files + if str(f).endswith('__init__.py') + and '__pycache__' not in str(f) + and not any( + part in IGNORED_TOP_LEVEL_NAMES + and part.replace("-", "").replace("_", "").lower() not in pkg_norm + for part in str(f).replace('\\', '/').split('/') + ) + ] if init_files: from pathlib import Path shortest_init = min(init_files, key=lambda p: len(Path(p).parts)) @@ -412,16 +466,10 @@ def find_module_from_package(pkg): if abs_where_dir not in sys.path: sys.path.insert(0, abs_where_dir) pkgs = setuptools.find_namespace_packages(where=where_dir) - ignored_prefixes = ("tests", "samples", "examples", "benchmark", "benchmarks", "third_party", "testing", "test_utils", "docs", "build", "dist", "bin", "ci", "scripts", "cloudbuild", "notebooks", "assets", "scratch", "specs") - ignored_starts = ("test_", "tests_", "sample_", "samples_", "bench_", "benchmarks_", "example_", "examples_", "doc_", "docs_", "notebook_", "notebooks_") - filtered = [] for p in pkgs: top = p.split(".")[0] - is_ignored_top = top in ignored_prefixes or top.startswith(ignored_starts) - if is_ignored_top and pkg == "google-cloud-testutils" and top == "test_utils": - is_ignored_top = False - if is_ignored_top or p in ("google", "google.cloud"): + if _should_ignore_namespace_package(top, p, pkg): continue filtered.append(p) diff --git a/scripts/import_profiler/test_profiler.py b/scripts/import_profiler/test_profiler.py index 49e94b602b7a..f8cecfd43b98 100644 --- a/scripts/import_profiler/test_profiler.py +++ b/scripts/import_profiler/test_profiler.py @@ -830,4 +830,40 @@ def test_cli_main_options(): runpy.run_path(profiler_path, run_name="__main__") +def test_should_ignore_namespace_package(): + from profiler import _should_ignore_namespace_package + + # Parent namespace roots should be ignored + assert _should_ignore_namespace_package("google", "google", "google-cloud-storage") is True + assert _should_ignore_namespace_package("google", "google.cloud", "google-cloud-storage") is True + + # Standard non-library top-level directories should be ignored + assert _should_ignore_namespace_package("tests", "tests", "google-cloud-storage") is True + assert _should_ignore_namespace_package("samples", "samples", "google-cloud-storage") is True + assert _should_ignore_namespace_package("test_utils", "test_utils", "google-cloud-storage") is True + assert _should_ignore_namespace_package("test_helpers", "test_helpers", "google-cloud-storage") is True + + # Exception: Target package explicitly contains the top-level directory name (e.g. google-cloud-testutils -> test_utils) + assert _should_ignore_namespace_package("test_utils", "test_utils", "google-cloud-testutils") is False + + # Valid library package should not be ignored + assert _should_ignore_namespace_package("google", "google.cloud.storage", "google-cloud-storage") is False + assert _should_ignore_namespace_package("my_library", "my_library", "my-library") is False + + +def test_find_module_from_package_testutils(): + """Verifies that google-cloud-testutils correctly resolves to test_utils namespace package.""" + sys.modules.setdefault("setuptools", MagicMock()) + with patch("importlib.metadata.files", side_effect=Exception), \ + patch("profiler.os.path.exists", return_value=True), \ + patch("profiler.os.path.isdir", return_value=True), \ + patch("setuptools.find_namespace_packages", return_value=["google", "google.cloud", "tests", "test_utils"]) as mock_find, \ + patch("profiler.os.path.isfile", return_value=True), \ + patch("importlib.util.find_spec", return_value=True): + res = find_module_from_package("google-cloud-testutils") + assert res == "test_utils" + mock_find.assert_called_once_with(where=".") + + + From baadb13a006d713e68e366ed11c4058aae96cb41 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Tue, 11 Aug 2026 23:09:14 +0000 Subject: [PATCH 2/5] test(profiler): update assertion in test_find_module_from_package_testutils --- scripts/import_profiler/test_profiler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/import_profiler/test_profiler.py b/scripts/import_profiler/test_profiler.py index f8cecfd43b98..4dc02a36a731 100644 --- a/scripts/import_profiler/test_profiler.py +++ b/scripts/import_profiler/test_profiler.py @@ -862,7 +862,7 @@ def test_find_module_from_package_testutils(): patch("importlib.util.find_spec", return_value=True): res = find_module_from_package("google-cloud-testutils") assert res == "test_utils" - mock_find.assert_called_once_with(where=".") + mock_find.assert_called_once_with(where="src") From 6b41730cca719ddba83d08cae8512a8c8b03c2c5 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Tue, 11 Aug 2026 23:12:01 +0000 Subject: [PATCH 3/5] refactor(profiler): remove PARENT_NAMESPACES constant and rely on init presence --- scripts/import_profiler/profiler.py | 15 +++--------- scripts/import_profiler/test_profiler.py | 31 ++++++++++++------------ 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/scripts/import_profiler/profiler.py b/scripts/import_profiler/profiler.py index 844b0b8fd0f8..eaf86e76f490 100644 --- a/scripts/import_profiler/profiler.py +++ b/scripts/import_profiler/profiler.py @@ -377,7 +377,6 @@ def validate_module_name(module_name): raise argparse.ArgumentTypeError(f"'{module_name}' is not a valid Python module identifier.") return module_name -PARENT_NAMESPACES = ("google", "google.cloud") IGNORED_TOP_LEVEL_NAMES = { "tests", "samples", "examples", "benchmark", "benchmarks", "third_party", "testing", "test_utils", "docs", "build", "dist", "bin", "ci", "scripts", @@ -389,22 +388,16 @@ def validate_module_name(module_name): ) -def _should_ignore_namespace_package(top_level, full_package, target_pkg): - """Determines if a discovered package path should be excluded from module selection. +def _should_ignore_namespace_package(top_level, target_pkg): + """Determines if a discovered top-level directory should be excluded from module selection. Args: top_level: Top-level directory component of the package (e.g. 'tests' or 'google'). - full_package: Full namespace package path (e.g. 'google.cloud.storage' or 'google'). target_pkg: The distribution package being profiled (e.g. 'google-cloud-storage' or 'google-cloud-testutils'). Returns: - True if the namespace package should be ignored, False otherwise. + True if the top-level directory represents a non-library folder, False otherwise. """ - # Parent namespace containers (e.g. 'google', 'google.cloud') are non-leaf packages, not concrete library modules. - if full_package in PARENT_NAMESPACES: - return True - - # Check if the top-level directory matches non-library folders (exact match) or prefixes (e.g. test_*) is_non_library_dir = ( top_level in IGNORED_TOP_LEVEL_NAMES or top_level.startswith(IGNORED_NAME_PREFIXES) @@ -469,7 +462,7 @@ def find_module_from_package(pkg): filtered = [] for p in pkgs: top = p.split(".")[0] - if _should_ignore_namespace_package(top, p, pkg): + if _should_ignore_namespace_package(top, pkg): continue filtered.append(p) diff --git a/scripts/import_profiler/test_profiler.py b/scripts/import_profiler/test_profiler.py index 4dc02a36a731..1bf4e7bdc494 100644 --- a/scripts/import_profiler/test_profiler.py +++ b/scripts/import_profiler/test_profiler.py @@ -650,10 +650,13 @@ def test_find_module_from_package_metadata_test_utils(): def test_find_module_from_package_setuptools(): sys.modules.setdefault("setuptools", MagicMock()) + def mock_isfile(path): + return "my_pkg" in path with patch("importlib.metadata.files", side_effect=Exception), \ - patch("os.path.exists", return_value=True), \ + patch("profiler.os.path.exists", return_value=True), \ + patch("profiler.os.path.isdir", return_value=True), \ patch("setuptools.find_namespace_packages", return_value=["google", "google.cloud", "tests.dummy", "my_pkg"]), \ - patch("os.path.isfile", return_value=True), \ + patch("profiler.os.path.isfile", side_effect=mock_isfile), \ patch("importlib.util.find_spec", return_value=True): res = find_module_from_package("my-pkg") assert res == "my_pkg" @@ -833,32 +836,30 @@ def test_cli_main_options(): def test_should_ignore_namespace_package(): from profiler import _should_ignore_namespace_package - # Parent namespace roots should be ignored - assert _should_ignore_namespace_package("google", "google", "google-cloud-storage") is True - assert _should_ignore_namespace_package("google", "google.cloud", "google-cloud-storage") is True - # Standard non-library top-level directories should be ignored - assert _should_ignore_namespace_package("tests", "tests", "google-cloud-storage") is True - assert _should_ignore_namespace_package("samples", "samples", "google-cloud-storage") is True - assert _should_ignore_namespace_package("test_utils", "test_utils", "google-cloud-storage") is True - assert _should_ignore_namespace_package("test_helpers", "test_helpers", "google-cloud-storage") is True + assert _should_ignore_namespace_package("tests", "google-cloud-storage") is True + assert _should_ignore_namespace_package("samples", "google-cloud-storage") is True + assert _should_ignore_namespace_package("test_utils", "google-cloud-storage") is True + assert _should_ignore_namespace_package("test_helpers", "google-cloud-storage") is True # Exception: Target package explicitly contains the top-level directory name (e.g. google-cloud-testutils -> test_utils) - assert _should_ignore_namespace_package("test_utils", "test_utils", "google-cloud-testutils") is False + assert _should_ignore_namespace_package("test_utils", "google-cloud-testutils") is False - # Valid library package should not be ignored - assert _should_ignore_namespace_package("google", "google.cloud.storage", "google-cloud-storage") is False - assert _should_ignore_namespace_package("my_library", "my_library", "my-library") is False + # Valid library package top-level should not be ignored + assert _should_ignore_namespace_package("google", "google-cloud-storage") is False + assert _should_ignore_namespace_package("my_library", "my-library") is False def test_find_module_from_package_testutils(): """Verifies that google-cloud-testutils correctly resolves to test_utils namespace package.""" sys.modules.setdefault("setuptools", MagicMock()) + def mock_isfile(path): + return "test_utils" in path with patch("importlib.metadata.files", side_effect=Exception), \ patch("profiler.os.path.exists", return_value=True), \ patch("profiler.os.path.isdir", return_value=True), \ patch("setuptools.find_namespace_packages", return_value=["google", "google.cloud", "tests", "test_utils"]) as mock_find, \ - patch("profiler.os.path.isfile", return_value=True), \ + patch("profiler.os.path.isfile", side_effect=mock_isfile), \ patch("importlib.util.find_spec", return_value=True): res = find_module_from_package("google-cloud-testutils") assert res == "test_utils" From 6dfedb27a33ba9c2d4dcaf565e01ca6c63d9d18f Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Thu, 13 Aug 2026 17:37:36 +0000 Subject: [PATCH 4/5] refactor(profiler): use positive package selection logic per PR feedback --- scripts/import_profiler/profiler.py | 33 ++++++++++-------------- scripts/import_profiler/test_profiler.py | 22 ++++++++-------- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/scripts/import_profiler/profiler.py b/scripts/import_profiler/profiler.py index eaf86e76f490..d52a0bf0989e 100644 --- a/scripts/import_profiler/profiler.py +++ b/scripts/import_profiler/profiler.py @@ -388,33 +388,29 @@ def validate_module_name(module_name): ) -def _should_ignore_namespace_package(top_level, target_pkg): - """Determines if a discovered top-level directory should be excluded from module selection. +def _should_process_namespace_package(top_level: str, target_pkg: str) -> bool: + """Determines if a discovered top-level directory should be processed. Args: top_level: Top-level directory component of the package (e.g. 'tests' or 'google'). - target_pkg: The distribution package being profiled (e.g. 'google-cloud-storage' or 'google-cloud-testutils'). + target_pkg: The distribution package being profiled (e.g. 'google-cloud-storage'). Returns: - True if the top-level directory represents a non-library folder, False otherwise. + True if the top-level directory should be processed, False otherwise. """ - is_non_library_dir = ( + is_excluded_candidate = ( top_level in IGNORED_TOP_LEVEL_NAMES or top_level.startswith(IGNORED_NAME_PREFIXES) ) - if not is_non_library_dir: - return False + if is_excluded_candidate: + normalized_target_pkg = target_pkg.replace("-", "").replace("_", "").lower() + normalized_top_level = top_level.replace("-", "").replace("_", "").lower() + # Note: If the top-level folder name is part of the target package name + # (e.g. top_level='test_utils' when target_pkg='google-cloud-testutils'), then it should be processed. + return normalized_top_level in normalized_target_pkg - # Exception: If the top-level folder name is part of the target package name - # (e.g. top_level='test_utils' when target_pkg='google-cloud-testutils'), do not ignore it. - normalized_target_pkg = target_pkg.replace("-", "").replace("_", "").lower() - normalized_top_level = top_level.replace("-", "").replace("_", "").lower() - - if normalized_top_level in normalized_target_pkg: - return False - - return True + return True # Not a candidate for exclusion, process it. def find_module_from_package(pkg): @@ -462,9 +458,8 @@ def find_module_from_package(pkg): filtered = [] for p in pkgs: top = p.split(".")[0] - if _should_ignore_namespace_package(top, pkg): - continue - filtered.append(p) + if _should_process_namespace_package(top, pkg): + filtered.append(p) # First preference: packages containing __init__.py for p in sorted(filtered, key=len): diff --git a/scripts/import_profiler/test_profiler.py b/scripts/import_profiler/test_profiler.py index 1bf4e7bdc494..871f5a0b810e 100644 --- a/scripts/import_profiler/test_profiler.py +++ b/scripts/import_profiler/test_profiler.py @@ -833,21 +833,21 @@ def test_cli_main_options(): runpy.run_path(profiler_path, run_name="__main__") -def test_should_ignore_namespace_package(): - from profiler import _should_ignore_namespace_package +def test_should_process_namespace_package(): + from profiler import _should_process_namespace_package - # Standard non-library top-level directories should be ignored - assert _should_ignore_namespace_package("tests", "google-cloud-storage") is True - assert _should_ignore_namespace_package("samples", "google-cloud-storage") is True - assert _should_ignore_namespace_package("test_utils", "google-cloud-storage") is True - assert _should_ignore_namespace_package("test_helpers", "google-cloud-storage") is True + # Standard non-library top-level directories should NOT be processed + assert _should_process_namespace_package("tests", "google-cloud-storage") is False + assert _should_process_namespace_package("samples", "google-cloud-storage") is False + assert _should_process_namespace_package("test_utils", "google-cloud-storage") is False + assert _should_process_namespace_package("test_helpers", "google-cloud-storage") is False # Exception: Target package explicitly contains the top-level directory name (e.g. google-cloud-testutils -> test_utils) - assert _should_ignore_namespace_package("test_utils", "google-cloud-testutils") is False + assert _should_process_namespace_package("test_utils", "google-cloud-testutils") is True - # Valid library package top-level should not be ignored - assert _should_ignore_namespace_package("google", "google-cloud-storage") is False - assert _should_ignore_namespace_package("my_library", "my-library") is False + # Valid library package top-level should be processed + assert _should_process_namespace_package("google", "google-cloud-storage") is True + assert _should_process_namespace_package("my_library", "my-library") is True def test_find_module_from_package_testutils(): From 906313f19e0dea815431b9bfd26612b067e59ea3 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Thu, 13 Aug 2026 17:48:45 +0000 Subject: [PATCH 5/5] refactor(profiler): simplify init_files list comprehension using _is_path_allowed helper per PR feedback --- scripts/import_profiler/profiler.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/scripts/import_profiler/profiler.py b/scripts/import_profiler/profiler.py index d52a0bf0989e..8ccfd7b218a9 100644 --- a/scripts/import_profiler/profiler.py +++ b/scripts/import_profiler/profiler.py @@ -413,9 +413,21 @@ def _should_process_namespace_package(top_level: str, target_pkg: str) -> bool: return True # Not a candidate for exclusion, process it. +def _is_path_allowed(path_obj, pkg_norm: str) -> bool: + """Checks if a path object is allowed based on ignored directory rules.""" + from pathlib import Path + parts = Path(path_obj).parts + return all( + part not in IGNORED_TOP_LEVEL_NAMES + or part.replace("-", "").replace("_", "").lower() in pkg_norm + for part in parts + ) + + def find_module_from_package(pkg): import importlib.metadata import importlib.util + from pathlib import Path # 1. Try to use importlib.metadata.files (works for standard installations from PyPI/wheels) try: @@ -424,13 +436,9 @@ def find_module_from_package(pkg): pkg_norm = pkg.replace("-", "").replace("_", "").lower() init_files = [ str(f) for f in files - if str(f).endswith('__init__.py') - and '__pycache__' not in str(f) - and not any( - part in IGNORED_TOP_LEVEL_NAMES - and part.replace("-", "").replace("_", "").lower() not in pkg_norm - for part in str(f).replace('\\', '/').split('/') - ) + if Path(f).name == '__init__.py' + and '__pycache__' not in Path(f).parts + and _is_path_allowed(f, pkg_norm) ] if init_files: from pathlib import Path