Test suite cleanup and reorganisation

[dean0x7d]

The first part of this PR closes #861: the test_issues.cpp/.py files are removed completely and the tests are redistributed into more appropriate files.

Part two breaks up test_python_types.cpp because it has also started to accumulate various not-so-related parts. The tests are redistributed into: test_builtin_casters for casters which are available in pybind11 without extra headers, test_stl for stl.h and test_pytypes for Python type wrappers and their C++ interfaces. There were also a couple of py::class_ tests which were moved into test_class (renamed from test_class_args).

Along the way, this also adds a few things to the main pybind11_tests.h header:

• UnregisteredType which is used to test conversion errors. A few test files were already using various versions of this.
• UserType which serves as the official user-defined type for testing (simple int holder). Multiple tests have their own versions of this (or use some locally exported class) but a lot of that is unnecessary duplication (I haven't gone through to replace everything yet).
• IncType: adapted from @jagerman's reference_wrapper tests. It's like UserType, but increments value on copy for quick reference vs. copy tests. Should be useful for quick tests without the need for full ConstructorStats instrumentation.
• TEST_SUBMODULE is a convenience macro which wraps test_initializer and defines a new submodule (instead of adding to the main pybind11_tests module). It also defines a function instead of a lambda because MSVC has issues with lambdas and local structs.

Proposal for new test conventions:

• Each .cpp file should create its own submodule. (Made easier by TEST_SUBMODULE).
• Each .py files imports that module as from pybind11_tests import submodule as m. This should reduce the number of import statements and the m name reflects the convention from the .cpp files.
• If a .py files has a function called test_name() then the .cpp file should add a corresponding // test_name comment above the bindings. This should help navigation between files.
• Where possible, it would be preferable to define local classes (at function scope) right above their py::class_ bindings and related lambda def bindings. As long as the classes are single-purpose for a specific test, this should help with organization. MSVC would frequently choke on this which made it impractical, but the TEST_SUBMODULE macro resolves that.

Thoughts?

Right now this is only applied to the new test files which have come out of test_python_types. If this seems good, it could be applied everywhere. There are also some other frequently reinvented classes which can be pulled out (seems like a move-only type is needed frequently for testing, etc.).

Closes #842, closes #861.

[jagerman]

This all sounds pretty good to me. Breaking up of test_python_types was definitely the next logical step after dispersing test_issues.

I'm more lukewarm on coalescing common types. I worry a bit that this is going to run into conflicts if pushed too far because future tests are going to want to change something something which in turn breaks other tests.

As for the m. everywhere, I have to say I prefer the from m import x, y, z rather than having dozens of m.x(), m.y(), etc., particular when functions are often repeated (for example, in test_single_char_arguments()). I think this should be on more of a per-test basis: sometimes importing is going to be nicer, sometimes just using m.whatever() will be nicer (in much the same way that a localized using namespace detail; can make some code cleaner).

As for moving into submodules, I think that's a great idea: I used to run into cases where cmake -DPYBIND11_TEST_OVERRIDE=whatever didn't work properly because a test relied on something set in one of the other tests. I think most of those are gone now, but isolating things to separate modules will make sure of that.

[dean0x7d]

I'm more lukewarm on coalescing common types. I worry a bit that this is going to run into conflicts if pushed too far because future tests are going to want to change something which in turn breaks other tests.

The intention here isn't to consolidate everything -- only the most common types in order to remove needless duplication. If a test needs something very specific then it's fine to introduce a new class only in its translation unit. But if a test just needs to check builtin vs. user-defined types then it doesn't make sense to create yet another int-holder.

As for the m. everywhere, I have to say I prefer the from m import x, y, z rather than having dozens of m.x(), m.y(), etc., particular when functions are often repeated (for example, in test_single_char_arguments()). I think this should be on more of a per-test basis: sometimes importing is going to be nicer, sometimes just using m.whatever() will be nicer (in much the same way that a localized using namespace detail; can make some code cleaner).

Sure, that makes sense. If a test uses a function very frequently, from m import x is definitely nicer. The m. convention shouldn't be a strict rule, just a good default because the majority of the test functions are used exactly once, so this saves on very long from m import (a, b, ..., y, z, ...) statements.

[jagerman]

I suggest we merge this one now (after also moving the new, conflicting tests added to test_python_types), and deal with transitioning the others one by one. Otherwise I think it's going to be painful keeping up with the resulting merge conflicts.

[dean0x7d]

I suggest we merge this one now (after also moving the new, conflicting tests added to test_python_types), and deal with transitioning the others one by one. Otherwise I think it's going to be painful keeping up with the resulting merge conflicts.

Agreed. I've rebased and resolved the conflicts with the new tests.

I've also included a few smaller changes:

• Consolidated a few shorter test files:

  Moved test_alias_initialization into test_virtual_functions -- test alias_init tests are very short and logically part of virtual_functions.

  Moved test_inheritance into test_class -- again, short tests, but they were actually checking more than just inheritance. Besides, single-inheritance is simple enough to be considered basic functionality of class_. The multiple inheritance tests are much more involved and therefore have a separate file.

• Moved the CMake code related to test_cmake_build into its own subdirectory -- makes the main pytest CMake code cleaner.

• Since test_issues.cpp is going away, I've cherry picked the new test from Add tests for pointer defaults. #842.

If there are no objections I'd like to merge this soon to avoid more conflicts with PRs. Porting the other tests to submodules can be done gradually later.

Note: I have accidentally pushed the latest changes to both my fork and origin. I've deleted the branch on origin, but Travis and AppVeyor have already registered it and now show both the PR tests and branch tests. The latter will fail because the branch has been deleted.