On 05/16/2012 09:23 PM, Stefan Teleman wrote:
On Wed, May 16, 2012 at 2:58 PM, Martin Sebor<mse...@gmail.com> wrote:
On 05/16/2012 11:55 AM, Travis Vitek wrote:
I approve the change, but with one caveat. The branching policy [1]
indicates that you should commit your changes directly to the 4.2.x branch.
They should be merged from 4.2.x to 4.3.x, and then from 4.3.x to trunk.
The test in the issue should be committed with the patch (after
adding the necessary asserts, being renamed to follow the naming
convention for regression tests, i.e., something like
regress/27.basic_ios.stdcxx-1058.cpp, and decorated with the ASF
license header).
I attached 27.ios_base.event_callback.stdcxx-1058.cpp to stdcxx-1058,
which will go in ${TOPDIR}/test/regress/.
Great! A few suggestions:
The naming convention used for the tests is based on a few things:
1. the section number in the standard of the tested functionality
(this is only used for sorting)
2. the name of class whose members are being tested (if any)
3. the name of the function or type (or member function or type)
being tested (may be omitted for tests that exercise multiple
members)
4. for regression tests, the issue number
Since this issue is about std::basic_ios::copyfmt() crashing (as
the subject says), its name should probably look something like:
27.basic_ios.copyfmt.stdcxx-1056.cpp
Each test should exercise just the affected class, function, or
type. It's best to avoid relying on parts of the library that
aren't affected or subject of the test so that when they break
as a result of a some unrelated change in the future we don't
start seeing failures in unrelated tests. In this case, I would
suggest to avoid using fstream (and especially cerr) and rely
directly on basic_ios (define a derived class to access its
protected ctor). Most regression tests don't tend to produce
debugging output but if you find it useful please use stdio,
not iostreams.
Finally, regression tests should verify expected postconditions
by using the assert() macro. The exit status can be 0 on success
and non-zero on failure, but this is not done consistently.
Martin
--Stefan
