gh-142654: show the clear error message when sampling on an unknown PID in tachyon (#142655)

Co-authored-by: Pablo Galindo Salgado <pablogsal@gmail.com>
This commit is contained in:
Keming 2025-12-17 22:15:22 +08:00 committed by GitHub
parent 1fc3039d71
commit d4095f25e8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 104 additions and 43 deletions

View file

@ -16,6 +16,7 @@
from test.support import is_emscripten, requires_remote_subprocess_debugging
from profiling.sampling.cli import main
from profiling.sampling.errors import SamplingScriptNotFoundError, SamplingModuleNotFoundError, SamplingUnknownProcessError
class TestSampleProfilerCLI(unittest.TestCase):
@ -203,12 +204,12 @@ def test_cli_mutually_exclusive_pid_script(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("sys.stderr", io.StringIO()) as mock_stderr,
self.assertRaises(SystemExit) as cm,
self.assertRaises(SamplingScriptNotFoundError) as cm,
):
main()
# Verify the error is about the non-existent script
self.assertIn("12345", str(cm.exception.code))
self.assertIn("12345", str(cm.exception))
def test_cli_no_target_specified(self):
# In new CLI, must specify a subcommand
@ -436,6 +437,7 @@ def test_cli_default_collapsed_filename(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
main()
@ -475,6 +477,7 @@ def test_cli_custom_output_filenames(self):
for test_args, expected_filename, expected_format in test_cases:
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
main()
@ -513,6 +516,7 @@ def test_argument_parsing_basic(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
main()
@ -534,6 +538,7 @@ def test_sort_options(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
main()
@ -547,6 +552,7 @@ def test_async_aware_flag_defaults_to_running(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
main()
@ -562,6 +568,7 @@ def test_async_aware_with_async_mode_all(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
main()
@ -576,6 +583,7 @@ def test_async_aware_default_is_none(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
main()
@ -697,14 +705,20 @@ def test_async_aware_incompatible_with_all_threads(self):
def test_run_nonexistent_script_exits_cleanly(self):
"""Test that running a non-existent script exits with a clean error."""
with mock.patch("sys.argv", ["profiling.sampling.cli", "run", "/nonexistent/script.py"]):
with self.assertRaises(SystemExit) as cm:
with self.assertRaisesRegex(SamplingScriptNotFoundError, "Script '[\\w/.]+' not found."):
main()
self.assertIn("Script not found", str(cm.exception.code))
@unittest.skipIf(is_emscripten, "subprocess not available")
def test_run_nonexistent_module_exits_cleanly(self):
"""Test that running a non-existent module exits with a clean error."""
with mock.patch("sys.argv", ["profiling.sampling.cli", "run", "-m", "nonexistent_module_xyz"]):
with self.assertRaises(SystemExit) as cm:
with self.assertRaisesRegex(SamplingModuleNotFoundError, "Module '[\\w/.]+' not found."):
main()
self.assertIn("Module not found", str(cm.exception.code))
def test_cli_attach_nonexistent_pid(self):
fake_pid = "99999"
with mock.patch("sys.argv", ["profiling.sampling.cli", "attach", fake_pid]):
with self.assertRaises(SamplingUnknownProcessError) as cm:
main()
self.assertIn(fake_pid, str(cm.exception))

View file

@ -17,7 +17,7 @@
import profiling.sampling.sample
from profiling.sampling.pstats_collector import PstatsCollector
from profiling.sampling.stack_collector import CollapsedStackCollector
from profiling.sampling.sample import SampleProfiler
from profiling.sampling.sample import SampleProfiler, _is_process_running
except ImportError:
raise unittest.SkipTest(
"Test only runs when _remote_debugging is available"
@ -602,7 +602,7 @@ def test_sample_target_module(self):
@requires_remote_subprocess_debugging()
class TestSampleProfilerErrorHandling(unittest.TestCase):
def test_invalid_pid(self):
with self.assertRaises((OSError, RuntimeError)):
with self.assertRaises((SystemExit, PermissionError)):
collector = PstatsCollector(sample_interval_usec=100, skip_idle=False)
profiling.sampling.sample.sample(-1, collector, duration_sec=1)
@ -638,7 +638,7 @@ def test_is_process_running(self):
sample_interval_usec=1000,
all_threads=False,
)
self.assertTrue(profiler._is_process_running())
self.assertTrue(_is_process_running(profiler.pid))
self.assertIsNotNone(profiler.unwinder.get_stack_trace())
subproc.process.kill()
subproc.process.wait()
@ -647,7 +647,7 @@ def test_is_process_running(self):
)
# Exit the context manager to ensure the process is terminated
self.assertFalse(profiler._is_process_running())
self.assertFalse(_is_process_running(profiler.pid))
self.assertRaises(
ProcessLookupError, profiler.unwinder.get_stack_trace
)

View file

@ -252,6 +252,7 @@ def test_gil_mode_validation(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
try:
@ -313,6 +314,7 @@ def test_gil_mode_cli_argument_parsing(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
try:
@ -432,6 +434,7 @@ def test_exception_mode_validation(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
try:
@ -493,6 +496,7 @@ def test_exception_mode_cli_argument_parsing(self):
with (
mock.patch("sys.argv", test_args),
mock.patch("profiling.sampling.cli._is_process_running", return_value=True),
mock.patch("profiling.sampling.cli.sample") as mock_sample,
):
try: