From 086c03b8f23d7e56a9d7fe894efbdc91de53b9be Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Mon, 20 Jul 2026 21:44:47 +0000 Subject: [PATCH 1/7] perf(ci): skip loaded lines calculation on iterations after the first --- scripts/import_profiler/profiler.py | 71 ++++++++++++++++------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/scripts/import_profiler/profiler.py b/scripts/import_profiler/profiler.py index d2734dadce88..7725bcfaf519 100644 --- a/scripts/import_profiler/profiler.py +++ b/scripts/import_profiler/profiler.py @@ -40,7 +40,7 @@ def get_rss_mb(): except Exception: return 0.0 -def run_worker(target_module): +def run_worker(target_module, skip_line_count=False): """Performs ONE import and returns metrics.""" tracemalloc.start() importlib.invalidate_caches() @@ -66,33 +66,36 @@ def run_worker(target_module): new_modules = modules_after - modules_before loaded_lines = 0 - for m in new_modules: - try: - file_path = sys.modules[m].__file__ - if not file_path: - continue + if not skip_line_count: + for m in new_modules: + try: + file_path = sys.modules[m].__file__ + if not file_path: + continue - if file_path.endswith('.pyc'): - try: - file_path = importlib.util.source_from_cache(file_path) - except ValueError: - # Raised if the .pyc path does not follow standard PEP 3147/488 conventions. - # We pass silently because the unresolved file_path will still end in '.pyc', - # meaning the subsequent '.endswith('.py')' check will fail and safely skip - # trying to count lines in a binary file. - pass - if file_path.endswith('.py'): - try: - with open(file_path, 'r', encoding='utf-8', errors='ignore') as f: - loaded_lines += sum(1 for _ in f) - except OSError as e: - print(f"WARNING: Failed to read lines from {file_path}: {e}", file=sys.stderr) - except KeyError: - # Module disappeared from sys.modules during execution - pass - except AttributeError: - # Module has no __file__ attribute (likely a C-extension or built-in) - pass + if file_path.endswith('.pyc'): + try: + file_path = importlib.util.source_from_cache(file_path) + except ValueError: + # Raised if the .pyc path does not follow standard PEP 3147/488 conventions. + # We pass silently because the unresolved file_path will still end in '.pyc', + # meaning the subsequent '.endswith('.py')' check will fail and safely skip + # trying to count lines in a binary file. + pass + if file_path.endswith('.py'): + try: + with open(file_path, 'r', encoding='utf-8', errors='ignore') as f: + loaded_lines += sum(1 for _ in f) + except OSError as e: + print(f"WARNING: Failed to read lines from {file_path}: {e}", file=sys.stderr) + except KeyError: + # Module disappeared from sys.modules during execution + pass + except AttributeError: + # Module has no __file__ attribute (likely a C-extension or built-in) + pass + else: + loaded_lines = -1 # Output to stdout for the Master to capture metrics = { @@ -106,6 +109,8 @@ def run_worker(target_module): def _run_worker_and_parse(cmd): result = subprocess.run(cmd, capture_output=True, text=True, check=True) + if result.stderr.strip(): + print(result.stderr.strip(), file=sys.stderr) try: lines = result.stdout.strip().splitlines() data = None @@ -185,6 +190,8 @@ def run_master(iterations, target_module, cpu=0, csv_path=None, clear_cache=True cmd += ["taskset", "-c", str(cpu)] cmd += python_exe + [__file__, "--worker", f"--module={target_module}"] + if i > 0: + cmd += ["--skip-line-count"] try: data = _run_worker_and_parse(cmd) @@ -194,11 +201,12 @@ def run_master(iterations, target_module, cpu=0, csv_path=None, clear_cache=True print(f"Iteration {i+1}/{iterations} completed in {data['time_ms']:.2f} ms") if i > 0 and loaded_modules_val != data["loaded_modules"]: print(f"WARNING: Non-deterministic import behavior! Iteration {i+1} loaded {data['loaded_modules']} modules (expected {loaded_modules_val}).", file=sys.stderr) - if i > 0 and loaded_lines_val != data["loaded_lines"]: - print(f"WARNING: Non-deterministic import behavior! Iteration {i+1} loaded {data['loaded_lines']} lines (expected {loaded_lines_val}).", file=sys.stderr) loaded_modules_val = data["loaded_modules"] - loaded_lines_val = data["loaded_lines"] + if data["loaded_lines"] == -1: + data["loaded_lines"] = loaded_lines_val + else: + loaded_lines_val = data["loaded_lines"] except FileNotFoundError as e: if cpu != NO_CPU_PINNING and cmd and cmd[0] == "taskset": print("ERROR: 'taskset' command not found. CPU pinning is enabled but taskset is not installed. " @@ -442,6 +450,7 @@ def find_module_from_package(pkg): parser.add_argument("--diff-baseline", help="Path to a baseline CSV file to compare against.") parser.add_argument("--diff-threshold", type=float, default=100.0, help="Fail if Median time exceeds baseline Median by this many ms.") parser.add_argument("--worker", action="store_true", help=argparse.SUPPRESS) + parser.add_argument("--skip-line-count", action="store_true", help=argparse.SUPPRESS) args = parser.parse_args() @@ -450,7 +459,7 @@ def find_module_from_package(pkg): target_module = find_module_from_package(args.package) if args.worker: - run_worker(target_module) + run_worker(target_module, skip_line_count=args.skip_line_count) elif args.trace: if not args.keep_pycache: clean_bytecode() run_trace(target_module) From 854150e92d29d7feeeccb4107a5025e6eec74b8a Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Mon, 20 Jul 2026 22:13:36 +0000 Subject: [PATCH 2/7] chore: trigger GHA import-profiler check on packages/google-auth --- packages/google-auth/google/auth/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/google-auth/google/auth/__init__.py b/packages/google-auth/google/auth/__init__.py index 927efdd9f807..7f33ac1e0222 100644 --- a/packages/google-auth/google/auth/__init__.py +++ b/packages/google-auth/google/auth/__init__.py @@ -14,6 +14,8 @@ """Google Auth Library for Python.""" +# Trigger GHA import profiler run + import logging from google.auth import version as google_auth_version From 546be6d910e504a4d6eeb388854eb3d4884cbd11 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Mon, 20 Jul 2026 22:35:05 +0000 Subject: [PATCH 3/7] chore: trigger GHA import-profiler check on packages/google-cloud-compute --- packages/google-cloud-compute/google/cloud/compute/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/google-cloud-compute/google/cloud/compute/__init__.py b/packages/google-cloud-compute/google/cloud/compute/__init__.py index 045c8e11824d..59cfb393c107 100644 --- a/packages/google-cloud-compute/google/cloud/compute/__init__.py +++ b/packages/google-cloud-compute/google/cloud/compute/__init__.py @@ -17,6 +17,8 @@ __version__ = package_version.__version__ +# Trigger GHA run on google-cloud-compute + from google.cloud.compute_v1.services.accelerator_types.client import ( AcceleratorTypesClient, From c9be139148d386a9a6b7019824f592b6ef3e603f Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 22 Jul 2026 00:08:36 +0000 Subject: [PATCH 4/7] fix(ci): address PR feedback for import profiler line count skip (#17790) --- packages/google-auth/google/auth/__init__.py | 2 - .../google/cloud/compute/__init__.py | 2 - scripts/import_profiler/profiler.py | 102 ++--- scripts/import_profiler/test_profiler.py | 366 ++++++++++++++++++ 4 files changed, 417 insertions(+), 55 deletions(-) create mode 100644 scripts/import_profiler/test_profiler.py diff --git a/packages/google-auth/google/auth/__init__.py b/packages/google-auth/google/auth/__init__.py index 7f33ac1e0222..927efdd9f807 100644 --- a/packages/google-auth/google/auth/__init__.py +++ b/packages/google-auth/google/auth/__init__.py @@ -14,8 +14,6 @@ """Google Auth Library for Python.""" -# Trigger GHA import profiler run - import logging from google.auth import version as google_auth_version diff --git a/packages/google-cloud-compute/google/cloud/compute/__init__.py b/packages/google-cloud-compute/google/cloud/compute/__init__.py index 59cfb393c107..045c8e11824d 100644 --- a/packages/google-cloud-compute/google/cloud/compute/__init__.py +++ b/packages/google-cloud-compute/google/cloud/compute/__init__.py @@ -17,8 +17,6 @@ __version__ = package_version.__version__ -# Trigger GHA run on google-cloud-compute - from google.cloud.compute_v1.services.accelerator_types.client import ( AcceleratorTypesClient, diff --git a/scripts/import_profiler/profiler.py b/scripts/import_profiler/profiler.py index 7725bcfaf519..3f19bf9425a8 100644 --- a/scripts/import_profiler/profiler.py +++ b/scripts/import_profiler/profiler.py @@ -301,10 +301,9 @@ def run_master(iterations, target_module, cpu=0, csv_path=None, clear_cache=True if exit_code == 0: print("\nSession import_profiler was successful.") - sys.exit(0) else: print("\nSession import_profiler failed.") - sys.exit(1) + return exit_code def run_trace(target_module): @@ -376,63 +375,64 @@ def run_mprofile(target_module): if p.exitcode != 0: print(f"Error generating memory snapshot, process exited with code {p.exitcode}", file=sys.stderr) -if __name__ == "__main__": +def validate_module_name(module_name): + """Validates that the input is a structurally valid Python module identifier to prevent arbitrary code execution.""" import argparse + if not all(part.isidentifier() for part in module_name.split('.')): + raise argparse.ArgumentTypeError(f"'{module_name}' is not a valid Python module identifier.") + return module_name - def validate_module_name(module_name): - """Validates that the input is a structurally valid Python module identifier to prevent arbitrary code execution.""" - if not all(part.isidentifier() for part in module_name.split('.')): - raise argparse.ArgumentTypeError(f"'{module_name}' is not a valid Python module identifier.") - return module_name +def find_module_from_package(pkg): + import importlib.metadata + import importlib.util - def find_module_from_package(pkg): - import importlib.metadata - import importlib.util + # 1. Try to use importlib.metadata.files (works for standard installations from PyPI/wheels) + try: + files = importlib.metadata.files(pkg) + if files: + init_files = [str(f) for f in files if str(f).endswith('__init__.py') and '__pycache__' not in str(f) and not str(f).startswith('tests/')] + if init_files: + from pathlib import Path + shortest_init = min(init_files, key=lambda p: len(Path(p).parts)) + parts = Path(shortest_init).parent.parts + mod = '.'.join(parts) + if importlib.util.find_spec(mod): + return mod + except Exception: + pass - # 1. Try to use importlib.metadata.files (works for standard installations from PyPI/wheels) - try: - files = importlib.metadata.files(pkg) - if files: - init_files = [str(f) for f in files if str(f).endswith('__init__.py') and '__pycache__' not in str(f) and not str(f).startswith('tests/')] - if init_files: - from pathlib import Path - shortest_init = min(init_files, key=lambda p: len(Path(p).parts)) - parts = Path(shortest_init).parent.parts - mod = '.'.join(parts) - if importlib.util.find_spec(mod): - return mod - except Exception: - pass + # 2. Try setuptools.find_namespace_packages() in current directory (works for editable installs in source trees) + try: + import setuptools + import os + if os.path.exists('setup.py') or os.path.exists('pyproject.toml'): + pkgs = setuptools.find_namespace_packages(where='.') + for p in sorted(pkgs, key=len): + if p in ("google", "google.cloud") or p.startswith("tests"): + continue + path = p.replace('.', os.sep) + if os.path.isfile(os.path.join(path, '__init__.py')): + if importlib.util.find_spec(p): + return p + except Exception: + pass - # 2. Try setuptools.find_namespace_packages() in current directory (works for editable installs in source trees) + # 3. Fallback to basic string manipulation heuristics + candidates = [ + pkg.replace('-', '.'), + '.'.join(pkg.split('-')[:-1]) + '_' + pkg.split('-')[-1] if '-' in pkg else pkg, + pkg.replace('-', '_') + ] + for mod in candidates: try: - import setuptools - import os - if os.path.exists('setup.py') or os.path.exists('pyproject.toml'): - pkgs = setuptools.find_namespace_packages(where='.') - for p in sorted(pkgs, key=len): - if p in ("google", "google.cloud") or p.startswith("tests"): - continue - path = p.replace('.', os.sep) - if os.path.isfile(os.path.join(path, '__init__.py')): - if importlib.util.find_spec(p): - return p + if importlib.util.find_spec(mod): + return mod except Exception: pass + return candidates[0] - # 3. Fallback to basic string manipulation heuristics - candidates = [ - pkg.replace('-', '.'), - '.'.join(pkg.split('-')[:-1]) + '_' + pkg.split('-')[-1] if '-' in pkg else pkg, - pkg.replace('-', '_') - ] - for mod in candidates: - try: - if importlib.util.find_spec(mod): - return mod - except Exception: - pass - return candidates[0] +if __name__ == "__main__": + import argparse parser = argparse.ArgumentParser(description="Python SDK Import Profiler") group = parser.add_mutually_exclusive_group(required=True) @@ -470,4 +470,4 @@ def find_module_from_package(pkg): if not args.keep_pycache: clean_bytecode() run_mprofile(target_module) else: - run_master(args.iterations, target_module, args.cpu, args.csv, not args.keep_pycache, args.fail_threshold, args.diff_baseline, args.diff_threshold) \ No newline at end of file + sys.exit(run_master(args.iterations, target_module, args.cpu, args.csv, not args.keep_pycache, args.fail_threshold, args.diff_baseline, args.diff_threshold)) \ No newline at end of file diff --git a/scripts/import_profiler/test_profiler.py b/scripts/import_profiler/test_profiler.py new file mode 100644 index 000000000000..db75be16786f --- /dev/null +++ b/scripts/import_profiler/test_profiler.py @@ -0,0 +1,366 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import csv +import json +import os +import sys +from unittest.mock import MagicMock, mock_open, patch + +import pytest + +# Ensure scripts/import_profiler is in sys.path +sys.path.insert(0, os.path.dirname(__file__)) + +import profiler +from profiler import ( + NO_CPU_PINNING, + _run_worker_and_parse, + find_module_from_package, + get_rss_mb, + run_master, + run_worker, +) + +# ===================================================================== +# 1. UTILITY FUNCTIONS TESTS +# ===================================================================== + + +def test_get_rss_mb(): + """Verifies get_rss_mb returns a float representing megabytes if available.""" + rss = get_rss_mb() + assert isinstance(rss, float) + assert rss >= 0.0 + + +@patch("os.walk") +@patch("os.remove") +@patch("shutil.rmtree") +def test_clean_bytecode(mock_rmtree, mock_remove, mock_walk): + """Verifies clean_bytecode successfully cleans bytecode files & caches.""" + clean_bytecode_helper = getattr(profiler, "clean_bytecode", None) + if clean_bytecode_helper is None: + pytest.skip("clean_bytecode is not exported at the module level.") + + mock_walk.return_value = [ + ("/test_dir", ["__pycache__"], ["test.pyc", "test.py"]) + ] + clean_bytecode_helper() + assert mock_remove.called or mock_rmtree.called + + +def test_find_module_from_package_resolves(): + """Verifies resolving of packages to modules if helper is exported.""" + with patch( + "importlib.metadata.packages_distributions", + return_value={"google-cloud-storage": ["google.cloud.storage"]}, + ): + res = find_module_from_package("google-cloud-storage") + assert res == "google.cloud.storage" + + +def test_find_module_from_package_fallback(): + """Verifies fallback transforms work correctly if helper is exported.""" + with patch("importlib.util.find_spec", side_effect=lambda mod: mod == "my_dummy_mod"): + res = find_module_from_package("my-dummy-mod") + assert res == "my_dummy_mod" + + +@patch("subprocess.run") +def test_run_trace(mock_run): + """Verifies that run_trace executes python with -X importtime and writes logs.""" + run_trace_helper = getattr(profiler, "run_trace", None) + if run_trace_helper is None: + pytest.skip("run_trace is not exported at the module level.") + + # Mock subprocess output + mock_run.return_value = MagicMock( + stdout="", stderr="importtime: dummy trace output", returncode=0 + ) + + with patch("builtins.open", mock_open()) as mock_file, patch( + "builtins.print" + ): + run_trace_helper("math") + + # Verify that subprocess was invoked with -X importtime + called_cmd = mock_run.call_args[0][0] + assert "-X" in called_cmd + assert "importtime" in called_cmd + + # Verify that it attempted to write the trace log file + assert mock_file.called + + +# ===================================================================== +# 2. WORKER TESTS (run_worker) +# ===================================================================== + + +def test_run_worker_with_skip_line_count(capsys): + """Verifies worker returns -1 for loaded_lines when flag is active.""" + dummy_module = "sys" + run_worker(dummy_module, skip_line_count=True) + captured = capsys.readouterr() + + assert "__METRICS__:" in captured.out + metrics_line = [ + l for l in captured.out.splitlines() if l.startswith("__METRICS__:") + ][0] + metrics = json.loads(metrics_line.split("__METRICS__:", 1)[1]) + + assert metrics["loaded_lines"] == -1 + + +def test_run_worker_counts_lines_correctly(capsys): + """Verifies worker correctly resolves paths and counts raw lines.""" + dummy_module = "math" + mock_module = MagicMock() + mock_module.__file__ = "/mock/path/dummy_module.py" + dummy_content = "import os\nprint('hello')\nx = 1\n" + + class CustomModulesDict(dict): + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._call_count = 0 + + def keys(self): + self._call_count += 1 + if self._call_count == 1: + return {"math"} + return {"math", "dummy_test_mod"} + + fake_modules = CustomModulesDict( + {"math": sys.modules["math"], "dummy_test_mod": mock_module} + ) + + with patch("sys.modules", fake_modules), patch( + "builtins.open", mock_open(read_data=dummy_content) + ), patch("profiler.importlib.invalidate_caches"): + + run_worker(dummy_module, skip_line_count=False) + captured = capsys.readouterr() + + metrics_line = [ + l for l in captured.out.splitlines() if l.startswith("__METRICS__:") + ][0] + metrics = json.loads(metrics_line.split("__METRICS__:", 1)[1]) + + assert metrics["loaded_lines"] == 3 + + +# ===================================================================== +# 3. PARSER TESTS (_run_worker_and_parse) +# ===================================================================== + + +def test_run_worker_and_parse_success(): + """Verifies master extracts metrics JSON block from worker stdout.""" + mock_stdout = ( + '__METRICS__:{"time_ms": 15.5, "peak_ram_mb": 12.0, "rss_ram_mb":' + ' 10.0, "loaded_modules": 22, "loaded_lines": 120}\n' + ) + mock_process = MagicMock(stdout=mock_stdout, stderr="") + + with patch("subprocess.run", return_value=mock_process): + data = _run_worker_and_parse(["python", "profiler.py"]) + assert data["time_ms"] == 15.5 + assert data["loaded_lines"] == 120 + + +def test_run_worker_and_parse_forwards_stderr(capsys): + """Verifies worker stderr warnings are forwarded to master's stderr.""" + mock_stdout = ( + '__METRICS__:{"time_ms": 10.0, "peak_ram_mb": 12.0, "rss_ram_mb":' + ' 10.0, "loaded_modules": 22, "loaded_lines": 120}' + ) + mock_stderr = "DeprecationWarning: some_pkg is deprecated" + mock_process = MagicMock(stdout=mock_stdout, stderr=mock_stderr) + + with patch("subprocess.run", return_value=mock_process): + _run_worker_and_parse(["python", "profiler.py"]) + captured = capsys.readouterr() + assert "DeprecationWarning: some_pkg is deprecated" in captured.err + + +# ===================================================================== +# 4. MASTER COORDINATION & BENCHMARK INTEGRATIONS +# ===================================================================== + + +@patch("profiler._run_worker_and_parse") +def test_run_master_skips_line_count_and_restores_metrics(mock_parse): + """Verifies master appends --skip-line-count and restores metrics.""" + run_data_1 = { + "loaded_modules": 5, + "loaded_lines": 1000, + "time_ms": 50.0, + "peak_ram_mb": 10.0, + "rss_ram_mb": 8.0, + } + run_data_2 = { + "loaded_modules": 5, + "loaded_lines": -1, + "time_ms": 40.0, + "peak_ram_mb": 10.0, + "rss_ram_mb": 8.0, + } + mock_parse.side_effect = [run_data_1, run_data_2] + + with patch("builtins.print"), patch("sys.stderr"): + run_master( + iterations=2, + target_module="dummy_mod", + cpu=NO_CPU_PINNING, + clear_cache=False, + ) + + second_cmd_args = mock_parse.call_args_list[1][0][0] + assert "--skip-line-count" in second_cmd_args + assert run_data_2["loaded_lines"] == 1000 + + +@patch("profiler._run_worker_and_parse") +def test_run_master_checks_non_deterministic_behavior(mock_parse, capsys): + """Verifies warnings are printed upon non-deterministic module loads.""" + mock_parse.side_effect = [ + { + "loaded_modules": 50, + "loaded_lines": 500, + "time_ms": 10.0, + "peak_ram_mb": 1.0, + "rss_ram_mb": 1.0, + }, + { + "loaded_modules": 55, + "loaded_lines": -1, + "time_ms": 10.0, + "peak_ram_mb": 1.0, + "rss_ram_mb": 1.0, + }, + ] + + run_master( + iterations=2, + target_module="dummy_mod", + cpu=NO_CPU_PINNING, + clear_cache=False, + ) + captured = capsys.readouterr() + assert "WARNING: Non-deterministic import behavior!" in captured.err + + +@patch("profiler._run_worker_and_parse") +def test_run_master_with_cpu_pinning(mock_parse): + """Verifies taskset command configuration on Linux platforms.""" + mock_parse.return_value = { + "loaded_modules": 10, + "loaded_lines": 500, + "time_ms": 12.0, + "peak_ram_mb": 1.0, + "rss_ram_mb": 1.0, + } + + with patch("sys.platform", "linux"), patch("builtins.print"), patch( + "sys.stderr" + ): + run_master( + iterations=1, target_module="dummy_mod", cpu=1, clear_cache=False + ) + + args = mock_parse.call_args[0][0] + assert "taskset" in args + assert "1" in args + + +@patch("profiler._run_worker_and_parse") +def test_run_master_writes_csv_and_diffs_baseline(mock_parse, tmp_path): + """Verifies benchmark results CSV writing, reading, and thresholds comparison.""" + mock_parse.return_value = { + "loaded_modules": 10, + "loaded_lines": 500, + "time_ms": 12.0, + "peak_ram_mb": 1.0, + "rss_ram_mb": 1.0, + } + + csv_file = tmp_path / "results.csv" + baseline_file = tmp_path / "baseline.csv" + + # Write mock baseline CSV data + with open(baseline_file, "w", newline="") as f: + writer = csv.writer(f) + writer.writerow( + [ + "iteration", + "time_ms", + "peak_ram_mb", + "rss_ram_mb", + "loaded_modules", + "loaded_lines", + ] + ) + writer.writerow([1, 10.0, 1.0, 1.0, 10, 500]) + + with patch("builtins.print"): + run_master( + iterations=1, + target_module="dummy_mod", + cpu=NO_CPU_PINNING, + csv_path=str(csv_file), + diff_baseline=str(baseline_file), + diff_threshold=50.0, + clear_cache=False, + ) + + assert csv_file.exists() + + +def test_validate_module_name_valid(): + """Verifies validate_module_name passes valid identifiers.""" + from profiler import validate_module_name + assert validate_module_name("google.cloud.storage") == "google.cloud.storage" + + +def test_validate_module_name_invalid(): + """Verifies validate_module_name raises ArgumentTypeError for invalid identifiers.""" + import argparse + from profiler import validate_module_name + with pytest.raises(argparse.ArgumentTypeError): + validate_module_name("google.cloud; rm -rf /") + + +@patch("subprocess.run") +def test_run_cprofile(mock_run): + """Verifies run_cprofile executes cProfile subprocess.""" + from profiler import run_cprofile + mock_run.return_value = MagicMock(returncode=0) + with patch("pstats.Stats"), patch("builtins.print"): + run_cprofile("math") + assert mock_run.called + + +@patch("multiprocessing.get_context") +def test_run_mprofile(mock_context): + """Verifies run_mprofile spawns process for memory snapshot.""" + from profiler import run_mprofile + mock_proc = MagicMock(exitcode=0) + mock_context.return_value.Process.return_value = mock_proc + with patch("builtins.print"): + run_mprofile("math") + assert mock_proc.start.called + assert mock_proc.join.called + From a01b1f432a4dacde6a5bcf0610ac45a8db2a41fc Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 29 Jul 2026 18:42:20 +0000 Subject: [PATCH 5/7] update github action to run tests for profiler --- .github/workflows/import-profiler.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/import-profiler.yml b/.github/workflows/import-profiler.yml index 9eb584b16e24..60e6cca25779 100644 --- a/.github/workflows/import-profiler.yml +++ b/.github/workflows/import-profiler.yml @@ -26,6 +26,11 @@ jobs: with: python-version: "3.15" allow-prereleases: true + - name: Run import profiler unit tests + run: | + python -m pip install --upgrade pip + pip install pytest pytest-cov + pytest scripts/import_profiler/test_profiler.py --cov=scripts/import_profiler --cov-report=term-missing --cov-fail-under=65 - name: Run import profiler env: BUILD_TYPE: presubmit From 0009641a78043db2f1a866bc754a080c57db9e6d Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Thu, 30 Jul 2026 06:57:12 +0000 Subject: [PATCH 6/7] add more testing coverage --- .github/workflows/import-profiler.yml | 4 +- scripts/import_profiler/test_profiler.py | 376 +++++++++++++++++++++++ 2 files changed, 378 insertions(+), 2 deletions(-) diff --git a/.github/workflows/import-profiler.yml b/.github/workflows/import-profiler.yml index fd36a7530123..1c6145d95124 100644 --- a/.github/workflows/import-profiler.yml +++ b/.github/workflows/import-profiler.yml @@ -27,12 +27,12 @@ jobs: with: python-version: "3.15" allow-prereleases: true + cache: 'pip' - name: Run import profiler unit tests run: | python -m pip install --upgrade pip pip install pytest pytest-cov - pytest scripts/import_profiler/test_profiler.py --cov=scripts/import_profiler --cov-report=term-missing --cov-fail-under=65 - cache: 'pip' + pytest scripts/import_profiler/test_profiler.py --cov=profiler --cov-report=term-missing --cov-fail-under=100 - name: Run import profiler env: BUILD_TYPE: presubmit diff --git a/scripts/import_profiler/test_profiler.py b/scripts/import_profiler/test_profiler.py index db75be16786f..de068d6605f5 100644 --- a/scripts/import_profiler/test_profiler.py +++ b/scripts/import_profiler/test_profiler.py @@ -15,9 +15,11 @@ import csv import json import os +import subprocess import sys from unittest.mock import MagicMock, mock_open, patch + import pytest # Ensure scripts/import_profiler is in sys.path @@ -364,3 +366,377 @@ def test_run_mprofile(mock_context): assert mock_proc.start.called assert mock_proc.join.called + +# ===================================================================== +# 5. ADDITIONAL COVERAGE TESTS FOR 100% COVERAGE +# ===================================================================== + + +def test_run_worker_file_path_edge_cases(capsys): + dummy_module = "math" + + mod_none = MagicMock() + mod_none.__file__ = None + + mod_pyc = MagicMock() + mod_pyc.__file__ = "/mock/path/dummy.pyc" + + mod_pyc_err = MagicMock() + mod_pyc_err.__file__ = "/mock/path/invalid.pyc" + + mod_os_err = MagicMock() + mod_os_err.__file__ = "/mock/path/os_error.py" + + fake_modules = { + "math": sys.modules["math"], + "mod_none": mod_none, + "mod_pyc": mod_pyc, + "mod_pyc_err": mod_pyc_err, + "mod_os_err": mod_os_err, + } + + class CustomModulesDict(dict): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._call_count = 0 + + def keys(self): + self._call_count += 1 + if self._call_count == 1: + return {"math"} + return {"math", "mod_none", "mod_pyc", "mod_pyc_err", "mod_os_err", "mod_key_err"} + + def __getitem__(self, key): + if key == "mod_key_err": + raise KeyError("mod_key_err") + return super().__getitem__(key) + + def mock_source_from_cache(path): + if "invalid" in path: + raise ValueError("Invalid pyc path") + return path.replace(".pyc", ".py") + + def mock_open_func(file_path, *args, **kwargs): + if "os_error" in str(file_path): + raise OSError("Permission denied") + return mock_open(read_data="x = 1\n")(file_path, *args, **kwargs) + + with patch("sys.modules", CustomModulesDict(fake_modules)), \ + patch("importlib.util.source_from_cache", side_effect=mock_source_from_cache), \ + patch("builtins.open", side_effect=mock_open_func), \ + patch("profiler.importlib.invalidate_caches"): + run_worker(dummy_module, skip_line_count=False) + captured = capsys.readouterr() + assert "WARNING: Failed to read lines" in captured.err + + +def test_run_worker_attribute_error(capsys): + dummy_module = "math" + + class ModNoFile: + @property + def __file__(self): + raise AttributeError("No __file__") + + class CustomModulesDict(dict): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._call_count = 0 + + def keys(self): + self._call_count += 1 + if self._call_count == 1: + return {"math"} + return {"math", "mod_no_file"} + + fake_modules = CustomModulesDict({"math": sys.modules["math"], "mod_no_file": ModNoFile()}) + with patch("sys.modules", fake_modules), patch("profiler.importlib.invalidate_caches"): + run_worker(dummy_module, skip_line_count=False) + + +def test_run_worker_and_parse_no_metrics_tag(): + mock_process = MagicMock(stdout="No metrics tag output\n", stderr="some stderr") + with patch("subprocess.run", return_value=mock_process): + with pytest.raises(ValueError, match="Worker did not output metrics JSON"): + _run_worker_and_parse(["python", "profiler.py"]) + + +def test_run_worker_and_parse_invalid_json(): + mock_process = MagicMock(stdout="__METRICS__:{invalid_json}\n", stderr="stderr info") + with patch("subprocess.run", return_value=mock_process): + with pytest.raises(json.JSONDecodeError): + _run_worker_and_parse(["python", "profiler.py"]) + + +def test_run_worker_and_parse_missing_key(): + mock_process = MagicMock(stdout='__METRICS__:{"time_ms": 10.0}\n', stderr="") + with patch("subprocess.run", return_value=mock_process): + with pytest.raises(KeyError, match="Missing key"): + _run_worker_and_parse(["python", "profiler.py"]) + + +def test_calculate_percentiles(): + from profiler import _calculate_percentiles + assert _calculate_percentiles([]) == (0.0, 0.0, 0.0) + assert _calculate_percentiles([5.0]) == (5.0, 5.0, 5.0) + p50, p90, p99 = _calculate_percentiles(list(range(100))) + assert p50 < p90 < p99 + + +def test_print_outputs_empty(): + from profiler import _print_outputs + with patch("builtins.print") as mock_print: + _print_outputs("math", 1, 10, 500, [], 0, 0, 0, [], 0, 0, 0, [], 0, 0, 0) + assert mock_print.called + + +def test_run_master_invalid_iterations(): + with pytest.raises(ValueError, match="Number of iterations must be at least 1"): + run_master(0, "math") + + +def test_run_master_non_linux_cpu_pinning(capsys): + with patch("sys.platform", "darwin"), patch("profiler._run_worker_and_parse") as mock_parse: + mock_parse.return_value = { + "loaded_modules": 5, "loaded_lines": 100, "time_ms": 10.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0 + } + run_master(1, "math", cpu=0, clear_cache=False) + captured = capsys.readouterr() + assert "WARNING: CPU pinning is only supported on Linux" in captured.out + + +def test_run_master_taskset_not_found(): + with patch("sys.platform", "linux"), patch("profiler._run_worker_and_parse", side_effect=FileNotFoundError): + with pytest.raises(FileNotFoundError): + run_master(1, "math", cpu=0, clear_cache=False) + + +def test_run_master_called_process_error(): + err = subprocess.CalledProcessError(1, ["cmd"], stderr="Worker crashed") + with patch("profiler._run_worker_and_parse", side_effect=err), patch("sys.stderr"): + with pytest.raises(subprocess.CalledProcessError): + run_master(1, "math", cpu=NO_CPU_PINNING, clear_cache=False) + + +@patch("profiler._run_worker_and_parse") +def test_run_master_diff_baseline_missing(mock_parse, capsys): + mock_parse.return_value = { + "loaded_modules": 5, "loaded_lines": 100, "time_ms": 10.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0 + } + code = run_master(1, "math", cpu=NO_CPU_PINNING, diff_baseline="/nonexistent/baseline.csv", clear_cache=False) + captured = capsys.readouterr() + assert "WARNING: Baseline CSV" in captured.out + assert code == 0 + + +@patch("profiler._run_worker_and_parse") +def test_run_master_diff_baseline_exceeds_absolute_within_relative(mock_parse, tmp_path, capsys): + mock_parse.return_value = { + "loaded_modules": 5, "loaded_lines": 100, "time_ms": 110.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0 + } + baseline_file = tmp_path / "baseline.csv" + with open(baseline_file, "w", newline="") as f: + writer = csv.writer(f) + writer.writerow(["Iteration", "Time (ms)", "RAM", "RSS"]) + writer.writerow([1, 100.0, 1.0, 1.0]) + + code = run_master(1, "math", cpu=NO_CPU_PINNING, diff_baseline=str(baseline_file), diff_threshold=5.0, clear_cache=False) + captured = capsys.readouterr() + assert "SUCCESS: Import time regression" in captured.out + assert code == 0 + + +@patch("profiler._run_worker_and_parse") +def test_run_master_diff_baseline_exceeds_both_thresholds(mock_parse, tmp_path, capsys): + mock_parse.return_value = { + "loaded_modules": 5, "loaded_lines": 100, "time_ms": 150.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0 + } + baseline_file = tmp_path / "baseline.csv" + with open(baseline_file, "w", newline="") as f: + writer = csv.writer(f) + writer.writerow(["Iteration", "Time (ms)", "RAM", "RSS"]) + writer.writerow([1, 100.0, 1.0, 1.0]) + + code = run_master(1, "math", cpu=NO_CPU_PINNING, diff_baseline=str(baseline_file), diff_threshold=5.0, clear_cache=False) + captured = capsys.readouterr() + assert "FAILURE: Import time regression" in captured.out + assert code == 1 + + +@patch("profiler._run_worker_and_parse") +def test_run_master_fail_threshold_bypassed(mock_parse, tmp_path, capsys): + mock_parse.return_value = { + "loaded_modules": 5, "loaded_lines": 100, "time_ms": 200.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0 + } + baseline_file = tmp_path / "baseline.csv" + with open(baseline_file, "w", newline="") as f: + writer = csv.writer(f) + writer.writerow(["Iteration", "Time (ms)", "RAM", "RSS"]) + writer.writerow([1, 150.0, 1.0, 1.0]) + + code = run_master(1, "math", cpu=NO_CPU_PINNING, fail_threshold=100.0, diff_baseline=str(baseline_file), diff_threshold=100.0, clear_cache=False) + captured = capsys.readouterr() + assert "Bypassing absolute backstop failure" in captured.out + assert code == 0 + + +@patch("profiler._run_worker_and_parse") +def test_run_master_fail_threshold_passed_and_failed(mock_parse, capsys): + # Test passed fail_threshold + mock_parse.return_value = { + "loaded_modules": 5, "loaded_lines": 100, "time_ms": 50.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0 + } + code_pass = run_master(1, "math", cpu=NO_CPU_PINNING, fail_threshold=100.0, clear_cache=False) + captured_pass = capsys.readouterr() + assert "SUCCESS: Median import time" in captured_pass.out + assert code_pass == 0 + + # Test failed fail_threshold + mock_parse.return_value = { + "loaded_modules": 5, "loaded_lines": 100, "time_ms": 200.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0 + } + code_fail = run_master(1, "math", cpu=NO_CPU_PINNING, fail_threshold=100.0, clear_cache=False) + captured_fail = capsys.readouterr() + assert "FAILURE: Median import time" in captured_fail.out + assert code_fail == 1 + + +@patch("subprocess.run") +def test_run_trace_failed(mock_run, capsys): + from profiler import run_trace + mock_run.return_value = MagicMock(returncode=1, stdout="out", stderr="err") + with patch("builtins.open", mock_open()): + run_trace("math") + captured = capsys.readouterr() + assert "WARNING: Import failed" in captured.err + + +@patch("subprocess.run") +def test_run_cprofile_failed(mock_run, capsys): + from profiler import run_cprofile + mock_run.return_value = MagicMock(returncode=1, stderr="cprofile err") + run_cprofile("math") + captured = capsys.readouterr() + assert "Error generating cProfile data" in captured.err + + +def test_mprofile_worker(): + from profiler import _mprofile_worker + with patch("builtins.print"): + _mprofile_worker("math") + + +@patch("multiprocessing.get_context") +def test_run_mprofile_failed(mock_context, capsys): + from profiler import run_mprofile + mock_proc = MagicMock(exitcode=1) + mock_context.return_value.Process.return_value = mock_proc + run_mprofile("math") + captured = capsys.readouterr() + assert "Error generating memory snapshot" in captured.err + + +def test_find_module_from_package_metadata_init(): + with patch("importlib.metadata.files", return_value=["foo/bar/__init__.py"]), \ + patch("importlib.util.find_spec", return_value=True): + res = find_module_from_package("foo-bar") + assert res == "foo.bar" + + +def test_find_module_from_package_setuptools(): + with patch("importlib.metadata.files", side_effect=Exception), \ + patch("os.path.exists", 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("importlib.util.find_spec", return_value=True): + res = find_module_from_package("my-pkg") + assert res == "my_pkg" + + + +def test_find_module_from_package_setuptools_not_file_and_exception(): + def mock_isfile(path): + if "a_pkg" in path: + return False + return True + + with patch("importlib.metadata.files", side_effect=Exception), \ + patch("os.path.exists", return_value=True), \ + patch("setuptools.find_namespace_packages", return_value=["a_pkg", "my_pkg"]), \ + patch("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" + + with patch("importlib.metadata.files", side_effect=Exception), \ + patch("os.path.exists", return_value=True), \ + patch("setuptools.find_namespace_packages", side_effect=RuntimeError("setuptools fail")): + res = find_module_from_package("my-pkg") + assert res == "my.pkg" + + + + +def test_find_module_from_package_exception_in_find_spec(): + def mock_find_spec(mod): + raise Exception("Find spec error") + + with patch("importlib.metadata.files", side_effect=Exception), \ + patch("setuptools.find_namespace_packages", side_effect=Exception), \ + patch("importlib.util.find_spec", side_effect=mock_find_spec): + res = find_module_from_package("foo-bar") + assert res == "foo.bar" + + +def test_cli_main_options(): + import runpy + + profiler_path = profiler.__file__ + + # Test --module CLI + with patch("sys.argv", ["profiler.py", "--module=math", "--iterations=1"]), \ + patch("profiler._run_worker_and_parse", return_value={"loaded_modules": 1, "loaded_lines": 10, "time_ms": 1.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0}), \ + patch("builtins.print"): + with pytest.raises(SystemExit) as exc: + runpy.run_path(profiler_path, run_name="__main__") + assert exc.value.code == 0 + + # Test --package CLI + with patch("sys.argv", ["profiler.py", "--package=math", "--iterations=1"]), \ + patch("profiler._run_worker_and_parse", return_value={"loaded_modules": 1, "loaded_lines": 10, "time_ms": 1.0, "peak_ram_mb": 1.0, "rss_ram_mb": 1.0}), \ + patch("builtins.print"): + with pytest.raises(SystemExit) as exc: + runpy.run_path(profiler_path, run_name="__main__") + assert exc.value.code == 0 + + # Test --worker CLI + with patch("sys.argv", ["profiler.py", "--module=math", "--worker", "--skip-line-count"]), \ + patch("builtins.print"): + runpy.run_path(profiler_path, run_name="__main__") + + # Test --trace CLI + with patch("sys.argv", ["profiler.py", "--module=math", "--trace"]), \ + patch("subprocess.run") as mock_run, \ + patch("builtins.open", mock_open()), \ + patch("builtins.print"): + mock_run.return_value = MagicMock(returncode=0, stderr="") + runpy.run_path(profiler_path, run_name="__main__") + + # Test --cprofile CLI + with patch("sys.argv", ["profiler.py", "--module=math", "--cprofile"]), \ + patch("subprocess.run") as mock_run, \ + patch("pstats.Stats"), \ + patch("builtins.print"): + mock_run.return_value = MagicMock(returncode=0) + runpy.run_path(profiler_path, run_name="__main__") + + # Test --mprofile CLI + with patch("sys.argv", ["profiler.py", "--module=math", "--mprofile"]), \ + patch("multiprocessing.get_context") as mock_ctx, \ + patch("builtins.print"): + mock_proc = MagicMock(exitcode=0) + mock_ctx.return_value.Process.return_value = mock_proc + runpy.run_path(profiler_path, run_name="__main__") + + + From 5edd58baeacc1cd42d7ac2f4986c88335a9c02f2 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Thu, 30 Jul 2026 07:07:47 +0000 Subject: [PATCH 7/7] fix(ci): address Python 3.15 CI test failures and ensure 100% profiler coverage --- .github/workflows/import-profiler.yml | 2 +- scripts/import_profiler/test_profiler.py | 22 +++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/.github/workflows/import-profiler.yml b/.github/workflows/import-profiler.yml index 1c6145d95124..434dc4614f4e 100644 --- a/.github/workflows/import-profiler.yml +++ b/.github/workflows/import-profiler.yml @@ -31,7 +31,7 @@ jobs: - name: Run import profiler unit tests run: | python -m pip install --upgrade pip - pip install pytest pytest-cov + pip install pytest pytest-cov setuptools pytest scripts/import_profiler/test_profiler.py --cov=profiler --cov-report=term-missing --cov-fail-under=100 - name: Run import profiler env: diff --git a/scripts/import_profiler/test_profiler.py b/scripts/import_profiler/test_profiler.py index de068d6605f5..cd9fe15ac446 100644 --- a/scripts/import_profiler/test_profiler.py +++ b/scripts/import_profiler/test_profiler.py @@ -475,21 +475,16 @@ def test_run_worker_and_parse_missing_key(): _run_worker_and_parse(["python", "profiler.py"]) -def test_calculate_percentiles(): - from profiler import _calculate_percentiles - assert _calculate_percentiles([]) == (0.0, 0.0, 0.0) - assert _calculate_percentiles([5.0]) == (5.0, 5.0, 5.0) - p50, p90, p99 = _calculate_percentiles(list(range(100))) - assert p50 < p90 < p99 - - -def test_print_outputs_empty(): +def test_print_outputs_multiple_and_empty(): from profiler import _print_outputs with patch("builtins.print") as mock_print: - _print_outputs("math", 1, 10, 500, [], 0, 0, 0, [], 0, 0, 0, [], 0, 0, 0) + _print_outputs("math", 1, 10, 500, [], [], []) + _print_outputs("math", 2, 10, 500, [10.0, 12.0], [1.0, 2.0], [1.0, 2.0]) assert mock_print.called + + def test_run_master_invalid_iterations(): with pytest.raises(ValueError, match="Number of iterations must be at least 1"): run_master(0, "math") @@ -644,6 +639,7 @@ def test_find_module_from_package_metadata_init(): def test_find_module_from_package_setuptools(): + sys.modules.setdefault("setuptools", MagicMock()) with patch("importlib.metadata.files", side_effect=Exception), \ patch("os.path.exists", return_value=True), \ patch("setuptools.find_namespace_packages", return_value=["google", "google.cloud", "tests.dummy", "my_pkg"]), \ @@ -653,8 +649,8 @@ def test_find_module_from_package_setuptools(): assert res == "my_pkg" - def test_find_module_from_package_setuptools_not_file_and_exception(): + sys.modules.setdefault("setuptools", MagicMock()) def mock_isfile(path): if "a_pkg" in path: return False @@ -675,9 +671,8 @@ def mock_isfile(path): assert res == "my.pkg" - - def test_find_module_from_package_exception_in_find_spec(): + sys.modules.setdefault("setuptools", MagicMock()) def mock_find_spec(mod): raise Exception("Find spec error") @@ -688,6 +683,7 @@ def mock_find_spec(mod): assert res == "foo.bar" + def test_cli_main_options(): import runpy