Boost logo

Boost :

Subject: Re: [boost] Call for Review: Boost.Test documentation rewrite
From: Mathieu Champlon (m.champlon_at_[hidden])
Date: 2014-01-06 03:57:51


As others have said, this is really a major and much needed improvement
over the existing documentation.
I especially like the little tricks here and there (heck I've been using
Boost.Test for years and I learned a few !) as well as the design
suggestions ("Testing Protected or Private Members") and the big picture
overviews ("Test Case Design and Maintenance").

Here are a few comments, feel free to discard some, most or all of them. :)

* source code links

When clicking on a source code link (any of the "Example source code" of
the tutorials section for instance) my browser (firefox) downloads the
file and attempts to open it using Visual Studio.
I would rather be able to display the source code as html for a quick
look instead.

* tutorials/testing_with_fixtures.html

The first example has a constructor and destructor for the fixture.
While it's explained a little further that the default ones are fine
here (and it should be obvious to a seasoned C++ developer), I fear that
beginners who skim through the documentation will simply copy-paste this
into their fixtures cluttering their code needlessly. Ideally having it
the other way around (start with a simple fixture without
constructor/destructor, then complicating things further) would be
better, but maybe just moving the "In most cases, the setup will be a
little more involved than just creating an instance variable and there
will be some work done in the constructor of the fixture. We could
simply use the default constructor and destructor of our fixture in this
case. We've written the constructor and destructor explicitly to
emphasize that this is where setup and tear down occur. " part into a
"Note" right after the code would prove enough ?

* tutorials/running_selected_tests.html

I find "We can list the available tests in a test executable with
--report_level set to detailed" a bit misleading as it doesn't just list
the tests but still primarily executes them.
Maybe rephrase it to something along the line of "Executed tests can be
listed with --report_level set to detailed when running a test executable" ?

Is there any reason why you didn't mention using
--run_test=hello_world_* first in order to filter on the test case names
instead of jumping into test suites ?
I personally rarely use test suites because I usually use a naming
convention like the one you introduced in a previous tutorial.

* guide/compilation/minimal_header.html

In the "Meaning" column I fear that for instance "the test fails and
test execution terminates" may leave a (perhaps non English native)
reader wondering whether it aborts the current test or the test
application ?

* guide/test_case_design.html

Maybe add a link to
to "(...), using test-driven development" ?

The Visual Studio configuration snapshot is a bit big in my opinion :)

Under "Goal: Repeatable Test" a bullet point list doesn't display properly.

* guide/testing_file_io.html

Just for the record I found writing tests with files to be one of the
recurring pitfalls where manipulating actual files does indeed help,
because when abstracting the file system things like re-entrance and
file locking can be easily remain untested.

Any reason for the "extern" when declaring text_files ?

I believe the static for defining ARBITRARY_DIRECTORY_NAME and all isn't
needed. Also the upper case names are usually reserved for macros (in

* guide/mocking_collaborators.html

Being the author of turtle I certainly appreciate the spotlight here,
but isn't this part a bit out of Boost.Test documentation scope ?

* guide/testing__main_.html

Again, any reason for the "extern" ? I'm just thinking it might confuse

* reference/test_case/boost_test_case_template.html

The fact that 'signed_types' is defined elsewhere is a bit odd
especially since this page comes first when browsing the reference pages
using the "next" button.
Also 'unsigned_types' doesn't show up anywhere as it's simply not included.
Maybe move both to the BOOST_TEST_CASE_TEMPLATE page or add them to both
relevant pages ?

* reference/test_suite/boost_test_suite.html

manual_registration.cpp starts with a "using namespace
boost::unit_test;" which doesn't appear in the documentation making it
difficult to understand where the functions come from.
Note that the same issue appears in
guide/manually_registering_test_cases_and_suites.html which includes the
same code.

* reference/assertion.html

There is a typo in the note e.g. "be defiend" instead of "be defined".

* reference/assertion/*

None of the pages provide links to the source code files, is it on purpose ?

* reference/assertion/_boost____level___close_.html and

Both have the exact same description and example code. An explanation on
how they differ would certainly prove useful as this is probably one of
the most asked question about Boost.Test on the mailing lists.
Also a note about when to use BOOST_level_SMALL instead would be nice.

* reference/assertion/boost_level_equal.html

In example_equal_custom_compare there's an extra space in the first
initialization which might be confusing.
Also I don't get how the comment is relevant given that operator== uses
std::string to make the comparison ?

* reference/assertion/_boost____level___equal_collections_.html

I believe "static" to be deprecated at a namespace level ?

I would personally rather use
         boost::begin( expected ), boost::end( expected ),
         boost::begin( actual ), boost::end( actual ));
which has the clear advantage to cover all the cases, but I understand
the need for more "raw" examples.

* reference/assertion/boost_level_exception.html,
reference/assertion/boost_level_message.html and

The "static" aren't really needed, I believe.

* reference/assertion/_boost____level___small_.html

Maybe explain the unit of the tolerance for completeness and coherence
with BOOST_level_CLOSE* ?

Thanks for doing this !


Boost list run by bdawes at, gregod at, cpdaniel at, john at