pydrake: Add UpdateGlobalsFromModule test utility

RobotLocomotion · Dec 10, 2018 · d774035 · d774035
1 parent a2fc39e
commit d774035
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 16 deletions.
diff --git a/bindings/pydrake/BUILD.bazel b/bindings/pydrake/BUILD.bazel
@@ -90,6 +90,14 @@ drake_cc_library(
     visibility = ["//visibility:public"],
 )
 
+drake_cc_library(
+    name = "test_util_pybind",
+    testonly = 1,
+    hdrs = ["test/test_util_pybind.h"],
+    declare_installed_headers = 0,
+    visibility = ["//visibility:public"],
+)
+
 drake_cc_library(
     name = "autodiff_types_pybind",
     hdrs = ["autodiff_types_pybind.h"],

diff --git a/bindings/pydrake/common/BUILD.bazel b/bindings/pydrake/common/BUILD.bazel
@@ -340,7 +340,10 @@ drake_py_unittest(
 
 drake_pybind_cc_googletest(
     name = "cpp_param_pybind_test",
-    cc_deps = [":cpp_param_pybind"],
+    cc_deps = [
+        ":cpp_param_pybind",
+        "//bindings/pydrake:test_util_pybind",
+    ],
     py_deps = [":cpp_param_py"],
 )
 
@@ -355,6 +358,7 @@ drake_pybind_cc_googletest(
     name = "cpp_template_pybind_test",
     cc_deps = [
         ":cpp_template_pybind",
+        "//bindings/pydrake:test_util_pybind",
         "//common:nice_type_name",
         "//common/test_utilities:expect_throws_message",
     ],
@@ -365,6 +369,7 @@ drake_pybind_cc_googletest(
     name = "drake_variant_pybind_test",
     cc_deps = [
         ":drake_variant_pybind",
+        "//bindings/pydrake:test_util_pybind",
     ],
 )
 
@@ -391,6 +396,7 @@ drake_pybind_cc_googletest(
     name = "type_safe_index_pybind_test",
     cc_deps = [
         ":type_safe_index_pybind",
+        "//bindings/pydrake:test_util_pybind",
         "//common:nice_type_name",
     ],
 )

diff --git a/bindings/pydrake/common/test/cpp_param_pybind_test.cc b/bindings/pydrake/common/test/cpp_param_pybind_test.cc
@@ -11,6 +11,8 @@
 #include "pybind11/eval.h"
 #include "pybind11/pybind11.h"
 
+#include "drake/bindings/pydrake/test/test_util_pybind.h"
+
 using std::string;
 
 namespace drake {
@@ -86,8 +88,7 @@ int main(int argc, char** argv) {
   // Define custom class only once here.
   py::class_<CustomCppType>(m, "CustomCppType");
 
-  // For Python3
-  py::globals().attr("update")(m.attr("__dict__"));
+  test::UpdateGlobalsFromModule(m);
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }

diff --git a/bindings/pydrake/common/test/cpp_template_pybind_test.cc b/bindings/pydrake/common/test/cpp_template_pybind_test.cc
@@ -12,6 +12,7 @@
 #include "pybind11/eval.h"
 #include "pybind11/pybind11.h"
 
+#include "drake/bindings/pydrake/test/test_util_pybind.h"
 #include "drake/common/nice_type_name.h"
 #include "drake/common/test_utilities/expect_throws_message.h"
 
@@ -22,6 +23,8 @@ namespace drake {
 namespace pydrake {
 namespace {
 
+using test::UpdateGlobalsFromModule;
+
 template <typename... Ts>
 struct SimpleTemplate {
   vector<string> GetNames() {
@@ -45,12 +48,6 @@ void CheckValue(const string& expr, const T& expected) {
   EXPECT_EQ(py::eval(expr).cast<T>(), expected);
 }
 
-// TODO(eric.cousineau): Figure out why this is necessary.
-// Necessary for Python3.
-void sync(py::module m) {
-  py::globals().attr("update")(m.attr("__dict__"));
-}
-
 GTEST_TEST(CppTemplateTest, TemplateClass) {
   py::module m("__main__");
 
@@ -59,14 +56,14 @@ GTEST_TEST(CppTemplateTest, TemplateClass) {
 
   const vector<string> expected_1 = {"int"};
   const vector<string> expected_2 = {"int", "double"};
-  sync(m);
+  UpdateGlobalsFromModule(m);
 
   CheckValue("DefaultInst().GetNames()", expected_1);
   CheckValue("SimpleTemplate[int]().GetNames()", expected_1);
   CheckValue("SimpleTemplate[int, float]().GetNames()", expected_2);
 
   m.def("simple_func", [](const SimpleTemplate<int>&) {});
-  sync(m);
+  UpdateGlobalsFromModule(m);
 
   // Check error message if a function is called with the incorrect arguments.
   // N.B. We use `[^\0]` because C++ regex does not have an equivalent of
@@ -93,7 +90,7 @@ GTEST_TEST(CppTemplateTest, TemplateFunction) {
 
   const vector<string> expected_1 = {"int"};
   const vector<string> expected_2 = {"int", "double"};
-  sync(m);
+  UpdateGlobalsFromModule(m);
   CheckValue("SimpleFunction[int]()", expected_1);
   CheckValue("SimpleFunction[int, float]()", expected_2);
 }
@@ -120,7 +117,7 @@ GTEST_TEST(CppTemplateTest, TemplateMethod) {
 
   const vector<string> expected_1 = {"int"};
   const vector<string> expected_2 = {"int", "double"};
-  sync(m);
+  UpdateGlobalsFromModule(m);
   CheckValue("SimpleType().SimpleMethod[int]()", expected_1);
   CheckValue("SimpleType().SimpleMethod[int, float]()", expected_2);
 }

diff --git a/bindings/pydrake/common/test/drake_variant_pybind_test.cc b/bindings/pydrake/common/test/drake_variant_pybind_test.cc
@@ -10,13 +10,16 @@
 #include "pybind11/pybind11.h"
 
 #include "drake/bindings/pydrake/pydrake_pybind.h"
+#include "drake/bindings/pydrake/test/test_util_pybind.h"
 
 using std::string;
 
 namespace drake {
 namespace pydrake {
 namespace {
 
+using test::UpdateGlobalsFromModule;
+
 string VariantToString(const variant<int, double, string>& value) {
   const bool is_int = holds_alternative<int>(value);
   const bool is_double = holds_alternative<double>(value);
@@ -36,7 +39,7 @@ GTEST_TEST(VariantTest, CheckCasting) {
   py::module m("__main__");
 
   m.def("VariantToString", &VariantToString, py::arg("value"));
-  py::globals().attr("update")(m.attr("__dict__"));
+  UpdateGlobalsFromModule(m);
   ExpectString("VariantToString(1)", "int(1)");
   ExpectString("VariantToString(0.5)", "double(0.5)");
   ExpectString("VariantToString('foo')", "string(foo)");

diff --git a/bindings/pydrake/common/test/type_safe_index_pybind_test.cc b/bindings/pydrake/common/test/type_safe_index_pybind_test.cc
@@ -11,13 +11,17 @@
 #include "pybind11/eval.h"
 #include "pybind11/pybind11.h"
 
+#include "drake/bindings/pydrake/test/test_util_pybind.h"
+
 using std::string;
 using std::vector;
 
 namespace drake {
 namespace pydrake {
 namespace {
 
+using test::UpdateGlobalsFromModule;
+
 template <typename T>
 void CheckValue(const string& expr, const T& expected) {
   EXPECT_EQ(py::eval(expr).cast<T>(), expected);
@@ -34,7 +38,7 @@ GTEST_TEST(TypeSafeIndexTest, CheckCasting) {
     EXPECT_EQ(x, 10);
     return x;
   });
-  py::globals().attr("update")(m.attr("__dict__"));  // For Python3
+  UpdateGlobalsFromModule(m);
   CheckValue("pass_thru_int(10)", 10);
   CheckValue("pass_thru_int(Index(10))", 10);
   // TypeSafeIndex<> is not implicitly constructible from an int.
@@ -46,7 +50,7 @@ GTEST_TEST(TypeSafeIndexTest, CheckCasting) {
     return x;
   });
 
-  py::globals().attr("update")(m.attr("__dict__"));  // For Python3
+  UpdateGlobalsFromModule(m);
 
   // TypeSafeIndex<> is not implicitly constructible from an int.
   // TODO(eric.cousineau): Consider relaxing this to *only* accept `int`s, and

diff --git a/bindings/pydrake/test/test_util_pybind.h b/bindings/pydrake/test/test_util_pybind.h
@@ -0,0 +1,22 @@
+#pragma once
+
+#include "pybind11/pybind11.h"
+
+#include "drake/bindings/pydrake/pydrake_pybind.h"
+
+namespace drake {
+namespace pydrake {
+namespace test {
+
+// TODO(eric.cousineau): Figure out if there is a better solution than this
+// hack.
+/// pybind11's Python3 implementation seems to disconnect the `globals()` from
+/// a embedded interpreter's `__main__` module. To remedy this, we must
+/// manually synchronize these variables.
+inline void UpdateGlobalsFromModule(py::module m) {
+  py::globals().attr("update")(m.attr("__dict__"));
+}
+
+}  // namespace test
+}  // namespace pydrake
+}  // namespace drake