Hi Patrick,

yes and no...

I have been thinking of adding a runtime switch to include the memory
test or not. The first simple issue, is that memory tests are useful,
but not needed all the time. Functionality goes before memory safety.
This follows the "fake it till you make it" philosophy and this check
is in the way. I would work without it and then before I check into
the source tree I would enable all tests.

I think the feature should follow the time constraints; off by
default, enabled locally or globally with possible local exception.
Plus the ability to compile it out, like for a release or profiling
run. I will look into it this week end.

I was a bit fast on the process of accepting this patch, since I
wanted to get the process going (showing that I was serious) and it is
a patch of fairly good quality.

Sean

On Fri, Jan 29, 2010 at 2:23 PM, Patrick Johnmeyer <pjohnme...@gmail.com> wrote:
> I have reviewed this work and I have a few objections to its inclusion
> as-is.  In order of most important to least important:
> 1) If a function under test has a line of code like this (one of many
> possible examples):
>            static std::tr1::shared_ptr<SomeType> pThing(new SomeType);
> it is perfectly valid but will be flagged as a memory leak with this change
> -- even though, in terms of a program running to termination, it clearly is
> not as the static shared_ptr will go out of scope and take the allocated
> memory with it.  One could imagine a mechanism similar to the existing
> UNITTEST_TIME_CONSTRAINT_EXEMPT macro, but explicitly choosing to opt out of
> memory leak testing in a single test gives the test too much knowledge of
> the underlying code in my opinion.
> This same problem could manifest with all memory management strategies like
> memory pools and the like.  This is why most working memory leak detection
> code I've seen uses _CrtSetAllocHook.
>  http://msdn.microsoft.com/en-us/library/820k4tb8.aspx
> 2) Despite what most (myself included) consider good practice, the simple
> fact is that some UTs in this world are written in such a way that they are
> dependent on previous tests.  This sometimes manifests as what would
> otherwise be considered a memory leak.
> 3) Putting a debug CRT dependency in a release library would make
> performance testing using UT++ highly suspect.  This is not a major issue
> because anybody can change the build options of the project to use whichever
> CRT they prefer, but it is still not ideal.
>
>
> On Sun, Jan 24, 2010 at 11:27 AM, Sean Farrell <sean.farr...@rioki.org>
> wrote:
>>
>> I must admit that I mostly review the code and trust on the unit tests
>> running... But thank you for your effort.
>>
>> Sean
>>
>> On Sun, Jan 24, 2010 at 6:17 PM, Clark Gaebel <cg.wowus...@gmail.com>
>> wrote:
>> > I'm testing it now. I wish there was a painless way to automate that...
>> >
>> > On 24/01/2010 12:14 PM, Sean Farrell wrote:
>> >> Hi all,
>> >>
>> >> I merged the leak detection patch from Clark Gaebel into my git
>> >> repository. Those who are interested can test or review it.
>> >>
>> >> As a reminder the repository can be found at:
>> >> http://github.com/rioki/unittest-cpp
>> >>
>> >> Finally I want to thank Clark Gaebel for the work on the patch and
>> >> preparing it.
>> >>
>> >> Sean
>> >>
>> >> P.S. If you have a patch that you REALLY want to see in the 1.5
>> >> release, the best chances are to prepare a git patch or public
>> >> repository that I can merge into my repository . That is most painless
>> >> for me.
>> >>
>> >> On Thu, Jan 21, 2010 at 11:28 PM, Clark Gaebel <cg.wowus...@gmail.com>
>> >> wrote:
>> >>> http://github.com/wowus/unittest-cpp
>> >>>
>> >>> I sent you a pull request. If anyone wants to review the code:
>> >>>
>> >>> http://github.com/wowus/unittest-cpp/commit/01a69e8eb5c0dd2c0ec16ad3d53a41e893cfed49
>> >>>
>> >>
>> >>
>> >> ------------------------------------------------------------------------------
>> >> Throughout its 18-year history, RSA Conference consistently attracts
>> >> the
>> >> world's best and brightest in the field, creating opportunities for
>> >> Conference
>> >> attendees to learn about information security's most important issues
>> >> through
>> >> interactions with peers, luminaries and emerging and established
>> >> companies.
>> >> http://p.sf.net/sfu/rsaconf-dev2dev
>> >> _______________________________________________
>> >> unittest-cpp-devel mailing list
>> >> unittest-cpp-devel@lists.sourceforge.net
>> >> https://lists.sourceforge.net/lists/listinfo/unittest-cpp-devel
>> >
>> >
>> > ------------------------------------------------------------------------------
>> > Throughout its 18-year history, RSA Conference consistently attracts the
>> > world's best and brightest in the field, creating opportunities for
>> > Conference
>> > attendees to learn about information security's most important issues
>> > through
>> > interactions with peers, luminaries and emerging and established
>> > companies.
>> > http://p.sf.net/sfu/rsaconf-dev2dev
>> > _______________________________________________
>> > unittest-cpp-devel mailing list
>> > unittest-cpp-devel@lists.sourceforge.net
>> > https://lists.sourceforge.net/lists/listinfo/unittest-cpp-devel
>> >
>>
>>
>> ------------------------------------------------------------------------------
>> Throughout its 18-year history, RSA Conference consistently attracts the
>> world's best and brightest in the field, creating opportunities for
>> Conference
>> attendees to learn about information security's most important issues
>> through
>> interactions with peers, luminaries and emerging and established
>> companies.
>> http://p.sf.net/sfu/rsaconf-dev2dev
>> _______________________________________________
>> unittest-cpp-devel mailing list
>> unittest-cpp-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/unittest-cpp-devel
>
>
> ------------------------------------------------------------------------------
> The Planet: dedicated and managed hosting, cloud storage, colocation
> Stay online with enterprise data centers and the best network in the
> business
> Choose flexible plans and management services without long-term contracts
> Personal 24x7 support from experience hosting pros just a phone call away.
> http://p.sf.net/sfu/theplanet-com
> _______________________________________________
> unittest-cpp-devel mailing list
> unittest-cpp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/unittest-cpp-devel
>
>

------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
unittest-cpp-devel mailing list
unittest-cpp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/unittest-cpp-devel

Reply via email to