From 12c0dc78c36a87249f63b1cc25cd9a177527e373 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 23 Jul 2023 23:11:20 +0200 Subject: [PATCH 01/10] gh-106368: Increase Argument Clinic CLI test coverage Instead of hacking into the Clinic class, use the Argument Clinic tool to run the ClinicWholeFileTest test suite. --- Lib/test/test_clinic.py | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index c24b8cd3899f28..b22735ac058e43 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -4,11 +4,14 @@ 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 @@ -1346,6 +1349,34 @@ def test_scaffolding(self): class ClinicExternalTest(TestCase): maxDiff = None + def _do_test(self, *args, expect_success=True): + clinic_py = os.path.join(test_tools.toolsdir, "clinic", "clinic.py") + with subprocess.Popen( + [sys.executable, "-Xutf8", clinic_py, *args], + encoding="utf-8", + bufsize=0, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) as proc: + proc.wait() + if expect_success == bool(proc.returncode): + self.fail("".join(proc.stderr)) + stdout = proc.stdout.read() + stderr = proc.stderr.read() + if expect_success: + self.assertEqual(stderr, "") + else: + self.assertEqual(stdout, "") + return stdout, stderr + + def expect_success(self, *args): + out, _ = self._do_test(*args) + return out + + def expect_failure(self, *args): + _, err = self._do_test(*args, expect_success=False) + return err + def test_external(self): CLINIC_TEST = 'clinic.test.c' # bpo-42398: Test that the destination file is left unchanged if the @@ -1361,7 +1392,9 @@ def test_external(self): f.write(orig_contents) old_mtime_ns = os.stat(testfile).st_mtime_ns - clinic.parse_file(testfile) + # Run clinic CLI and verify that it does not complain. + out = self.expect_success("-f", testfile) + self.assertEqual(out, "") with open(testfile, 'r', encoding='utf-8') as f: new_contents = f.read() @@ -1372,6 +1405,10 @@ def test_external(self): # if the content does not change self.assertEqual(new_mtime_ns, old_mtime_ns) + def test_cli_help(self): + out = self.expect_success("-h") + self.assertIn("usage: clinic.py", out) + try: import _testclinic as ac_tester From e09e4164a2290a121b5338490668853c3a72d23e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 23 Jul 2023 23:23:03 +0200 Subject: [PATCH 02/10] Add test for the --converters opt --- Lib/test/test_clinic.py | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index b22735ac058e43..d678f02a7d333e 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1409,6 +1409,60 @@ def test_cli_help(self): out = self.expect_success("-h") self.assertIn("usage: clinic.py", out) + def test_cli_converters(self): + expected = dedent(""" + Legacy converters: + B C D L O S U Y Z Z# + b c d f h i l p s s# s* u u# w* y y# y* z z# z* + + Converters: + bool(accept={}) + byte(bitwise=False) + char() + defining_class(type=None) + double() + fildes() + float() + int(accept={}, type=None) + long() + long_long() + object(converter=None, type=None, subclass_of=None) + Py_buffer(accept={}) + Py_complex() + Py_ssize_t(accept={}) + Py_UNICODE(accept={}, zeroes=False) + PyByteArrayObject() + PyBytesObject() + self(type=None) + short() + size_t() + slice_index(accept={, }) + str(accept={}, encoding=None, zeroes=False) + unicode() + unsigned_char(bitwise=False) + unsigned_int(bitwise=False) + unsigned_long(bitwise=False) + unsigned_long_long(bitwise=False) + unsigned_short(bitwise=False) + + Return converters: + bool() + double() + float() + init() + int() + long() + Py_ssize_t() + size_t() + unsigned_int() + unsigned_long() + + All converters also accept (c_default=None, py_default=None, annotation=None). + All return converters also accept (py_default=None). + """) + out = self.expect_success("--converters") + self.assertEqual(out, expected) + try: import _testclinic as ac_tester From f9d637b7531b2ba0b76398db8f698ac39748740b Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 23 Jul 2023 23:47:01 +0200 Subject: [PATCH 03/10] Fix converters test --- Lib/test/test_clinic.py | 111 ++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 50 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index d678f02a7d333e..a9d26d12963373 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1410,58 +1410,69 @@ def test_cli_help(self): self.assertIn("usage: clinic.py", out) def test_cli_converters(self): - expected = dedent(""" - Legacy converters: - B C D L O S U Y Z Z# - b c d f h i l p s s# s* u u# w* y y# y* z z# z* - - Converters: - bool(accept={}) - byte(bitwise=False) - char() - defining_class(type=None) - double() - fildes() - float() - int(accept={}, type=None) - long() - long_long() - object(converter=None, type=None, subclass_of=None) - Py_buffer(accept={}) - Py_complex() - Py_ssize_t(accept={}) - Py_UNICODE(accept={}, zeroes=False) - PyByteArrayObject() - PyBytesObject() - self(type=None) - short() - size_t() - slice_index(accept={, }) - str(accept={}, encoding=None, zeroes=False) - unicode() - unsigned_char(bitwise=False) - unsigned_int(bitwise=False) - unsigned_long(bitwise=False) - unsigned_long_long(bitwise=False) - unsigned_short(bitwise=False) - - Return converters: - bool() - double() - float() - init() - int() - long() - Py_ssize_t() - size_t() - unsigned_int() - unsigned_long() - - All converters also accept (c_default=None, py_default=None, annotation=None). - All return converters also accept (py_default=None). + prelude = dedent(""" + Legacy converters: + B C D L O S U Y Z Z# + b c d f h i l p s s# s* u u# w* y y# y* z z# z* + + Converters: + """) + expected_converters = ( + "bool", + "byte", + "char", + "defining_class", + "double", + "fildes", + "float", + "int", + "long", + "long_long", + "object", + "Py_buffer", + "Py_complex", + "Py_ssize_t", + "Py_UNICODE", + "PyByteArrayObject", + "PyBytesObject", + "self", + "short", + "size_t", + "slice_index", + "str", + "unicode", + "unsigned_char", + "unsigned_int", + "unsigned_long", + "unsigned_long_long", + "unsigned_short", + ) + finale = dedent(""" + Return converters: + bool() + double() + float() + init() + int() + long() + Py_ssize_t() + size_t() + unsigned_int() + unsigned_long() + + All converters also accept (c_default=None, py_default=None, annotation=None). + All return converters also accept (py_default=None). """) out = self.expect_success("--converters") - self.assertEqual(out, expected) + # We cannot simply compare the output, because the repr of the *accept* + # param may change (it's a set, thus unordered). So, let's compare the + # start and end of the expected output, and then assert we've got a + # line for each converter. + self.assertTrue(out.startswith(prelude)) + for converter in expected_converters: + with self.subTest(converter=converter): + self.assertIn(converter, out) + self.assertTrue(out.endswith(finale)) try: From 9f617def0b7c591a0fe0da94d20aeeca2313f55e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 24 Jul 2023 00:16:12 +0200 Subject: [PATCH 04/10] Split test_external in two test_external: run clinic on clinic.test.c test_no_change: check that dest is left unchanged if content does not change --- Lib/test/test_clinic.py | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index a9d26d12963373..c3b39889b73a6e 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1379,31 +1379,38 @@ def expect_failure(self, *args): def test_external(self): CLINIC_TEST = 'clinic.test.c' - # bpo-42398: Test that the destination file is left unchanged if the - # content does not change. Moreover, check also that the file - # modification time does not change in this case. source = support.findfile(CLINIC_TEST) with open(source, 'r', encoding='utf-8') as f: orig_contents = f.read() - with os_helper.temp_dir() as tmp_dir: - testfile = os.path.join(tmp_dir, CLINIC_TEST) - with open(testfile, 'w', encoding='utf-8') as f: - f.write(orig_contents) - old_mtime_ns = os.stat(testfile).st_mtime_ns + # Run clinic CLI and verify that it does not complain. + out = self.expect_success("-f", "-o", TESTFN, source) + self.assertEqual(out, "") - # Run clinic CLI and verify that it does not complain. - out = self.expect_success("-f", testfile) - self.assertEqual(out, "") - - with open(testfile, 'r', encoding='utf-8') as f: - new_contents = f.read() - new_mtime_ns = os.stat(testfile).st_mtime_ns + with open(TESTFN, 'r', encoding='utf-8') as f: + new_contents = f.read() self.assertEqual(new_contents, orig_contents) + + def test_no_change(self): + # bpo-42398: Test that the destination file is left unchanged if the + # content does not change. Moreover, check also that the file + # modification time does not change in this case. + code = dedent(""" + /*[clinic input] + [clinic start generated code]*/ + /*[clinic end generated code: output=da39a3ee5e6b4b0d input=da39a3ee5e6b4b0d]*/ + """) + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write(code) + pre_mtime = os.stat(fn).st_mtime_ns + self.expect_success(fn) + post_mtime = os.stat(fn).st_mtime_ns # Don't change the file modification time # if the content does not change - self.assertEqual(new_mtime_ns, old_mtime_ns) + self.assertEqual(pre_mtime, post_mtime) def test_cli_help(self): out = self.expect_success("-h") From 39ee0eb98dcc55cc09a0d42e4ea84fefe00acc59 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 24 Jul 2023 00:34:13 +0200 Subject: [PATCH 05/10] Test force/no-force runs --- Lib/test/test_clinic.py | 54 ++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index c3b39889b73a6e..8d93e2cd76625b 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1363,19 +1363,15 @@ def _do_test(self, *args, expect_success=True): self.fail("".join(proc.stderr)) stdout = proc.stdout.read() stderr = proc.stderr.read() - if expect_success: - self.assertEqual(stderr, "") - else: - self.assertEqual(stdout, "") - return stdout, stderr + # Clinic never writes to stderr. + self.assertEqual(stderr, "") + return stdout def expect_success(self, *args): - out, _ = self._do_test(*args) - return out + return self._do_test(*args) def expect_failure(self, *args): - _, err = self._do_test(*args, expect_success=False) - return err + return self._do_test(*args, expect_success=False) def test_external(self): CLINIC_TEST = 'clinic.test.c' @@ -1412,6 +1408,46 @@ def test_no_change(self): # if the content does not change self.assertEqual(pre_mtime, post_mtime) + def test_cli_force(self): + invalid_input = dedent(""" + /*[clinic input] + output preset block + module test + test.fn + a: int + [clinic start generated code]*/ + + const char *hand_edited = "output block is overwritten"; + /*[clinic end generated code: output=bogus input=bogus]*/ + """) + fail_msg = dedent(""" + Checksum mismatch! + Expected: bogus + Computed: 2ed19 + Suggested fix: remove all generated code including the end marker, + or use the '-f' option. + """) + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write(invalid_input) + # 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)) + # Then, force regeneration; success expected. + out = self.expect_success("-f", fn) + self.assertEqual(out, "") + # Verify by checking the checksum. + checksum = ( + "/*[clinic end generated code: " + "output=2124c291eb067d76 input=9543a8d2da235301]*/\n" + ) + with open(fn, 'r', encoding='utf-8') as f: + generated = f.read() + self.assertTrue(generated.endswith(checksum)) + def test_cli_help(self): out = self.expect_success("-h") self.assertIn("usage: clinic.py", out) From a3f505eb85e8f8fb8ee9313f3b9375322bb514ab Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 24 Jul 2023 00:36:35 +0200 Subject: [PATCH 06/10] Test verbose mode --- Lib/test/test_clinic.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 8d93e2cd76625b..2877c69c41b869 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1448,6 +1448,14 @@ def test_cli_force(self): generated = f.read() self.assertTrue(generated.endswith(checksum)) + def test_cli_verbose(self): + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write("") + out = self.expect_success("-v", fn) + self.assertEqual(out.strip(), fn) + def test_cli_help(self): out = self.expect_success("-h") self.assertIn("usage: clinic.py", out) From ff6a33ccae5d8fa8ef160f45257c07b1036eada8 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 24 Jul 2023 01:51:14 +0200 Subject: [PATCH 07/10] Add test cleanup to test_external --- Lib/test/test_clinic.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 2877c69c41b869..00a5dda209c5f7 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1380,6 +1380,7 @@ def test_external(self): orig_contents = f.read() # Run clinic CLI and verify that it does not complain. + self.addCleanup(unlink, TESTFN) out = self.expect_success("-f", "-o", TESTFN, source) self.assertEqual(out, "") From e97414c2fb3f7c7db71be75ad50e17d8998947e7 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 24 Jul 2023 10:12:53 +0200 Subject: [PATCH 08/10] Update Lib/test/test_clinic.py Co-authored-by: Nikita Sobolev --- Lib/test/test_clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 00a5dda209c5f7..46ebd68837fbd9 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1520,7 +1520,7 @@ def test_cli_converters(self): # param may change (it's a set, thus unordered). So, let's compare the # start and end of the expected output, and then assert we've got a # line for each converter. - self.assertTrue(out.startswith(prelude)) + self.assertTrue(out.startswith(prelude), out) for converter in expected_converters: with self.subTest(converter=converter): self.assertIn(converter, out) From e048663f94199fc61b9f2ce4e48f10a42bbb5cbd Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 24 Jul 2023 10:12:58 +0200 Subject: [PATCH 09/10] Update Lib/test/test_clinic.py Co-authored-by: Nikita Sobolev --- Lib/test/test_clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 46ebd68837fbd9..72b3c8da0908ac 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1524,7 +1524,7 @@ def test_cli_converters(self): for converter in expected_converters: with self.subTest(converter=converter): self.assertIn(converter, out) - self.assertTrue(out.endswith(finale)) + self.assertTrue(out.endswith(finale), out) try: From a4adb705d7d8e23d413a8f12f7066df558d10ca0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 24 Jul 2023 15:16:06 +0200 Subject: [PATCH 10/10] Improve accuracy of test_cli_converters --- Lib/test/test_clinic.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 72b3c8da0908ac..f5271203362263 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1518,14 +1518,22 @@ def test_cli_converters(self): out = self.expect_success("--converters") # We cannot simply compare the output, because the repr of the *accept* # param may change (it's a set, thus unordered). So, let's compare the - # start and end of the expected output, and then assert we've got a - # line for each converter. + # start and end of the expected output, and then assert that the + # converters appear lined up in alphabetical order. self.assertTrue(out.startswith(prelude), out) - for converter in expected_converters: - with self.subTest(converter=converter): - self.assertIn(converter, out) self.assertTrue(out.endswith(finale), out) + out = out.removeprefix(prelude) + out = out.removesuffix(finale) + lines = out.split("\n") + for converter, line in zip(expected_converters, lines): + line = line.lstrip() + with self.subTest(converter=converter): + self.assertTrue( + line.startswith(converter), + f"expected converter {converter!r}, got {line!r}" + ) + try: import _testclinic as ac_tester