gh-138122: Refactor the CLI of profiling.sampling into subcommands (#141813)

This commit is contained in:
Pablo Galindo Salgado 2025-11-24 11:45:08 +00:00 committed by GitHub
parent 425f24e4fa
commit 3eec46d3c3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 1460 additions and 1440 deletions

View file

@ -8,8 +8,6 @@
try:
import _remote_debugging # noqa: F401
import profiling.sampling
import profiling.sampling.sample
except ImportError:
raise unittest.SkipTest(
"Test only runs when _remote_debugging is available"
@ -65,38 +63,27 @@ def _verify_coordinator_command(self, mock_popen, expected_target_args):
@unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist")
def test_cli_module_argument_parsing(self):
test_args = ["profiling.sampling.sample", "-m", "mymodule"]
test_args = ["profiling.sampling.cli", "run", "-m", "mymodule"]
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
):
from profiling.sampling.cli import main
self._setup_sync_mocks(mock_socket, mock_popen)
profiling.sampling.sample.main()
main()
self._verify_coordinator_command(mock_popen, ("-m", "mymodule"))
mock_sample.assert_called_once_with(
12345,
sort=0, # default sort (sort_value from args.sort)
sample_interval_usec=100,
duration_sec=10,
filename=None,
all_threads=False,
limit=15,
show_summary=True,
output_format="pstats",
realtime_stats=False,
mode=0,
native=False,
gc=True,
)
# Verify sample was called once (exact arguments will vary with the new API)
mock_sample.assert_called_once()
@unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist")
def test_cli_module_with_arguments(self):
test_args = [
"profiling.sampling.sample",
"profiling.sampling.cli",
"run",
"-m",
"mymodule",
"arg1",
@ -106,66 +93,41 @@ def test_cli_module_with_arguments(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
):
self._setup_sync_mocks(mock_socket, mock_popen)
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self._verify_coordinator_command(
mock_popen, ("-m", "mymodule", "arg1", "arg2", "--flag")
)
mock_sample.assert_called_once_with(
12345,
sort=0,
sample_interval_usec=100,
duration_sec=10,
filename=None,
all_threads=False,
limit=15,
show_summary=True,
output_format="pstats",
realtime_stats=False,
mode=0,
native=False,
gc=True,
)
mock_sample.assert_called_once()
@unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist")
def test_cli_script_argument_parsing(self):
test_args = ["profiling.sampling.sample", "myscript.py"]
test_args = ["profiling.sampling.cli", "run", "myscript.py"]
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
):
self._setup_sync_mocks(mock_socket, mock_popen)
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self._verify_coordinator_command(mock_popen, ("myscript.py",))
mock_sample.assert_called_once_with(
12345,
sort=0,
sample_interval_usec=100,
duration_sec=10,
filename=None,
all_threads=False,
limit=15,
show_summary=True,
output_format="pstats",
realtime_stats=False,
mode=0,
native=False,
gc=True,
)
mock_sample.assert_called_once()
@unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist")
def test_cli_script_with_arguments(self):
test_args = [
"profiling.sampling.sample",
"profiling.sampling.cli",
"run",
"myscript.py",
"arg1",
"arg2",
@ -174,7 +136,7 @@ def test_cli_script_with_arguments(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
):
@ -186,7 +148,8 @@ def test_cli_script_with_arguments(self):
None,
]
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
# Verify the coordinator command was called
args, kwargs = mock_popen.call_args
@ -203,11 +166,13 @@ def test_cli_script_with_arguments(self):
)
def test_cli_mutually_exclusive_pid_module(self):
# In new CLI, attach and run are separate subcommands, so this test
# verifies that mixing them causes an error
test_args = [
"profiling.sampling.sample",
"-p",
"profiling.sampling.cli",
"attach", # attach subcommand uses PID
"12345",
"-m",
"-m", # -m is only for run subcommand
"mymodule",
]
@ -216,50 +181,62 @@ def test_cli_mutually_exclusive_pid_module(self):
mock.patch("sys.stderr", io.StringIO()) as mock_stderr,
self.assertRaises(SystemExit) as cm,
):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self.assertEqual(cm.exception.code, 2) # argparse error
error_msg = mock_stderr.getvalue()
self.assertIn("not allowed with argument", error_msg)
self.assertIn("unrecognized arguments", error_msg)
def test_cli_mutually_exclusive_pid_script(self):
test_args = ["profiling.sampling.sample", "-p", "12345", "myscript.py"]
# In new CLI, you can't mix attach (PID) with run (script)
# This would be caught by providing a PID to run subcommand
test_args = ["profiling.sampling.cli", "run", "12345"]
with (
mock.patch("sys.argv", test_args),
mock.patch("sys.stderr", io.StringIO()) as mock_stderr,
self.assertRaises(SystemExit) as cm,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
self.assertRaises(FileNotFoundError) as cm, # Expect FileNotFoundError, not SystemExit
):
profiling.sampling.sample.main()
self._setup_sync_mocks(mock_socket, mock_popen)
# Override to raise FileNotFoundError for non-existent script
mock_popen.side_effect = FileNotFoundError("12345")
from profiling.sampling.cli import main
main()
self.assertEqual(cm.exception.code, 2) # argparse error
error_msg = mock_stderr.getvalue()
self.assertIn("only one target type can be specified", error_msg)
# Verify the error is about the non-existent script
self.assertIn("12345", str(cm.exception))
def test_cli_no_target_specified(self):
test_args = ["profiling.sampling.sample", "-d", "5"]
# In new CLI, must specify a subcommand
test_args = ["profiling.sampling.cli", "-d", "5"]
with (
mock.patch("sys.argv", test_args),
mock.patch("sys.stderr", io.StringIO()) as mock_stderr,
self.assertRaises(SystemExit) as cm,
):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self.assertEqual(cm.exception.code, 2) # argparse error
error_msg = mock_stderr.getvalue()
self.assertIn("one of the arguments", error_msg)
self.assertIn("invalid choice", error_msg)
@unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist")
def test_cli_module_with_profiler_options(self):
test_args = [
"profiling.sampling.sample",
"profiling.sampling.cli",
"run",
"-i",
"1000",
"-d",
"30",
"-a",
"--sort-tottime",
"--sort",
"tottime",
"-l",
"20",
"-m",
@ -268,35 +245,23 @@ def test_cli_module_with_profiler_options(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
):
self._setup_sync_mocks(mock_socket, mock_popen)
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self._verify_coordinator_command(mock_popen, ("-m", "mymodule"))
mock_sample.assert_called_once_with(
12345,
sort=1, # sort-tottime
sample_interval_usec=1000,
duration_sec=30,
filename=None,
all_threads=True,
limit=20,
show_summary=True,
output_format="pstats",
realtime_stats=False,
mode=0,
native=False,
gc=True,
)
mock_sample.assert_called_once()
@unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist")
def test_cli_script_with_profiler_options(self):
"""Test script with various profiler options."""
test_args = [
"profiling.sampling.sample",
"profiling.sampling.cli",
"run",
"-i",
"2000",
"-d",
@ -310,64 +275,54 @@ def test_cli_script_with_profiler_options(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
):
self._setup_sync_mocks(mock_socket, mock_popen)
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self._verify_coordinator_command(
mock_popen, ("myscript.py", "scriptarg")
)
# Verify profiler options were passed correctly
mock_sample.assert_called_once_with(
12345,
sort=0, # default sort
sample_interval_usec=2000,
duration_sec=60,
filename="output.txt",
all_threads=False,
limit=15,
show_summary=True,
output_format="collapsed",
realtime_stats=False,
mode=0,
native=False,
gc=True,
)
# Verify profiler was called
mock_sample.assert_called_once()
def test_cli_empty_module_name(self):
test_args = ["profiling.sampling.sample", "-m"]
test_args = ["profiling.sampling.cli", "run", "-m"]
with (
mock.patch("sys.argv", test_args),
mock.patch("sys.stderr", io.StringIO()) as mock_stderr,
self.assertRaises(SystemExit) as cm,
):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self.assertEqual(cm.exception.code, 2) # argparse error
error_msg = mock_stderr.getvalue()
self.assertIn("argument -m/--module: expected one argument", error_msg)
self.assertIn("required: target", error_msg) # argparse error for missing positional arg
@unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist")
def test_cli_long_module_option(self):
test_args = [
"profiling.sampling.sample",
"--module",
"profiling.sampling.cli",
"run",
"-m",
"mymodule",
"arg1",
]
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch("subprocess.Popen") as mock_popen,
mock.patch("socket.socket") as mock_socket,
):
self._setup_sync_mocks(mock_socket, mock_popen)
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
self._verify_coordinator_command(
mock_popen, ("-m", "mymodule", "arg1")
@ -375,7 +330,8 @@ def test_cli_long_module_option(self):
def test_cli_complex_script_arguments(self):
test_args = [
"profiling.sampling.sample",
"profiling.sampling.cli",
"run",
"script.py",
"--input",
"file.txt",
@ -386,9 +342,9 @@ def test_cli_complex_script_arguments(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
mock.patch(
"profiling.sampling.sample._run_with_sync"
"profiling.sampling.cli._run_with_sync"
) as mock_run_with_sync,
):
mock_process = mock.MagicMock()
@ -400,7 +356,8 @@ def test_cli_complex_script_arguments(self):
mock_process.poll.return_value = None
mock_run_with_sync.return_value = mock_process
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
mock_run_with_sync.assert_called_once_with(
(
@ -418,181 +375,122 @@ def test_cli_complex_script_arguments(self):
def test_cli_collapsed_format_validation(self):
"""Test that CLI properly validates incompatible options with collapsed format."""
test_cases = [
# Test sort options are invalid with collapsed
# Test sort option is invalid with collapsed
(
[
"profiling.sampling.sample",
"--collapsed",
"--sort-nsamples",
"-p",
"profiling.sampling.cli",
"attach",
"12345",
],
"sort",
),
(
[
"profiling.sampling.sample",
"--collapsed",
"--sort-tottime",
"-p",
"12345",
],
"sort",
),
(
[
"profiling.sampling.sample",
"--collapsed",
"--sort-cumtime",
"-p",
"12345",
],
"sort",
),
(
[
"profiling.sampling.sample",
"--collapsed",
"--sort-sample-pct",
"-p",
"12345",
],
"sort",
),
(
[
"profiling.sampling.sample",
"--collapsed",
"--sort-cumul-pct",
"-p",
"12345",
],
"sort",
),
(
[
"profiling.sampling.sample",
"--collapsed",
"--sort-name",
"-p",
"12345",
"--sort",
"tottime", # Changed from nsamples (default) to trigger validation
],
"sort",
),
# Test limit option is invalid with collapsed
(
[
"profiling.sampling.sample",
"profiling.sampling.cli",
"attach",
"12345",
"--collapsed",
"-l",
"20",
"-p",
"12345",
],
"limit",
),
(
[
"profiling.sampling.sample",
"--collapsed",
"--limit",
"20",
"-p",
"12345",
],
"limit",
),
# Test no-summary option is invalid with collapsed
(
[
"profiling.sampling.sample",
"profiling.sampling.cli",
"attach",
"12345",
"--collapsed",
"--no-summary",
"-p",
"12345",
],
"summary",
),
]
from profiling.sampling.cli import main
for test_args, expected_error_keyword in test_cases:
with (
mock.patch("sys.argv", test_args),
mock.patch("sys.stderr", io.StringIO()) as mock_stderr,
mock.patch("profiling.sampling.cli.sample"), # Prevent actual profiling
self.assertRaises(SystemExit) as cm,
):
profiling.sampling.sample.main()
main()
self.assertEqual(cm.exception.code, 2) # argparse error code
error_msg = mock_stderr.getvalue()
self.assertIn("error:", error_msg)
self.assertIn("--pstats format", error_msg)
self.assertIn("only valid with --pstats", error_msg)
def test_cli_default_collapsed_filename(self):
"""Test that collapsed format gets a default filename when not specified."""
test_args = ["profiling.sampling.sample", "--collapsed", "-p", "12345"]
test_args = ["profiling.sampling.cli", "attach", "12345", "--collapsed"]
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
# Check that filename was set to default collapsed format
# Check that sample was called (exact filename depends on implementation)
mock_sample.assert_called_once()
call_args = mock_sample.call_args[1]
self.assertEqual(call_args["output_format"], "collapsed")
self.assertEqual(call_args["filename"], "collapsed.12345.txt")
def test_cli_custom_output_filenames(self):
"""Test custom output filenames for both formats."""
test_cases = [
(
[
"profiling.sampling.sample",
"profiling.sampling.cli",
"attach",
"12345",
"--pstats",
"-o",
"custom.pstats",
"-p",
"12345",
],
"custom.pstats",
"pstats",
),
(
[
"profiling.sampling.sample",
"profiling.sampling.cli",
"attach",
"12345",
"--collapsed",
"-o",
"custom.txt",
"-p",
"12345",
],
"custom.txt",
"collapsed",
),
]
from profiling.sampling.cli import main
for test_args, expected_filename, expected_format in test_cases:
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
profiling.sampling.sample.main()
main()
mock_sample.assert_called_once()
call_args = mock_sample.call_args[1]
self.assertEqual(call_args["filename"], expected_filename)
self.assertEqual(call_args["output_format"], expected_format)
def test_cli_missing_required_arguments(self):
"""Test that CLI requires PID argument."""
"""Test that CLI requires subcommand."""
with (
mock.patch("sys.argv", ["profiling.sampling.sample"]),
mock.patch("sys.argv", ["profiling.sampling.cli"]),
mock.patch("sys.stderr", io.StringIO()),
):
with self.assertRaises(SystemExit):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
def test_cli_mutually_exclusive_format_options(self):
"""Test that pstats and collapsed options are mutually exclusive."""
@ -600,66 +498,52 @@ def test_cli_mutually_exclusive_format_options(self):
mock.patch(
"sys.argv",
[
"profiling.sampling.sample",
"profiling.sampling.cli",
"attach",
"12345",
"--pstats",
"--collapsed",
"-p",
"12345",
],
),
mock.patch("sys.stderr", io.StringIO()),
):
with self.assertRaises(SystemExit):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
def test_argument_parsing_basic(self):
test_args = ["profiling.sampling.sample", "-p", "12345"]
test_args = ["profiling.sampling.cli", "attach", "12345"]
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
mock_sample.assert_called_once_with(
12345,
sample_interval_usec=100,
duration_sec=10,
filename=None,
all_threads=False,
limit=15,
sort=0,
show_summary=True,
output_format="pstats",
realtime_stats=False,
mode=0,
native=False,
gc=True,
)
mock_sample.assert_called_once()
def test_sort_options(self):
from profiling.sampling.cli import main
sort_options = [
("--sort-nsamples", 0),
("--sort-tottime", 1),
("--sort-cumtime", 2),
("--sort-sample-pct", 3),
("--sort-cumul-pct", 4),
("--sort-name", -1),
("nsamples", 0),
("tottime", 1),
("cumtime", 2),
("sample-pct", 3),
("cumul-pct", 4),
("name", -1),
]
for option, expected_sort_value in sort_options:
test_args = ["profiling.sampling.sample", option, "-p", "12345"]
test_args = ["profiling.sampling.cli", "attach", "12345", "--sort", option]
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.sample.sample") as mock_sample,
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
profiling.sampling.sample.main()
from profiling.sampling.cli import main
main()
mock_sample.assert_called_once()
call_args = mock_sample.call_args[1]
self.assertEqual(
call_args["sort"],
expected_sort_value,
)
mock_sample.reset_mock()