diff --git a/Lib/profiling/sampling/binary_collector.py b/Lib/profiling/sampling/binary_collector.py index 64afe632fae175..afbbc829269067 100644 --- a/Lib/profiling/sampling/binary_collector.py +++ b/Lib/profiling/sampling/binary_collector.py @@ -94,6 +94,7 @@ def export(self, filename=None): filename: Ignored (binary files are written incrementally) """ self._writer.finalize() + return True @property def total_samples(self): diff --git a/Lib/profiling/sampling/cli.py b/Lib/profiling/sampling/cli.py index f4b31aad45b922..71e946d76111f1 100644 --- a/Lib/profiling/sampling/cli.py +++ b/Lib/profiling/sampling/cli.py @@ -660,10 +660,14 @@ def _handle_output(collector, args, pid, mode): filename = os.path.join(args.outfile, _generate_output_filename(args.format, pid)) else: filename = args.outfile or _generate_output_filename(args.format, pid) - collector.export(filename) + export_ok = collector.export(filename) # Auto-open browser for HTML output if --browser flag is set - if args.format in ('flamegraph', 'heatmap') and getattr(args, 'browser', False): + if ( + export_ok + and args.format in ('flamegraph', 'heatmap') + and getattr(args, 'browser', False) + ): _open_in_browser(filename) @@ -1203,10 +1207,14 @@ def progress_callback(current, total): collector.print_stats(sort_mode, limit, not args.no_summary, PROFILING_MODE_WALL) else: filename = args.outfile or _generate_output_filename(args.format, os.getpid()) - collector.export(filename) + export_ok = collector.export(filename) # Auto-open browser for HTML output if --browser flag is set - if args.format in ('flamegraph', 'heatmap') and getattr(args, 'browser', False): + if ( + export_ok + and args.format in ('flamegraph', 'heatmap') + and getattr(args, 'browser', False) + ): _open_in_browser(filename) print(f"Replayed {count} samples") diff --git a/Lib/profiling/sampling/collector.py b/Lib/profiling/sampling/collector.py index 7dc095c6c279bd..9e8436a90161e6 100644 --- a/Lib/profiling/sampling/collector.py +++ b/Lib/profiling/sampling/collector.py @@ -81,7 +81,11 @@ def collect_failed_sample(self): @abstractmethod def export(self, filename): - """Export collected data to a file.""" + """Export collected data. + + Returns: + bool: True if output was generated, False if there was no data to export. + """ @staticmethod def _filter_internal_frames(frames): diff --git a/Lib/profiling/sampling/gecko_collector.py b/Lib/profiling/sampling/gecko_collector.py index 28ef9b69bf7968..65928e6e19d4a8 100644 --- a/Lib/profiling/sampling/gecko_collector.py +++ b/Lib/profiling/sampling/gecko_collector.py @@ -701,6 +701,7 @@ def spin(): print( f"Open in Firefox Profiler: https://profiler.firefox.com/" ) + return True def _build_marker_schema(self): """Build marker schema definitions for Firefox Profiler.""" diff --git a/Lib/profiling/sampling/heatmap_collector.py b/Lib/profiling/sampling/heatmap_collector.py index b6d9ff79e8ceec..2be6e0348f41e7 100644 --- a/Lib/profiling/sampling/heatmap_collector.py +++ b/Lib/profiling/sampling/heatmap_collector.py @@ -714,7 +714,7 @@ def export(self, output_path): """ if not self.file_samples: print("Warning: No heatmap data to export") - return + return False try: output_dir = self._prepare_output_directory(output_path) @@ -728,6 +728,7 @@ def export(self, output_path): self._generate_index_html(output_dir / 'index.html', file_stats) self._print_export_summary(output_dir, file_stats) + return True except Exception as e: print(f"Error: Failed to export heatmap: {e}") diff --git a/Lib/profiling/sampling/pstats_collector.py b/Lib/profiling/sampling/pstats_collector.py index 6be1d698ffaa9a..a32c36d20b8ee8 100644 --- a/Lib/profiling/sampling/pstats_collector.py +++ b/Lib/profiling/sampling/pstats_collector.py @@ -61,6 +61,7 @@ def collect(self, stack_frames, timestamps_us=None): def export(self, filename): self.create_stats() self._dump_stats(filename) + return True def _dump_stats(self, file): stats_with_marker = dict(self.stats) diff --git a/Lib/profiling/sampling/stack_collector.py b/Lib/profiling/sampling/stack_collector.py index 5a3497a5408414..8feab8cc7c208b 100644 --- a/Lib/profiling/sampling/stack_collector.py +++ b/Lib/profiling/sampling/stack_collector.py @@ -61,6 +61,7 @@ def export(self, filename): for stack, count in lines: f.write(f"{stack} {count}\n") print(f"Collapsed stack output written to {filename}") + return True class FlamegraphCollector(StackTraceCollector): @@ -157,7 +158,7 @@ def export(self, filename): print( "Warning: No functions found in profiling data. Check if sampling captured any data." ) - return + return False html_content = self._create_flamegraph_html(flamegraph_data) @@ -165,6 +166,7 @@ def export(self, filename): f.write(html_content) print(f"Flamegraph saved to: {filename}") + return True @staticmethod @functools.lru_cache(maxsize=None) diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_cli.py b/Lib/test/test_profiling/test_sampling_profiler/test_cli.py index 0d92bd1796e9af..e2e1c1d6769d55 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_cli.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_cli.py @@ -4,6 +4,7 @@ import subprocess import sys import unittest +from types import SimpleNamespace from unittest import mock try: @@ -15,7 +16,7 @@ from test.support import is_emscripten, requires_remote_subprocess_debugging -from profiling.sampling.cli import main +from profiling.sampling.cli import _handle_output, main from profiling.sampling.errors import SamplingScriptNotFoundError, SamplingModuleNotFoundError, SamplingUnknownProcessError class TestSampleProfilerCLI(unittest.TestCase): @@ -700,6 +701,24 @@ def test_async_aware_incompatible_with_all_threads(self): self.assertIn("--all-threads", error_msg) self.assertIn("incompatible with --async-aware", error_msg) + def test_handle_output_browser_not_opened_when_export_fails(self): + collector = mock.MagicMock() + collector.export.return_value = False + args = SimpleNamespace( + format="flamegraph", + outfile="profile.html", + browser=True, + ) + + with ( + mock.patch("profiling.sampling.cli.os.path.isdir", return_value=False), + mock.patch("profiling.sampling.cli._open_in_browser") as mock_open, + ): + _handle_output(collector, args, pid=12345, mode=0) + + collector.export.assert_called_once_with("profile.html") + mock_open.assert_not_called() + @unittest.skipIf(is_emscripten, "subprocess not available") def test_run_nonexistent_script_exits_cleanly(self): """Test that running a non-existent script exits with a clean error."""