Issue #14769: test_capi now has SkipitemTest, which cleverly checks

for "parity" between PyArg_ParseTuple() and the Python/getargs.c static
function skipitem() for all possible "format units".

3 files changed, 134 insertions, 39 deletions

diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index a58c577..e24bd6f 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -214,9 +214,78 @@ class EmbeddingTest(unittest.TestCase):
         finally:
             os.chdir(oldcwd)
 
+class SkipitemTest(unittest.TestCase):
+
+    def test_skipitem(self):
+        """
+        If this test failed, you probably added a new "format unit"
+        in Python/getargs.c, but neglected to update our poor friend
+        skipitem() in the same file.  (If so, shame on you!)
+
+        This function brute-force tests all** ASCII characters (1 to 127
+        inclusive) as format units, checking to see that
+        PyArg_ParseTupleAndKeywords() return consistent errors both when
+        the unit is attempted to be used and when it is skipped.  If the
+        format unit doesn't exist, we'll get one of two specific error
+        messages (one for used, one for skipped); if it does exist we
+        *won't* get that error--we'll get either no error or some other
+        error.  If we get the "does not exist" error for one test and
+        not for the other, there's a mismatch, and the test fails.
+
+          ** Okay, it actually skips some ASCII characters.  Some characters
+             have special funny semantics, and it would be difficult to
+             accomodate them here.
+        """
+        empty_tuple = ()
+        tuple_1 = (0,)
+        dict_b = {'b':1}
+        keywords = ["a", "b"]
+
+        # Python C source files must be ASCII,
+        # therefore we'll never have a format unit > 127
+        for i in range(1, 128):
+            c = chr(i)
+
+            # skip non-printable characters, no one is insane enough to define
+            #    one as a format unit
+            # skip parentheses, the error reporting is inconsistent about them
+            # skip 'e', it's always a two-character code
+            # skip '|' and '$', they don't represent arguments anyway
+            if (not c.isprintable()) or (c in '()e|$'):
+                continue
+
+            # test the format unit when not skipped
+            format = c + "i"
+            try:
+                # (note: the format string must be bytes!)
+                _testcapi.parse_tuple_and_keywords(tuple_1, dict_b,
+                    format.encode("ascii"), keywords)
+                when_not_skipped = False
+            except TypeError as e:
+                s = "argument 1 must be impossible<bad format char>, not int"
+                when_not_skipped = (str(e) == s)
+            except RuntimeError as e:
+                when_not_skipped = False
+
+            # test the format unit when skipped
+            optional_format = "|" + format
+            try:
+                _testcapi.parse_tuple_and_keywords(empty_tuple, dict_b,
+                    optional_format.encode("ascii"), keywords)
+                when_skipped = False
+            except RuntimeError as e:
+                s = "impossible<bad format char>: '{}'".format(format)
+                when_skipped = (str(e) == s)
+
+            message = ("test_skipitem_parity: "
+                "detected mismatch between convertsimple and skipitem "
+                "for format unit '{}' ({}), not skipped {}, skipped {}".format(
+                    c, i, when_skipped, when_not_skipped))
+            self.assertIs(when_skipped, when_not_skipped, message)
 
 def test_main():
-    support.run_unittest(CAPITest, TestPendingCalls, Test6012, EmbeddingTest)
+    support.run_unittest(CAPITest, TestPendingCalls,
+                         Test6012, EmbeddingTest, SkipitemTest)
 
     for name in dir(_testcapi):
         if name.startswith('test_'):
diff --git a/Misc/NEWS b/Misc/NEWS
index 1149ecf..8abe981 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -165,6 +165,10 @@ Documentation
 Tests
 -----
 
+- Issue #14769: test_capi now has SkipitemTest, which cleverly checks
+  for "parity" between PyArg_ParseTuple() and the Python/getargs.c static
+  function skipitem() for all possible "format units".
+
 - test_nntplib now tolerates being run from behind NNTP gateways that add
   "X-Antivirus" headers to articles
 
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index bdc465a..ca526bd 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -1195,50 +1195,72 @@ test_s_code(PyObject *self)
 }
 
 static PyObject *
-test_bug_7414(PyObject *self)
+parse_tuple_and_keywords(PyObject *self, PyObject *args)
 {
-    /* Issue #7414: for PyArg_ParseTupleAndKeywords, 'C' code wasn't being
-       skipped properly in skipitem() */
-    int a = 0, b = 0, result;
-    char *kwlist[] = {"a", "b", NULL};
-    PyObject *tuple = NULL, *dict = NULL, *b_str;
+    PyObject *sub_args;
+    PyObject *sub_kwargs;
+    char *sub_format;
+    PyObject *sub_keywords;
 
-    tuple = PyTuple_New(0);
-    if (tuple == NULL)
-        goto failure;
-    dict = PyDict_New();
-    if (dict == NULL)
-        goto failure;
-    b_str = PyUnicode_FromString("b");
-    if (b_str == NULL)
-        goto failure;
-    result = PyDict_SetItemString(dict, "b", b_str);
-    Py_DECREF(b_str);
-    if (result < 0)
-        goto failure;
+    Py_ssize_t i, size;
+    char *keywords[8 + 1]; /* space for NULL at end */
+    PyObject *o;
+    PyObject *converted[8];
 
-    result = PyArg_ParseTupleAndKeywords(tuple, dict, "|CC",
-                                         kwlist, &a, &b);
-    if (!result)
-        goto failure;
+    int result;
+    PyObject *return_value = NULL;
 
-    if (a != 0)
-        return raiseTestError("test_bug_7414",
-            "C format code not skipped properly");
-    if (b != 'b')
-        return raiseTestError("test_bug_7414",
-            "C format code returned wrong value");
+    char buffers[32][8];
 
-    Py_DECREF(dict);
-    Py_DECREF(tuple);
-    Py_RETURN_NONE;
+    if (!PyArg_ParseTuple(args, "OOyO:parse_tuple_and_keywords",
+        &sub_args, &sub_kwargs,
+        &sub_format, &sub_keywords))
+        return NULL;
 
-  failure:
-    Py_XDECREF(dict);
-    Py_XDECREF(tuple);
-    return NULL;
-}
+    if (!(PyList_CheckExact(sub_keywords) || PyTuple_CheckExact(sub_keywords))) {
+        PyErr_SetString(PyExc_ValueError,
+            "parse_tuple_and_keywords: sub_keywords must be either list or tuple");
+        return NULL;
+    }
+
+    memset(buffers, 0, sizeof(buffers));
+    memset(converted, 0, sizeof(converted));
+    memset(keywords, 0, sizeof(keywords));
+
+    size = PySequence_Fast_GET_SIZE(sub_keywords);
+    if (size > 8) {
+        PyErr_SetString(PyExc_ValueError,
+            "parse_tuple_and_keywords: too many keywords in sub_keywords");
+        goto exit;
+    }
+
+    for (i = 0; i < size; i++) {
+        o = PySequence_Fast_GET_ITEM(sub_keywords, i);
+        if (!PyUnicode_FSConverter(o, (void *)(converted + i))) {
+            PyErr_Format(PyExc_ValueError,
+                "parse_tuple_and_keywords: could not convert keywords[%s] to narrow string", i);
+            goto exit;
+        }
+        keywords[i] = PyBytes_AS_STRING(converted[i]);
+    }
 
+    result = PyArg_ParseTupleAndKeywords(sub_args, sub_kwargs,
+        sub_format, keywords,
+        buffers + 0, buffers + 1, buffers + 2, buffers + 3,
+        buffers + 4, buffers + 5, buffers + 6, buffers + 7);
+
+    if (result) {
+        return_value = Py_None;
+        Py_INCREF(Py_None);
+    }
+
+exit:
+    size = sizeof(converted) / sizeof(converted[0]);
+    for (i = 0; i < size; i++) {
+        Py_XDECREF(converted[i]);
+    }
+    return return_value;
+}
 
 static volatile int x;
 
@@ -2426,7 +2448,7 @@ static PyMethodDef TestMethods[] = {
     {"test_long_numbits",       (PyCFunction)test_long_numbits,  METH_NOARGS},
     {"test_k_code",             (PyCFunction)test_k_code,        METH_NOARGS},
     {"test_empty_argparse", (PyCFunction)test_empty_argparse,METH_NOARGS},
-    {"test_bug_7414", (PyCFunction)test_bug_7414, METH_NOARGS},
+    {"parse_tuple_and_keywords", parse_tuple_and_keywords, METH_VARARGS},
     {"test_null_strings",       (PyCFunction)test_null_strings,  METH_NOARGS},
     {"test_string_from_format", (PyCFunction)test_string_from_format, METH_NOARGS},
     {"test_with_docstring", (PyCFunction)test_with_docstring, METH_NOARGS,