Adding or removing tests is a little bit cumbersome currently: the test
needs to be added to CMakeLists.txt, the init function needs to be
predeclared in pybind11_tests.cpp, then called in the plugin
initialization. While this isn't a big deal for tests that are being
committed, it's more of a hassle when working on some new feature or
test code for which I temporarily only care about building and linking
the test being worked on rather than the entire test suite.
This commit changes tests to self-register their initialization by
having each test initialize a local object (which stores the
initialization function in a static variable). This makes changing the
set of tests being build easy: one only needs to add or comment out
test names in tests/CMakeLists.txt.
A couple other minor changes that go along with this:
- test_eigen.cpp is now included in the test list, then removed if eigen
isn't available. This lets you disable the eigen tests by commenting
it out, just like all the other tests, but keeps the build working
without eigen eigen isn't available. (Also, if it's commented out, we
don't even bother looking for and reporting the building with/without
eigen status message).
- pytest is now invoked with all the built test names (with .cpp changed
to .py) so that it doesn't try to run tests that weren't built.
This makes the output considerably easier to use: it now highlights (in
red) matched tabs (instead of just listing the filenames), and adds
line numbers to both the tabs check and the space-less if check outputs.
Problem
=======
The template trampoline pattern documented in PR #322 has a problem with
virtual method overloads in intermediate classes in the inheritance
chain between the trampoline class and the base class.
For example, consider the following inheritance structure, where `B` is
the actual class, `PyB<B>` is the trampoline class, and `PyA<B>` is an
intermediate class adding A's methods into the trampoline:
PyB<B> -> PyA<B> -> B -> A
Suppose PyA<B> has a method `some_method()` with a PYBIND11_OVERLOAD in
it to overload the virtual `A::some_method()`. If a Python class `C` is
defined that inherits from the pybind11-registered `B` and tries to
provide an overriding `some_method()`, the PYBIND11_OVERLOADs declared
in PyA<B> fails to find this overloaded method, and thus never invoke it
(or, if pure virtual and not overridden in PyB<B>, raises an exception).
This happens because the base (internal) `PYBIND11_OVERLOAD_INT` macro
simply calls `get_overload(this, name)`; `get_overload()` then uses the
inferred type of `this` to do a type lookup in `registered_types_cpp`.
This is where it fails: `this` will be a `PyA<B> *`, but `PyA<B>` is
neither the base type (`B`) nor the trampoline type (`PyB<B>`). As a
result, the overload fails and we get a failed overload lookup.
The fix
=======
The fix is relatively simple: we can cast `this` passed to
`get_overload()` to a `const B *`, which lets get_overload look up the
correct class. Since trampoline classes should be derived from `B`
classes anyway, this cast should be perfectly safe.
This does require adding the class name as an argument to the
PYBIND11_OVERLOAD_INT macro, but leaves the public macro signatures
unchanged.
The check-style exit status wasn't being propagated properly because
the loops were running in a subshell (and so the change the the
`errors` variable wasn't in the active command shell). This fixes it
by running the greps in subshells and the loops in the main shell.
This also avoids the if(/for(/while( style check on
tests/CMakeLists.txt, since it *does* have if() statements with no space
that are producing error messages, but that is (acceptable) CMake style.
This adds a tool that checks style (currently just for tabs instead of
spaces in files under include/tests/docs) and produces a travis-ci build
failure if any problems are found.
Fixes#365. `sysconfig.get_config_var('SO')` already returns the correct
PYTHON_MODULE_EXTENSION, even for debug builds, so there is no need to
add anything else manually.
Installing something outside the project directory from a cmake
invocation is overly intrusive; this changes tests/CMakeLists.txt to
just fail with an informative message instead, and changes the
travis-ci builds to install pytest via pip or apt-get.
ccache on Travis was never configured properly so the setting never
actually did anything. Enabling ccache for real brings other issues:
due to the way the preprocessor is handled, some of the Python header
macros produce bogus compiler warnings (which in turn produce errors
with -Werror). ccache also requires additional configuration on OS X
and docker. It would reduce compile time by ~30 seconds at best, so
it's not worth the trouble.
[skip appveyor]
This build makes sure everything still works without optional
dependencies (numpy/scipy/eigen) and also tests the automatic
discovery functions in CMake (Python version, C++ standard).
[skip appveyor]
- ICPC can't handle the NCVirt trampoline which returns a non-copyable
type, which is likely due to a constexpr/SFINAE issue. This disables
the test on that compiler so that at least the rest can be tested.
pytest can capture test output both globally (controlled by the cmd line
flag --capture) or locally (`capsys` and `capfd` fixtures). Enabling both
methods at the same time causes problems on Windows: test output is not
captured sometimes, resulting in test failure. This happens seemingly at
random.
This workaround disables global output capture ("-s", i.e. "--capture=no")
leaving only the local capture fixtures. As a side-effect test output on
AppVeyor CI is a little messy, but this will have to do until a better
solution is found.
Jason Rhinelander
Jason Newton
Wenzel Jakob
Nickolai Belakovski
Dean Moldovan
Ivan Smirnov