[vtk-developers] Use of std::cout in tests

Thu Sep 20 11:57:55 EDT 2012

Why print the output in that case? It would still segfault when just
calling the function (or leak) wouldn't it? It would help me (and I am
sure others) to enumerate what we should be aiming to incorporate in
our tests, and the various ways in which we want tests to exercise,
verify and/or stress test code. You seem frustrated talking about
process, conventions etc, but without it I am not sure I can take part
and help improve the tests beyond what I have already done.

I have always focused on verification of expected output, and tried to
test error conditions. Should I write tests that will hit asserts and
have expected fails? Are there ways in which tests could be written to
improve valgrind, is calling a function without verifying its output
reasonable to then trigger valgrind reports? I would like to move to
have more unit tests that deliberately exercise individual classes,
but in that case having something like GTest would be very helpful.

The charts would also have vastly higher coverage if it were easier to
record interactive tests, I will see about doing that - a large amount
of untested code is interaction or interaction related code.

Marcus

On Thu, Sep 20, 2012 at 11:41 AM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
> But, it could segfault for have valgrind defects...  It is a form of testing
> to check for memory access and leaks.
>
>
> On Thu, Sep 20, 2012 at 11:13 AM, Marcus D. Hanwell
> <marcus.hanwell at kitware.com> wrote:
>>
>> This sounds reasonable to me, I would like to point out that being
>> able to selectively pull symbols into the current namespace is a
>> feature of namespaces, using std::cout to bring in a symbol you will
>> use a lot is pretty common.
>>
>> We should be careful to use the new C++ headers consistently, and if
>> we are going to relax this language then I still think that changing
>> all occurrences to your preferred style should be discouraged in
>> existing code (this will add noise to changes, and confuse functional
>> changes over style changes), instead preferring the style currently
>> used in that file.
>>
>> Even if the style is not applied everywhere we should still have an up
>> to date style guide, and aim to stick with it automated tools or not.
>> As a new contributor to a project that is one of the things I look
>> for, or simply try to stick to the existing style in a given source
>> file.
>>
>> My major objection to the patch that started all this was that whilst
>> it increased coverage it did not actually verify any of the results
>> and would always pass whether the output was correct or not (assuming
>> it didn't segfault). I want to increase coverage, but would like to
>> ensure that the tests will fail if someone breaks the code, and that
>> is also why I am not sure I see the value in outputting lots of data
>> (it appears I misunderstood duplicate CDash data, and that is far less
>> of an issue than I thought it was).
>>
>> Marcus
>>
>> On Wed, Sep 19, 2012 at 5:44 PM, David Gobbi <david.gobbi at gmail.com>
>> wrote:
>> > The only reason that VTK didn't use std:: historically was so that it
>> > could support pre-standard or broken compilers.  The last badly broken
>> > compiler we had to support was MSVC 2002.  But we don't support it
>> > anymore, do we?
>> >
>> > Therefore there is no longer any reason to avoid std::cout, and the
>> > style guidelines should be amended.
>> >
>> > So vtkstd:: has become std:: as of VTK 6
>> > likewise std::cout should be preferred over cout
>> > and std::string over vtkStdString.
>> >
>> > I don't like to see style guidelines being broken (even in tests) so
>> > the sooner the guidelines can be amended, the better.  Just remove the
>> > sentence that insists on using "cout".  And please do not add a line
>> > that insists on "std::cout".
>> >
>> >  - David
>> >
>> >
>> > On Wed, Sep 19, 2012 at 3:27 PM, Bill Lorensen <bill.lorensen at gmail.com>
>> > wrote:
>> >> Julien,
>> >>
>> >> In the early days of VTK, at GE, coding style was a high priority. At
>> >> that
>> >> time we had a paid-for utility that enforced it. Later, before KWStyle,
>> >> VTK
>> >> stopped enforcing the style.
>> >>
>> >> Someday, I hope, we can use KWStyle to bring the style back to what is
>> >> used
>> >> to be. But this will be a big effort. First to define the style via
>> >> KWStyle,
>> >> then to bring VTK into line with the style, and finally to maintain it.
>> >>
>> >> BTW, I still feel that code coverage should be the high priority. But,
>> >> every
>> >> time we bring this up, people raise issues about process, style,
>> >> output,
>> >> etc.  I'd rather have code covered with tests that produce too much
>> >> output,
>> >> or use varying styles versus no test coverage at all.
>> >>
>> >> Bill
>> >>
>> >>
>> >>
>> >> On Wed, Sep 19, 2012 at 5:13 PM, Julien Finet
>> >> <julien.finet at kitware.com>
>> >> wrote:
>> >>>
>> >>> Actually, the coding style should have been defined before (aka is the
>> >>> top
>> >>> priority) the first unit test was written. And the latter should have
>> >>> been
>> >>> written before the first code was written :-P
>> >>> Defining/changing/applying coding style might be a good thing to do in
>> >>> VTK
>> >>> 6.0 (higher priority) as it might change the API. Code coverage (and
>> >>> fixing
>> >>> bugs) can always be done in succeeding minor versions.
>> >>>
>> >>> Concerning your 2nd question (in your first email), if it comes to a
>> >>> vote,
>> >>> I would vote in favor of always prefixing with the namespace of the
>> >>> external
>> >>> library (here std::), as long as it is the standard namespace name
>> >>> (std::
>> >>> and not vtkstd::). It can't hurt to be specific.
>> >>>
>> >>> Julien.
>> >>>
>> >>> On Wed, Sep 19, 2012 at 4:56 PM, Bill Lorensen
>> >>> <bill.lorensen at gmail.com>
>> >>> wrote:
>> >>>>
>> >>>> David,
>> >>>>
>> >>>> There is not style checknig in VTK, nor is there an consistent style
>> >>>> in
>> >>>> VTK. Years ago we had a style and enforced through a third party
>> >>>> package.
>> >>>>
>> >>>> ITK enforces style with KWStyle and we have nightly tests that check
>> >>>> the
>> >>>> style.
>> >>>>
>> >>>> To be honest, style is pretty low in priority versus code coverage
>> >>>> and
>> >>>> valgrind.
>> >>>>
>> >>>> Bill
>> >>>>
>> >>>>
>> >>>> On Wed, Sep 19, 2012 at 4:52 PM, David Thompson
>> >>>> <david.thompson at kitware.com> wrote:
>> >>>>>
>> >>>>> I don't want to be so strict about formatting that we start using
>> >>>>>
>> >>>>>   http://uncrustify.sourceforge.net/
>> >>>>>
>> >>>>> as another pass before commits can be pushed to Gerrit, but it would
>> >>>>> be
>> >>>>> nice to have a "style file" for uncrustify (or a similar tool) that
>> >>>>> embodies
>> >>>>> the VTK code style so less code review time is spent on formatting
>> >>>>> and more
>> >>>>> on the logic, documentation, tests, and organization.
>> >>>>>
>> >>>>>         David
>> >>>>>
>> >>>>> On Sep 19, 2012, at 4:40 PM, "Marcus D. Hanwell"
>> >>>>> <marcus.hanwell at kitware.com> wrote:
>> >>>>>
>> >>>>> > On Wed, Sep 19, 2012 at 4:06 PM, Bill Lorensen
>> >>>>> > <bill.lorensen at gmail.com> wrote:
>> >>>>> >> Folks,
>> >>>>> >>
>> >>>>> >> There has been some recent turmoil regarding the use of std::cout
>> >>>>> >> in
>> >>>>> >> tests
>> >>>>> >> versus cout. The current style guidelines recommend the latter.
>> >>>>> >> First, why is this even an issue. Second, can we amend the style
>> >>>>> >> to
>> >>>>> >> allow
>> >>>>> >> std::cout? Much c++ code I see uses std::cout.
>> >>>>> >>
>> >>>>> >> I'd much rather spend time improving VTK'sl ow coverage that
>> >>>>> >> argue
>> >>>>> >> over what
>> >>>>> >> seems to be a non-issue.
>> >>>>> >>
>> >>>>> > I guess I was the source of the aforementioned turmoil. I think it
>> >>>>> > is
>> >>>>> > important to stick to a common style, and actively changing test
>> >>>>> > code
>> >>>>> > to not match the style seems bad to me. I don't see the point in
>> >>>>> > fully
>> >>>>> > specifying std::cout when we have brought it into the global
>> >>>>> > namespace, but if there is a strong feeling we should then my
>> >>>>> > point
>> >>>>> > was simply that we should specify that is the preferred style from
>> >>>>> > this point forward (or that either is acceptable).
>> >>>>> >
>> >>>>> > If we all start modifying the VTK coding standards then our code
>> >>>>> > will
>> >>>>> > be far less consistent, and harder to understand due to using
>> >>>>> > several
>> >>>>> > styles. If there is general agreement on relaxing some parts of
>> >>>>> > the
>> >>>>> > style I really don't feel very strongly, but I think it should be
>> >>>>> > done
>> >>>>> > deliberately rather than on an ad-hoc basis.
>> >>>>> >
>> >>>>> > Where would you draw the line if this should be relaxed? Could
>> >>>>> > standard indentation be used instead of the special VTK
>> >>>>> > indentation in
>> >>>>> > classes I author? I am sure we don't want to allow that, but it
>> >>>>> > does
>> >>>>> > require some definition on what is strictly required and what is
>> >>>>> > advised/optional.
>> >>>>> >
>> >>>>> > Marcus
>> >>>>> > _______________________________________________
>> >>>>> > Powered by www.kitware.com
>> >>>>> >
>> >>>>> > Visit other Kitware open-source projects at
>> >>>>> > http://www.kitware.com/opensource/opensource.html
>> >>>>> >
>> >>>>> > Follow this link to subscribe/unsubscribe:
>> >>>>> > http://www.vtk.org/mailman/listinfo/vtk-developers
>> >>>>> >
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> Unpaid intern in BillsBasement at noware dot com
>> >>>>
>> >>>>
>> >>>> _______________________________________________
>> >>>> Powered by www.kitware.com
>> >>>>
>> >>>> Visit other Kitware open-source projects at
>> >>>> http://www.kitware.com/opensource/opensource.html
>> >>>>
>> >>>> Follow this link to subscribe/unsubscribe:
>> >>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>> >>>>
>> >>>>
>> >>>
>> >>
>> >>
>> >>
>> >> --
>> >> Unpaid intern in BillsBasement at noware dot com
>> >>
>> >>
>> >> _______________________________________________
>> >> Powered by www.kitware.com
>> >>
>> >> Visit other Kitware open-source projects at
>> >> http://www.kitware.com/opensource/opensource.html
>> >>
>> >> Follow this link to subscribe/unsubscribe:
>> >> http://www.vtk.org/mailman/listinfo/vtk-developers
>> >>
>> >>
>
>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com
>