Skip to content

Commit 6e4c0d9

Browse files
committed
Address Laszlo's review
1 parent 170760b commit 6e4c0d9

File tree

2 files changed

+76
-9
lines changed

2 files changed

+76
-9
lines changed

Lib/profiling/sampling/cli.py

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -263,15 +263,15 @@ def _add_pstats_options(parser):
263263
"nsamples-cumul",
264264
"name",
265265
],
266-
default="nsamples",
267-
help="Sort order for pstats output",
266+
default=None,
267+
help="Sort order for pstats output (default: nsamples)",
268268
)
269269
pstats_group.add_argument(
270270
"-l",
271271
"--limit",
272272
type=int,
273-
default=15,
274-
help="Limit the number of rows in the output",
273+
default=None,
274+
help="Limit the number of rows in the output (default: 15)",
275275
)
276276
pstats_group.add_argument(
277277
"--no-summary",
@@ -343,10 +343,12 @@ def _handle_output(collector, args, pid, mode):
343343
if args.outfile:
344344
collector.export(args.outfile)
345345
else:
346-
# Print to stdout
347-
sort_mode = _sort_to_mode(args.sort)
346+
# Print to stdout with defaults applied
347+
sort_choice = args.sort if args.sort is not None else "nsamples"
348+
limit = args.limit if args.limit is not None else 15
349+
sort_mode = _sort_to_mode(sort_choice)
348350
collector.print_stats(
349-
sort_mode, args.limit, not args.no_summary, mode
351+
sort_mode, limit, not args.no_summary, mode
350352
)
351353
else:
352354
# Export to file
@@ -374,6 +376,21 @@ def _validate_args(args, parser):
374376
parser.error(
375377
f"--live is incompatible with {format_flag}. Live mode uses a TUI interface."
376378
)
379+
380+
# Live mode is also incompatible with pstats-specific options
381+
issues = []
382+
if args.sort is not None:
383+
issues.append("--sort")
384+
if args.limit is not None:
385+
issues.append("--limit")
386+
if args.no_summary:
387+
issues.append("--no-summary")
388+
389+
if issues:
390+
parser.error(
391+
f"Options {', '.join(issues)} are incompatible with --live. "
392+
"Live mode uses a TUI interface with its own controls."
393+
)
377394
return
378395

379396
# Validate gecko mode doesn't use non-wall mode
@@ -386,9 +403,9 @@ def _validate_args(args, parser):
386403
# Validate pstats-specific options are only used with pstats format
387404
if args.format != "pstats":
388405
issues = []
389-
if args.sort != "nsamples":
406+
if args.sort is not None:
390407
issues.append("--sort")
391-
if args.limit != 15:
408+
if args.limit is not None:
392409
issues.append("--limit")
393410
if args.no_summary:
394411
issues.append("--no-summary")

Lib/test/test_profiling/test_sampling_profiler/test_integration.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,3 +730,53 @@ def test_script_error_treatment(self):
730730
self.assertIn(
731731
"No such file or directory: 'nonexistent_file.txt'", output
732732
)
733+
734+
def test_live_incompatible_with_pstats_options(self):
735+
"""Test that --live is incompatible with individual pstats options."""
736+
test_cases = [
737+
(["--sort", "tottime"], "--sort"),
738+
(["--limit", "30"], "--limit"),
739+
(["--no-summary"], "--no-summary"),
740+
]
741+
742+
for args, expected_flag in test_cases:
743+
with self.subTest(args=args):
744+
test_args = ["profiling.sampling.cli", "run", "--live"] + args + ["test.py"]
745+
with mock.patch("sys.argv", test_args):
746+
with self.assertRaises(SystemExit) as cm:
747+
from profiling.sampling.cli import main
748+
main()
749+
self.assertNotEqual(cm.exception.code, 0)
750+
751+
def test_live_incompatible_with_multiple_pstats_options(self):
752+
"""Test that --live is incompatible with multiple pstats options."""
753+
test_args = [
754+
"profiling.sampling.cli", "run", "--live",
755+
"--sort", "cumtime", "--limit", "25", "--no-summary", "test.py"
756+
]
757+
758+
with mock.patch("sys.argv", test_args):
759+
with self.assertRaises(SystemExit) as cm:
760+
from profiling.sampling.cli import main
761+
main()
762+
self.assertNotEqual(cm.exception.code, 0)
763+
764+
def test_live_incompatible_with_pstats_default_values(self):
765+
"""Test that --live blocks pstats options even with default values."""
766+
# Test with --sort=nsamples (the default value)
767+
test_args = ["profiling.sampling.cli", "run", "--live", "--sort=nsamples", "test.py"]
768+
769+
with mock.patch("sys.argv", test_args):
770+
with self.assertRaises(SystemExit) as cm:
771+
from profiling.sampling.cli import main
772+
main()
773+
self.assertNotEqual(cm.exception.code, 0)
774+
775+
# Test with --limit=15 (the default value)
776+
test_args = ["profiling.sampling.cli", "run", "--live", "--limit=15", "test.py"]
777+
778+
with mock.patch("sys.argv", test_args):
779+
with self.assertRaises(SystemExit) as cm:
780+
from profiling.sampling.cli import main
781+
main()
782+
self.assertNotEqual(cm.exception.code, 0)

0 commit comments

Comments
 (0)