Changes between Version 5 and Version 6 of BestPracticeHandbook

Timestamp:

May 4, 2015, 2:25:58 PM (7 years ago)

Author:

Niall Douglas

Comment:

--

BestPracticeHandbook

@@ -18 +18 @@
 
 There was very significant divergence in best practices between these libraries at the time of examination, and moreover the divergences were uneven possibly reflecting an uneven understanding of what best practice might be in C++ 11/14. Thence from my examination I have prepared the list below of what I felt were best practices in C++ 11/14 mandatory libraries with hyperlinks to the relevant source code files in the relevant libraries above. As what a "best practice" is or is not may vary according to opinion, I have attempted to provide a rationale for each of the suggestions below. It is up to library authors to weigh each suggestion and to decide whether to apply it, or not, to their library.
+
+One of the most strikingly consistent features of these new libraries is the lack of dependence on Boost, even to the extent of not using the boost namespace as well as no Boost at all. The big advantage of this is modularity, so almost all of these libraries can be used standalone which is an oft requested feature by Boost end users. However this is likely not sustainable as more C++ 11/14 libraries become useful to other C++ 11/14 libraries and coupling therefore increases. I therefore dedicate significant effort below into how best to couple your library to other libraries which has very significantly improved with new C++ 11 features.
 
 One will note that the list below is much more C++ 11/14 focused than Boost focused. This is because it is derived from the first crop of C++ 11/14 mandatory Boost libraries. This is not a handbook for writing Boost libraries or even C++ 11/14 Boost libraries, if you want that [http://www.boost.org/development/requirements.html first start reading here] (note some of the guidelines on that page don't really apply to C++ 11/14 libraries) and [http://rrsd.com/blincubator.com/requirements/ then read here] and [http://rrsd.com/blincubator.com/advice/ here].
@@ -84 +86 @@
 
 
-== 2. Very strongly consider versioning your library's namespace using inline namespaces and requesting users to alias a versioned namespace instead of using it directly ==
+== 2. Strongly consider versioning your library's namespace using inline namespaces and requesting users to alias a versioned namespace instead of using it directly ==
 
 C++ 11 adds a new feature called inline namespaces which are far more powerful than they first appear:
@@ -181 +183 @@
 What are the problems with this technique?
 
-1. You now need to ship multiple copies of your library, maintain multiple copies of your library, and make sure simultaneous use of multiple library versions in the same executable doesn't conflict. I suspect this cost is worth it for the added flexibility for most library maintainers.
+1. You now need to ship multiple copies of your library, maintain multiple copies of your library, and make sure simultaneous use of multiple library versions in the same executable doesn't conflict. I suspect this cost is worth it for the added flexibility to evolve breaking changes for most library maintainers.
 2. The above technique alone is insufficient for header only end users where multiple versions of your library must coexist within the same translation unit. With some additional extra work, it is possible to allow multiple header only library versions to also coexist in the same translation unit, but this is covered in a separate recommendation below.
 3. Many end users are not used to locally aliasing a library namespace in order to use it, and may continue to directly qualify it using the 03 idiom. You may consider defaulting to not using an inline namespace for the version to make sure users don't end up doing this in ways which hurt themselves, but that approach has both pros and cons.
@@ -211 +213 @@
 == 4. Consider making it possible to use an XML outputting unit testing framework, even if not enabled by default ==
 
-A very noticeable trend in C++ 11/14 mandatory Boost libraries is that less than half are able to use Boost.Test, and use alternative unit testing systems.
-
-There are many very good reasons not to use Boost.Test, but there are few good reasons to not be able to use Boost.Test at all.
-
-TODO IN PROGRESS
+A very noticeable trend in the libraries above is that around half use good old C assert() and static_assert() instead of a unit testing framework.
+
+There are many very good reasons not to use a unit testing framework by default, but there are few good reasons to not be able to use a unit testing framework at all. A big problem for the release managers when your library cannot output XML indicating exactly which tests pass and which fail (including the static ones) is that all they get instead is failure to compile or failure to execute. This forces them to dig into compiler error diagnostics and unit test diagnostics respectively. It also makes what may be a very minor problem easily delegated appear as serious as the most serious possible problem because there is no way to quickly disambiguate without diving into potentially a debugger, so all these are good reasons to support some XML outputting unit testing framework which reports an XML entry one per test for each test case in every test suite in your library.
+
+Let me give you an example with Boost.AFIO which executes about a hundred thousand tests for about 70 test platforms and configurations per commit. I once committed a change and noticed in the test matrix that only statically linked libraries were failing. The cause was immediately obvious to me: I had leaked ABI in a way that the unit tests which deliberately build mismatching versions of AFIO to ensure namespace version changes don't conflict had tripped, and without even having to investigate the error itself I knew to revisit my commit for ABI leakage. For someone less familiar with the library, a quick look into the failing test would have revealed the name of the failing test case and instantly realise it was an ABI leakage problem. This sort of extra information is a big win for anyone trying to get a release out the door.
+
+There are big advantages for unit test stability analysis tooling as well. Jenkins CI can record the unit tests for thousands of builds, and if you have a test that regularly but rarely fails then Jenkins can flag such unstable tests. Atlassian tooling free for open source can display unit test aggregate statistics on a dashboard, and free web service tooling able to do ever more sophisticated statistical analysis which you once had to pay for is becoming ever more common.
+
+Finally, specifically for Boost libraries we have an [http://www.boost.org/development/tests/master/developer/summary.html automated regression testing] system which works by various end users uploading XML results generated by Boost.Test to an FTP site where a cron script regularly runs to generate static HTML tables of passing and failing tests. Needless to say, if your library was as useful as possible to that system everybody wins, and your library is not as useful to that system if it uses assert() and even static_assert() because the XML uploaded is a compiler error console log or an assert failure diagnostic instead of a detailed list of which tests passed and which failed.
+
+Hopefully by now I have persuaded you to use an XML outputting unit test framework. If you are a Boost library, the obvious choice is to use Boost.Test. Despite its many problems, being an arse to develop against and lack of maintenance, Boost.Test is still a very competitive choice, and if you ignore the overly dense documentation and simply lift the pattern from this quick sample you'll be up and running very quickly:
+
+{{{#!C++
+#include "boost/test/unit_test.hpp"  // Note the lack of angle brackets
+
+BOOST_AUTO_TEST_SUITE(all)  // Can actually be any name you like
+
+BOOST_AUTO_TEST_CASE(works/spinlock, "Tests that the spinlock works as intended")  // Note the forward slashes in the test name
+{
+  boost::spinlock::spinlock<bool> lock;
+  BOOST_REQUIRE(lock.try_lock());
+  BOOST_REQUIRE(!lock.try_lock());
+  lock.unlock();
+  
+  std::lock_guard<decltype(lock)> h(lock);
+  BOOST_REQUIRE(!lock.try_lock());
+}
+// More BOOST_AUTO_TEST_CASE(), as many as is wanted
+
+BOOST_AUTO_TEST_SUITE_END()
+}}}
+
+Already those familiar with Boost.Test will notice some unusual choices, but I'll come back to why shortly. For reference there are additional common tests in addition to BOOST_REQUIRE:
+
+ BOOST_CHECK(expr):: Check if expr is true, continuing the test case anyway if false.
+ BOOST_CHECK_THROWS(expr):: Check if expr throws an exception, continuing the test case anyway if false.
+ BOOST_CHECK_THROW(expr, type):: Check if expr throws an exception of a specific type, continuing the test case anyway if false.
+ BOOST_CHECK_NO_THROW(expr):: Check if expr does not throw an exception, continuing the test case anyway if false.
+
+ BOOST_REQUIRE(expr):: Check if expr is true, immediately exiting the test case if false.
+ BOOST_REQUIRE_THROWS(expr):: Check if expr throws an exception, immediately exiting the test case if false.
+ BOOST_REQUIRE_THROW(expr, type):: Check if expr throws an exception of a specific type, immediately exiting the test case if false.
+ BOOST_REQUIRE_NO_THROW(expr):: Check if expr does not throw an exception, immediately exiting the test case if false.
+
+ BOOST_TEST_MESSAGE(msg):: Log a message with the XML output.
+ BOOST_CHECK_MESSAGE(pred, msg):: If pred is false, log a message with the XML output.
+ BOOST_WARN_MESSAGE(pred, msg):: If pred is false, log a warning message with the XML output.
+ BOOST_FAIL(msg):: Immediately exit this test case with a message.
+
+Boost.Test provides an enormous amount of extra stuff (especially in its unstable branch), but for 99% of C++ code the above is all you will ever need. There is also a very specific reason I chose this exact subset of Boost.Test's functionality to suggest using here, because [https://github.com/ned14/Boost.APIBind/blob/master/include/boost/test/unit_test.hpp Boost.APIBind]'s lightweight header only Boost.Test emulation defines just the above subset and usefully does so into a header inside APIBind called "boost/test/unit_test.hpp", so if you include just that header you get compatibility with APIBind and Boost.Test. In other words, by using the pattern just suggested you can:
+
+1. With a macro switch turn on full fat Boost.Test.
+2. For the default use Boost.APIBind's thin wrap of [https://github.com/philsquared/Catch the CATCH header only unit testing library] which I have forked with added thread safety support. CATCH is very convenient to develop against, provides pretty coloured console unit test output and useful diagnostics, and on request on the command line can also output JUnit format XML ready for consumption by almost every unit test XML consuming tool out there. Boost.Test theoretically can be used header only, but you'll find it's very slow, whereas CATCH is always header only and has a minimal effect on compile time. CATCH also comes as a single kitchen sink header file, and APIBind includes a copy for you.
+3. For those so motivated that they really want assert() and nothing more, simply wrap the above macros with calls to assert(). Your single unit test code base can now target up to three separate ways of reporting unit test fails.
+
+Note if CATCH doesn't have enough features and Boost.Test is too flaky, another popular choice is [https://code.google.com/p/googletest/ Google Test].
+
+What are the problems with replacing asserts with a unit test framework?
+
+1. Asserts are fast and don't synchronise threads. Unit test frameworks almost always must grab a mutex for every single check, even if that check passes, which can profoundly damage the effectiveness of your testing. The obvious workaround is to prepend an if statement of the test before every check, so {{{ if(!expr) BOOST_CHECK(expr); }}} but be aware now only failures will be output into XML, and many CI parsers will consider zero XML test results in a test case to be a fatal error (workaround: always do a BOOST_CHECK(true) at the very end of the test).
+
+2. Getting static_asserts to decay cleanly into a BOOST_CHECK without #ifdef-ing is not always obvious. The obvious beginning is:
+
+ {{{#!c++
+ #ifndef AM_USING_BOOST_TEST_FOR_STATIC_ASSERTS
+ #define BOOST_CHECK_MESSAGE(pred, msg) static_assert(pred, msg)
+ #endif
+ }}}
+ ... and now use BOOST_CHECK_MESSAGE instead of static_assert directly. If your static assert is inside library implementation code, consider a macro which the unit tests override when being built with a unit test framework, but otherwise defaults to static_assert.
+
+3. Asserts have no effect when NDEBUG is defined. Your test code may assume this for optimised builds, and a simple regex find and replace may not be sufficient.
 
 == 5. Consider not doing compiler feature detection yourself ==
+
+TODO