Make the runtime tests be invoked via an extension module (mypyc/mypyc#615)

msullivan · web-flow · commit 020b6b8427ee · 2019-05-15T12:31:00.000-07:00
Currently the runtime tests are built as a standalone binary linked against
libpython by using `python-config` to produce cflags and ldflags. This turns
out to be shockingly fragile and error-prone, since python-config location
and behavior doesn't really seem to be standardized. For example,
`python3-config`, which we default to using, doesn't exist in virtualenvs,
though a `python-config` does. So when run from a virtualenv, we actually
run the runtime tests against the system default Python 3, not the virtualenv's
Python. But we can't just switch to using `python-config` because that will
often point to Python 2.
Worse than all that, for reasons I have yet to fully understand I was never able
to get the python-config for dev versions on Travis to work. Using the python3-config
in the root python 3.8-dev install directory failed because it didn't include the library
path (??) and using the python-config in the virtualenv failed because it didn't include
`-lpython` (???) (or maybe vice versa).

Meanwhile building C extension modules inside of virtualenvs works pretty reliably,
so just switch to driving the runtime tests from a C extension module instead of from
a standalone binary. This means that the RT tests will run against the correct python
version on developer machines and we can turn on 3.8-dev tests in Travis.
diff --git a/.gitignore b/.gitignore
@@ -14,3 +14,5 @@ test_capi
 /.dmypy.json
 /.mypyc-flake8-cache.json
 /build
+/mypyc/lib-rt/build/
+/mypyc/lib-rt/*.so
diff --git a/external/googletest/make/Makefile b/external/googletest/make/Makefile
@@ -16,20 +16,13 @@
 # Remember to tweak this if you move this file.
 GTEST_DIR = ..
 
-# Where to find user code.
-USER_DIR = ../samples
-
 # Flags passed to the preprocessor.
 # Set Google Test's header directory as a system directory, such that
 # the compiler doesn't generate warnings in Google Test headers.
 CPPFLAGS += -isystem $(GTEST_DIR)/include
 
 # Flags passed to the C++ compiler.
-CXXFLAGS += -g -Wall -Wextra -pthread
-
-# All tests produced by this Makefile.  Remember to add new tests you
-# created to the list.
-TESTS = sample1_unittest
+CXXFLAGS += -g -Wall -Wextra -pthread -fPIC
 
 # All Google Test headers.  Usually you shouldn't change this
 # definition.
@@ -38,12 +31,12 @@ GTEST_HEADERS = $(GTEST_DIR)/include/gtest/*.h \
 
 # House-keeping build targets.
 
-all : $(TESTS)
+all : libgtest.a libgtest_main.a
 
 clean :
-	rm -f $(TESTS) gtest.a gtest_main.a *.o
+	rm -f libgtest.a libgtest_main.a *.o
 
-# Builds gtest.a and gtest_main.a.
+# Builds libgtest.a and libgtest_main.a.
 
 # Usually you shouldn't tweak such internal variables, indicated by a
 # trailing _.
@@ -61,22 +54,8 @@ gtest_main.o : $(GTEST_SRCS_)
 	$(CXX) $(CPPFLAGS) -I$(GTEST_DIR) $(CXXFLAGS) -c \
             $(GTEST_DIR)/src/gtest_main.cc
 
-gtest.a : gtest-all.o
+libgtest.a : gtest-all.o
 	$(AR) $(ARFLAGS) $@ $^
 
-gtest_main.a : gtest-all.o gtest_main.o
+libgtest_main.a : gtest-all.o gtest_main.o
 	$(AR) $(ARFLAGS) $@ $^
-
-# Builds a sample test.  A test should link with either gtest.a or
-# gtest_main.a, depending on whether it defines its own main()
-# function.
-
-sample1.o : $(USER_DIR)/sample1.cc $(USER_DIR)/sample1.h $(GTEST_HEADERS)
-	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $(USER_DIR)/sample1.cc
-
-sample1_unittest.o : $(USER_DIR)/sample1_unittest.cc \
-                     $(USER_DIR)/sample1.h $(GTEST_HEADERS)
-	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $(USER_DIR)/sample1_unittest.cc
-
-sample1_unittest : sample1.o sample1_unittest.o gtest_main.a
-	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -lpthread $^ -o $@
diff --git a/mypyc/lib-rt/CPy.cc b/mypyc/lib-rt/CPy.cc
@@ -0,0 +1 @@
+CPy.c
diff --git a/mypyc/lib-rt/Makefile b/mypyc/lib-rt/Makefile
diff --git a/mypyc/lib-rt/setup.py b/mypyc/lib-rt/setup.py
@@ -0,0 +1,13 @@
+from distutils.core import setup, Extension
+
+setup(name='test_capi',
+      version='0.1',
+      ext_modules=[Extension(
+          'test_capi',
+          ['test_capi.cc', 'CPy.cc'],
+          depends=['CPy.h', 'mypyc_util.h', 'pythonsupport.h'],
+          extra_compile_args=['--std=c++11', '-Wno-unused-function', '-Wno-sign-compare'],
+          library_dirs=['../../external/googletest/make'],
+          libraries=['gtest'],
+          include_dirs=['../../external/googletest', '../../external/googletest/include'],
+      )])
diff --git a/mypyc/lib-rt/test_capi.cc b/mypyc/lib-rt/test_capi.cc
@@ -4,7 +4,6 @@
 #include <Python.h>
 #include "CPy.h"
 
-static bool is_initialized = false;
 static PyObject *moduleDict;
 
 static PyObject *int_from_str(const char *str) {
@@ -84,21 +83,9 @@ class CAPITest : public ::testing::Test {
     Py_ssize_t c_max_neg_long;
 
     virtual void SetUp() {
-        if (!is_initialized) {
-            wchar_t *program = Py_DecodeLocale("test_capi", 0);
-            Py_SetProgramName(program);
-            Py_Initialize();
-            PyObject *module = PyModule_New("test");
-            if (module == 0) {
-                fail("Could not create module 'test'");
-            }
-            moduleDict = PyModule_GetDict(module);
-            if (module == 0) {
-                fail("Could not fine module dictionary");
-            }
-            is_initialized = true;
+        if (!moduleDict) {
+            fail("Could not find module dictionary");
         }
-        // TODO: Call Py_Finalize() and PyMem_RawFree(program) at the end somehow.
 
         max_short = int_from_str("4611686018427387903"); // 2**62-1
         min_pos_long = int_from_str("4611686018427387904"); // 2**62
@@ -540,9 +527,52 @@ TEST_F(CAPITest, test_tagged_as_long_long) {
     EXPECT_FALSE(PyErr_Occurred());
     EXPECT_TRUE(CPyTagged_AsSsize_t(l) == -1);
     EXPECT_TRUE(PyErr_Occurred());
+    PyErr_Clear();
+}
+
+
+////
+// Python module glue to drive the C-API tests.
+//
+// The reason we have this as an extension module instead of a
+// standalone binary is because building an extension module is pretty
+// well behaved (just specify it with distutils/setuptools and it will
+// get compiled and linked against the running python) while linking a
+// library against libpython is a huge non-standard
+// PITA: python-config locations are janky and it behaves in weird
+// ways that I don't understand, while this works very cleanly.
+
+static PyObject *run_tests(PyObject *dummy, PyObject *should_be_null) {
+    // Fake command line arguments. We could arrange to actually pass
+    // in command line arguments (either real ones or ones given as
+    // arguments) but have not bothered.
+    int argc = 1;
+    char asdf[] = "test_capi"; // InitGoogleTest wants char** which means it can't be const...
+    char *argv[] = {asdf, NULL};
+    ::testing::InitGoogleTest(&argc, argv);
+    return PyLong_FromLong(RUN_ALL_TESTS());
 }
 
-int main(int argc, char **argv) {
-    ::testing::InitGoogleTest(&argc, argv);
-    return RUN_ALL_TESTS();
+
+static PyMethodDef test_methods[] = {
+    {"run_tests",  run_tests, METH_NOARGS, "Run the C API tests"},
+    {NULL, NULL, 0, NULL}
+};
+
+static struct PyModuleDef test_module = {
+    PyModuleDef_HEAD_INIT,
+    "test_capi",
+    NULL,
+    -1,
+    test_methods
+};
+
+PyMODINIT_FUNC
+PyInit_test_capi(void)
+{
+    PyObject *module = PyModule_Create(&test_module);
+    if (module) {
+        moduleDict = PyModule_GetDict(module);
+    }
+    return module;
 }
diff --git a/mypyc/test/test_external.py b/mypyc/test/test_external.py
@@ -1,5 +1,7 @@
 """Test cases that run tests as subprocesses."""
 
+from typing import List
+
 import os
 import subprocess
 import sys
@@ -17,18 +19,27 @@ def test_c_unit_test(self) -> None:
         """Run C unit tests in a subprocess."""
         # Build Google Test, the C++ framework we use for testing C code.
         # The source code for Google Test is copied to this repository.
+        cppflags = []  # type: List[str]
+        env = os.environ.copy()
         if sys.platform == 'darwin':
-            env = {'CPPFLAGS': '-mmacosx-version-min=10.10'}
-        else:
-            env = os.environ.copy()
-        subprocess.check_call(['make', 'gtest_main.a'],
+            cppflags += ['-mmacosx-version-min=10.10', '-stdlib=libc++']
+        env['CPPFLAGS'] = ' '.join(cppflags)
+        subprocess.check_call(['make', 'libgtest.a'],
                               env=env,
                               cwd=os.path.join(base_dir, 'external', 'googletest', 'make'))
-        # Build and run C unit tests.
+        # Build Python wrapper for C unit tests.
+        env = os.environ.copy()
+        env['CPPFLAGS'] = ' '.join(cppflags)
+        status = subprocess.check_call(
+            [sys.executable, 'setup.py', 'build_ext', '--inplace'],
+            env=env,
+            cwd=os.path.join(base_dir, 'mypyc', 'lib-rt'))
+        # Run C unit tests.
         env = os.environ.copy()
         if 'GTEST_COLOR' not in os.environ:
             env['GTEST_COLOR'] = 'yes'  # Use fancy colors
-        status = subprocess.call(['make', 'test'],
+        status = subprocess.call([sys.executable, '-c',
+                                  'import sys, test_capi; sys.exit(test_capi.run_tests())'],
                                  env=env,
                                  cwd=os.path.join(base_dir, 'mypyc', 'lib-rt'))
         if status != 0:
