Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-107467: Restructure Argument Clinic command-line interface #107469

Merged
merged 8 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 33 additions & 41 deletions Lib/test/test_clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@

from test import support, test_tools
from test.support import os_helper
from test.support import SHORT_TIMEOUT, requires_subprocess
from test.support.os_helper import TESTFN, unlink
from textwrap import dedent
from unittest import TestCase
import collections
import inspect
import os.path
import subprocess
import sys
import unittest

Expand Down Expand Up @@ -1391,30 +1389,26 @@ def test_scaffolding(self):

class ClinicExternalTest(TestCase):
maxDiff = None
clinic_py = os.path.join(test_tools.toolsdir, "clinic", "clinic.py")

def _do_test(self, *args, expect_success=True):
with subprocess.Popen(
[sys.executable, "-Xutf8", self.clinic_py, *args],
encoding="utf-8",
bufsize=0,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
) as proc:
proc.wait()
if expect_success and proc.returncode:
self.fail("".join([*proc.stdout, *proc.stderr]))
stdout = proc.stdout.read()
stderr = proc.stderr.read()
# Clinic never writes to stderr.
self.assertEqual(stderr, "")
return stdout

def run_clinic(self, *args):
with (
support.captured_stdout() as out,
support.captured_stderr() as err,
self.assertRaises(SystemExit) as cm
):
clinic.main(args)
return out.getvalue(), err.getvalue(), cm.exception.code

def expect_success(self, *args):
return self._do_test(*args)
out, err, code = self.run_clinic(*args)
self.assertEqual(code, 0, f"Unexpected failure: {args=}")
self.assertEqual(err, "")
return out

def expect_failure(self, *args):
return self._do_test(*args, expect_success=False)
out, err, code = self.run_clinic(*args)
self.assertNotEqual(code, 0, f"Unexpected success: {args=}")
return out, err

def test_external(self):
CLINIC_TEST = 'clinic.test.c'
Expand Down Expand Up @@ -1478,8 +1472,9 @@ def test_cli_force(self):
# First, run the CLI without -f and expect failure.
# Note, we cannot check the entire fail msg, because the path to
# the tmp file will change for every run.
out = self.expect_failure(fn)
self.assertTrue(out.endswith(fail_msg))
out, _ = self.expect_failure(fn)
self.assertTrue(out.endswith(fail_msg),
f"{out!r} does not end with {fail_msg!r}")
# Then, force regeneration; success expected.
out = self.expect_success("-f", fn)
self.assertEqual(out, "")
Expand Down Expand Up @@ -1621,33 +1616,30 @@ def test_cli_converters(self):
)

def test_cli_fail_converters_and_filename(self):
out = self.expect_failure("--converters", "test.c")
msg = (
"Usage error: can't specify --converters "
"and a filename at the same time"
)
self.assertIn(msg, out)
_, err = self.expect_failure("--converters", "test.c")
msg = "can't specify --converters and a filename at the same time"
self.assertIn(msg, err)

def test_cli_fail_no_filename(self):
out = self.expect_failure()
self.assertIn("usage: clinic.py", out)
_, err = self.expect_failure()
self.assertIn("no input files", err)

def test_cli_fail_output_and_multiple_files(self):
out = self.expect_failure("-o", "out.c", "input.c", "moreinput.c")
msg = "Usage error: can't use -o with multiple filenames"
self.assertIn(msg, out)
_, err = self.expect_failure("-o", "out.c", "input.c", "moreinput.c")
msg = "error: can't use -o with multiple filenames"
self.assertIn(msg, err)

def test_cli_fail_filename_or_output_and_make(self):
msg = "can't use -o or filenames with --make"
for opts in ("-o", "out.c"), ("filename.c",):
with self.subTest(opts=opts):
out = self.expect_failure("--make", *opts)
msg = "Usage error: can't use -o or filenames with --make"
self.assertIn(msg, out)
_, err = self.expect_failure("--make", *opts)
self.assertIn(msg, err)

def test_cli_fail_make_without_srcdir(self):
out = self.expect_failure("--make", "--srcdir", "")
msg = "Usage error: --srcdir must not be empty with --make"
self.assertIn(msg, out)
_, err = self.expect_failure("--make", "--srcdir", "")
msg = "error: --srcdir must not be empty with --make"
self.assertIn(msg, err)


try:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The Argument Clinic command-line tool now prints to stderr instead of stdout
on failure.
47 changes: 22 additions & 25 deletions Tools/clinic/clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from __future__ import annotations

import abc
import argparse
import ast
import builtins as bltns
import collections
Expand Down Expand Up @@ -5620,10 +5621,9 @@ def state_terminal(self, line: str | None) -> None:
clinic = None


def main(argv: list[str]) -> None:
import sys
import argparse
def create_cli() -> argparse.ArgumentParser:
cmdline = argparse.ArgumentParser(
prog="clinic.py",
description="""Preprocessor for CPython C files.

The purpose of the Argument Clinic is automating all the boilerplate involved
Expand All @@ -5646,14 +5646,15 @@ def main(argv: list[str]) -> None:
help="the directory tree to walk in --make mode")
cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*",
help="the list of files to process")
ns = cmdline.parse_args(argv)
return cmdline


def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None:
if ns.converters:
if ns.filename:
print("Usage error: can't specify --converters and a filename at the same time.")
print()
cmdline.print_usage()
sys.exit(-1)
parser.error(
"can't specify --converters and a filename at the same time"
)
converters: list[tuple[str, str]] = []
return_converters: list[tuple[str, str]] = []
ignored = set("""
Expand Down Expand Up @@ -5707,19 +5708,13 @@ def main(argv: list[str]) -> None:
print()
print("All converters also accept (c_default=None, py_default=None, annotation=None).")
print("All return converters also accept (py_default=None).")
sys.exit(0)
return

if ns.make:
if ns.output or ns.filename:
print("Usage error: can't use -o or filenames with --make.")
print()
cmdline.print_usage()
sys.exit(-1)
parser.error("can't use -o or filenames with --make")
if not ns.srcdir:
print("Usage error: --srcdir must not be empty with --make.")
print()
cmdline.print_usage()
sys.exit(-1)
parser.error("--srcdir must not be empty with --make")
for root, dirs, files in os.walk(ns.srcdir):
for rcs_dir in ('.svn', '.git', '.hg', 'build', 'externals'):
if rcs_dir in dirs:
Expand All @@ -5735,21 +5730,23 @@ def main(argv: list[str]) -> None:
return

if not ns.filename:
cmdline.print_usage()
sys.exit(-1)
parser.error("no input files")

if ns.output and len(ns.filename) > 1:
print("Usage error: can't use -o with multiple filenames.")
print()
cmdline.print_usage()
sys.exit(-1)
parser.error("can't use -o with multiple filenames")

for filename in ns.filename:
if ns.verbose:
print(filename)
parse_file(filename, output=ns.output, verify=not ns.force)


if __name__ == "__main__":
main(sys.argv[1:])
def main(argv: list[str] | None = None) -> NoReturn:
parser = create_cli()
args = parser.parse_args(argv)
run_clinic(parser, args)
sys.exit(0)
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved


if __name__ == "__main__":
main()
Loading