Revert "pythonGH-120754: Add a strace helper and test set of syscalls…

… for o… (python#123303)

Revert "pythonGH-120754: Add a strace helper and test set of syscalls for open().read() (python#121143)"

This reverts commit e38d0af.

Lib/test/test_fileio.py

@@ -11,7 +11,7 @@
 
 from test.support import (
  cpython_only, swap_attr, gc_collect, is_emscripten, is_wasi,
- infinite_recursion, strace_helper
+ infinite_recursion,
 )
 from test.support.os_helper import (
  TESTFN, TESTFN_ASCII, TESTFN_UNICODE, make_bad_fd,
@@ -24,9 +24,6 @@
 import _pyio # Python implementation of io
 
 
-_strace_flags=["--trace=%file,%desc"]
-
-
 class AutoFileTests:
  # file tests for which a test file is automatically set up
 
@@ -362,94 +359,6 @@ def testErrnoOnClosedReadinto(self, f):
  a = array('b', b'x'*10)
  f.readinto(a)
 
- @strace_helper.requires_strace()
- def test_syscalls_read(self):
- """Check that the set of system calls produced by the I/O stack is what
- is expected for various read cases.
-
- It's expected as bits of the I/O implementation change, this will need
- to change. The goal is to catch changes that unintentionally add
- additional systemcalls (ex. additional calls have been looked at in
- bpo-21679 and gh-120754).
- """
- self.f.write(b"Hello, World!")
- self.f.close()
-
-
- def check_readall(name, code, prelude="", cleanup=""):
- with self.subTest(name=name):
- syscalls = strace_helper.get_syscalls(code, _strace_flags,
- prelude=prelude,
- cleanup=cleanup)
-
- # There are a number of related syscalls used to implement
- # behaviors in a libc (ex. fstat, newfstatat, open, openat).
- # Allow any that use the same substring.
- def count_similarname(name):
- return len([sc for sc in syscalls if name in sc])
-
- # Should open and close the file exactly once
- self.assertEqual(count_similarname('open'), 1)
- self.assertEqual(count_similarname('close'), 1)
-
- # Should only have one fstat (bpo-21679, gh-120754)
- self.assertEqual(count_similarname('fstat'), 1)
-
-
- # "open, read, close" file using different common patterns.
- check_readall(
- "open builtin with default options",
- f"""
- f = open('{TESTFN}')
- f.read()
- f.close()
- """
- )
-
- check_readall(
- "open in binary mode",
- f"""
- f = open('{TESTFN}', 'rb')
- f.read()
- f.close()
- """
- )
-
- check_readall(
- "open in text mode",
- f"""
- f = open('{TESTFN}', 'rt')
- f.read()
- f.close()
- """
- )
-
- check_readall(
- "pathlib read_bytes",
- "p.read_bytes()",
- prelude=f"""from pathlib import Path; p = Path("{TESTFN}")"""
-
- )
-
- check_readall(
- "pathlib read_text",
- "p.read_text()",
- prelude=f"""from pathlib import Path; p = Path("{TESTFN}")"""
- )
-
- # Focus on just `read()`.
- calls = strace_helper.get_syscalls(
- prelude=f"f = open('{TESTFN}')",
- code="f.read()",
- cleanup="f.close()",
- strace_flags=_strace_flags
- )
- # One to read all the bytes
- # One to read the EOF and get a size 0 return.
- self.assertEqual(calls.count("read"), 2)
-
-
-
 class CAutoFileTests(AutoFileTests, unittest.TestCase):
  FileIO = _io.FileIO
  modulename = '_io'

Lib/test/test_subprocess.py

@@ -4,7 +4,6 @@
 from test.support import check_sanitizer
 from test.support import import_helper
 from test.support import os_helper
-from test.support import strace_helper
 from test.support import warnings_helper
 from test.support.script_helper import assert_python_ok
 import subprocess
@@ -3416,33 +3415,44 @@ def __del__(self):
 
  @unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"),
  "vfork() not enabled by configure.")
- @strace_helper.requires_strace()
+ @unittest.skipIf(sys.platform != "linux", "Linux only, requires strace.")
  @mock.patch("subprocess._USE_POSIX_SPAWN", new=False)
  def test_vfork_used_when_expected(self):
  # This is a performance regression test to ensure we default to using
  # vfork() when possible.
  # Technically this test could pass when posix_spawn is used as well
  # because libc tends to implement that internally using vfork. But
  # that'd just be testing a libc+kernel implementation detail.
-
- # Are intersted in the system calls:
- # clone,clone2,clone3,fork,vfork,exit,exit_group
- # Unfortunately using `--trace` with that list to strace fails because
- # not all are supported on all platforms (ex. clone2 is ia64 only...)
- # So instead use `%process` which is recommended by strace, and contains
- # the above.
+ strace_binary = "/usr/bin/strace"
+ # The only system calls we are interested in.
+ strace_filter = "--trace=clone,clone2,clone3,fork,vfork,exit,exit_group"
  true_binary = "/bin/true"
- strace_args = ["--trace=%process"]
+ strace_command = [strace_binary, strace_filter]
+
+ try:
+ does_strace_work_process = subprocess.run(
+ strace_command + [true_binary],
+ stderr=subprocess.PIPE,
+ stdout=subprocess.DEVNULL,
+ )
+ rc = does_strace_work_process.returncode
+ stderr = does_strace_work_process.stderr
+ except OSError:
+ rc = -1
+ stderr = ""
+ if rc or (b"+++ exited with 0 +++" not in stderr):
+ self.skipTest("strace not found or not working as expected.")
 
  with self.subTest(name="default_is_vfork"):
- vfork_result = strace_helper.strace_python(
- f"""\
- import subprocess
- subprocess.check_call([{true_binary!r}])""",
- strace_args
+ vfork_result = assert_python_ok(
+ "-c",
+ textwrap.dedent(f"""\
+ import subprocess
+ subprocess.check_call([{true_binary!r}])"""),
+ __run_using_command=strace_command,
  )
  # Match both vfork() and clone(..., flags=...|CLONE_VFORK|...)
- self.assertRegex(vfork_result.event_bytes, br"(?i)vfork")
+ self.assertRegex(vfork_result.err, br"(?i)vfork")
  # Do NOT check that fork() or other clones did not happen.
  # If the OS denys the vfork it'll fallback to plain fork().
 
@@ -3455,20 +3465,21 @@ def test_vfork_used_when_expected(self):
  ("setgroups", "", "extra_groups=[]", True),
  ):
  with self.subTest(name=sub_name):
- non_vfork_result = strace_helper.strace_python(
- f"""\
+ non_vfork_result = assert_python_ok(
+ "-c",
+ textwrap.dedent(f"""\
  import subprocess
  {preamble}
  try:
  subprocess.check_call(
  [{true_binary!r}], **dict({sp_kwarg}))
  except PermissionError:
  if not {expect_permission_error}:
- raise""",
- strace_args
+ raise"""),
+ __run_using_command=strace_command,
  )
  # Ensure neither vfork() or clone(..., flags=...|CLONE_VFORK|...).
- self.assertNotRegex(non_vfork_result.event_bytes, br"(?i)vfork")
+ self.assertNotRegex(non_vfork_result.err, br"(?i)vfork")
 
 
 @unittest.skipUnless(mswindows, "Windows specific tests")

Misc/ACKS

@@ -1164,7 +1164,6 @@ Grzegorz Makarewicz
 David Malcolm
 Greg Malcolm
 William Mallard
-Cody Maloney
 Ken Manheimer
 Vladimir Marangozov
 Colin Marc