[Please do not mail me a copy of your followup]
Thank you Mathieu for your detailed feedback.
Mathieu Champlon
* 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.
I think when the code gets deployed to the web site, you end up seeing a browser-ified version of the source code. For instance, the existing links to source files appear this way on the web site, but I cannot find any indiciation that the quickbook source for other libraries, or the existing non-quicbook documentation for boost.test, does anything special to provide this.
* 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 ?
So if I might paraphrase -- you're saying to display best practices first and make a note of where the setup/tear down code would appear?
* 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.
Yes, the trunk version of boost.test has a specific option for listing the tests called --list_content and not executing them. I'm not a fan of the name of this argument, but it is present on trunk and not on release. When I originally wrote this documentation it was based on trunk but I rebased it on release in order to get it moving forward.
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" ?
I'll reword this to say that the tests are executed as well as test names shown.
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 was using this as a way to introduce test suites :-).
I personally rarely use test suites because I usually use a naming convention like the one you introduced in a previous tutorial.
If you prefer that style of test organization, was the explanation in this tutorial good enough for you to see how to use that style?
* 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 ?
I can reword this for more clarification.
* guide/test_case_design.html
Maybe add a link to http://en.wikipedia.org/wiki/Test-driven_development to "(...), using test-driven development" ?
Good idea. More linking never hurts.
The Visual Studio configuration snapshot is a bit big in my opinion :)
It is native size. Were you thinking that providing a smaller, filtered version of the screen shot with the image being a link to the native size version?
Under "Goal: Repeatable Test" a bullet point list doesn't display properly.
Ah. Good catch.
* 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.
This is why I talk about acceptance testing being an integral part of any plan for automated testing. Unit tests alone are not enough because they exercise pieces of the system in isolation; you still need some sort of automated tests that bring all the pieces together and exercise them as a whole.
Any reason for the "extern" when declaring text_files ?
It is a function with external linkage. I don't want to declare it as a local function, but one that is provided externally. Do you feel that this is a leftover "C"-ism?
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 Boost).
That may simply be a matter of style -- I picked this up from "Modern C++ Programming with Test-Driven Development" where he advises that using named constants for arbitrary inputs helps make it clear that the actual value used in the test isn't important so much as how that value appears in inputs and expected outputs. Regarding the casing of the identifier, I was falling back on constants being all caps, but I'll consult the boost style guidelines and adjust per their recommendations.
* 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 ?
It is, but mocking objects is too important to be left unmentioned. Since Turtle is designed to work directly with Boost.Test I wanted to give it a mention.
* guide/testing__main_.html
Again, any reason for the "extern" ? I'm just thinking it might confuse beginners.
Same as above.
* 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 ?
OK, I'll expand this example to be more complete when viewed standalone.
* 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.
OK. I'll revise the examples to avoid using namespaces in order to make it more obvious where functions originate.
* reference/assertion.html
There is a typo in the note e.g. "be defiend" instead of "be defined".
Good catch. I will run a spell checker on everything to make sure I find any other typos like this.
* reference/assertion/*
None of the pages provide links to the source code files, is it on purpose ?
I thought the example snippets as they stood would be sufficient. I can certainly link to the example source file.
* reference/assertion/_boost____level___close_.html and reference/assertion/_boost____level___close_fraction_.html
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.
The floating-point comparison is the one area where I still need to write something real instead of just a placeholder. This is the last area that I have yet to finish.
* reference/assertion/boost_level_equal.html
In example_equal_custom_compare there's an extra space in the first initialization which might be confusing.
Which space are you referring to? The space between the definition of the string s and the first assertion?
Also I don't get how the comment is relevant given that operator== uses std::string to make the comparison ?
The standard library provides operator== and operator<< for std::string, so BOOST_REQUIRE_EQUAL can be used without requiring you to define these. For your own custom type you'll have to define them.
* reference/assertion/_boost____level___equal_collections_.html
I believe "static" to be deprecated at a namespace level ?
Static linkage to a function means that it is only visible within that single compilation unit. However, since generate_list is the system under test, this may be a little confusing. I could separate it into another compilation unit and give it a head, but then the whole example becomes much more complicated being spread across three source files, a new static library and an executable. For the purposes of making a small illustrative example I felt that was overkill.
I would personally rather use BOOST_REQUIRE_EQUAL_COLLECTIONS( 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.
There is a feature request from 4 years ago(!) for exactly this: https://svn.boost.org/trac/boost/ticket/3871 I recently tried to read the Boost.Range documentation and I confess that I didn't exaclty find it accessible at a quick read. In this documentation I wanted it to be as standalone as possible and not drag the reader unnecessarily into other libraries. In this case I think that the vast majority of C++ programmers are not going to have a problem with container.begin() or container.end() whereas many may not have seen boost::begin(container) and boost::end(container). Is the motivation for this suggestion to guide C++ programmers more towards C++11 style usage where std::begin() and std::end() are available? I can see how their use for fixed length arrays is preferable over the old "sizeof(array)/sizeof(array[0])" business, but how exactly is it preferable for containers where we have begin() and end() member functions?
* reference/assertion/boost_level_exception.html, reference/assertion/boost_level_message.html and reference/assertion/_boost____level___predicate_.html
The "static" aren't really needed, I believe.
See above. They are given static linkage to intentionally limit the accessibility of their names.
* reference/assertion/_boost____level___small_.html
Maybe explain the unit of the tolerance for completeness and coherence with BOOST_level_CLOSE* ?
This is part of the remaining area that needs to be done. -- "The Direct3D Graphics Pipeline" free book http://tinyurl.com/d3d-pipeline The Computer Graphics Museum http://computergraphicsmuseum.org The Terminals Wiki http://terminals.classiccmp.org Legalize Adulthood! (my blog) http://legalizeadulthood.wordpress.com