You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Martin Sebor <se...@roguewave.com> on 2007/09/14 06:57:38 UTC

design of regression tests (was: Re: [jira] Updated: (STDCXX-436) [Linux] MB_LEN_MAX incorrect)

Travis, all,

I'm trying to decide how we should treat the latest regression test
for STDCXX-436:

http://issues.apache.org/jira/secure/attachment/12365789/18.limits.stdcxx-436.cpp

Up until now our approach to "designing" regression tests has been
to simply copy the test case from each issue into the test suite
with only a minimum of changes. The test you've added goes far
beyond that, which makes it valuable because none of the other
macros is being tested, but at the same time it marks quite the
radical departure from the approach taken in all the other tests
which I hesitate to make without bringing it for discussion first.

On the one hand, when an issue comes in that points out a problem
with a feature so closely related to one or more others it makes
perfect sense to make sure that (and add tests for) the other
related features aren't broken, too. On the other hand, the name
of each regression test clearly indicates which bug it is designed
to exercise and when it should fail for some other reason (e.g.,
a regression in one of the other related macros) it would mislead
one into thinking that there's a problem with MB_LEN_MAX.

I suppose that my view on this is in cases like this, when the
bug report highlights a whole slew of features that aren't being
tested we should add an ordinary (unit) test for the whole area
and, if possible, also a regression test just for the bug. My
rationale for keeping the two separate (even at the cost of
duplicating some tested functionality) is that the bigger unit
test is more likely to be enhanced or tweaked in the future and
thus more likely to be subject to accidentally removing the test
for the bug (or otherwise "regressing"), while the regression
test is much more likely to be left  alone and consequently less
prone to such accidental regressions.

Opinions? Comments? Thoughts?

Martin

Travis Vitek (JIRA) wrote:
>      [ https://issues.apache.org/jira/browse/STDCXX-436?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
> 
> Travis Vitek updated STDCXX-436:
> --------------------------------
> 
>     Attachment: 18.limits.stdcxx-436.cpp
> 
> Updated regression test compiles a simple program that simulates getconf and then uses that program to get the values to compare against. The alternative was to just invoke getconv as suggested, but that didn't work on windows, and getconf on some platforms doesn't provide values for all constants [LONG_MIN and LONG_MAX on linux is one example].
> 
>> [Linux] MB_LEN_MAX incorrect
>> ----------------------------
>>
>>                 Key: STDCXX-436
>>                 URL: https://issues.apache.org/jira/browse/STDCXX-436
>>             Project: C++ Standard Library
>>          Issue Type: Bug
>>          Components: 18. Language Support
>>    Affects Versions: 4.1.3
>>         Environment: gcc version 4.1.1 20070105 (Red Hat 4.1.1-51)
>>            Reporter: Mark Brown
>>            Assignee: Travis Vitek
>>            Priority: Critical
>>             Fix For: 4.2
>>
>>         Attachments: 18.limits.stdcxx-436.cpp, LIMITS.cpp.patch, stdcxx-436.patch
>>
>>
>> On my Linux system MB_LEN_MAX is normally defined to 16 but when I use the macro in a program compiled with stdcxx the macro evaluates to 1. The test case goes like this:
>> $ cat test.cpp && make CPPOPTS="-DGETCONF_MB_LEN_MAX=`getconf MB_LEN_MAX`" test && ./test
>> #include <assert.h>
>> #include <limits.h>
>> int main ()
>> {
>>     assert (MB_LEN_MAX == GETCONF_MB_LEN_MAX);
>> }
>> gcc -c -I/home/mbrown/stdcxx/include/ansi -D_RWSTDDEBUG    -I/home/mbrown/stdcxx/include -I/home/mbrown/stdcxx-gcc-4.1.1-11s/include -I/home/mbrown/stdcxx/examples/include -DGETCONF_MB_LEN_MAX=16 -pedantic -nostdinc++ -g  -W -Wall -Wcast-qual -Winline -Wshadow -Wwrite-strings -Wno-long-long -Wcast-align   test.cpp
>> gcc u.o -o u  -L/home/mbrown/stdcxx-gcc-4.1.1-11s/lib  -lstd11s -lsupc++ -lm 
>> test: test.cpp:6: int main(): Assertion `1 == 16' failed.
>> Aborted
> 


RE: design of regression tests (was: Re: [jira] Updated: (STDCXX-436) [Linux] MB_LEN_MAX incorrect)

Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Travis Vitek [mailto:tvitek@quovadx.com] 
> Sent: Friday, September 14, 2007 10:46 AM
> To: stdcxx-dev@incubator.apache.org
> Subject: RE: design of regression tests (was: Re: [jira] 
> Updated: (STDCXX-436) [Linux] MB_LEN_MAX incorrect)


> That said, I'm actually hoping to get feedback an the guts of 
> the test itself. It feels a bit fragile to me because it 
> compiles a separate executable to emulate getconf [for the 
> necessary constants only], and to do this I had to hardcode 
> the compiler names and flags into the test.
> I'm just hoping that this isn't something that will cause a 
> bunch of trouble in the future.

  The test relies on fact that cl.exe / icl.exe are accessible
through PATH environment variable, but it might be not true.

> If it looks fragile to you 
> guys, then maybe it would be best to just pass for windows 
> builds and invoke the system getconf as you suggested earlier 
> on other platforms.

  Once the 22.locale.messages.cpp test had compiled the message catalog
using rc.exe and link.exe on Windows... And now we have the util/gencat
utilty ;-)

Farid.

Re: design of regression tests

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
[...]
>> The formatting code is also slightly buggy
>> because it doesn't account for integer promotion and sign extension.
>>
> 
> I'm not sure what you mean by this. Can you give an example of how this
> would be a problem?

This was a thinko on my part. We're not concerned about the formatting
of arbitrary character values (e.g., '\x80' and unexpectedly getting
back -128 instead of 128 when char is a signed type) but just the two
values, CHAR_MAX and CHAR_MIN, and we do expect to see the negative
value when CHAR_MIN is negative. Sorry about that!

Martin

RE: design of regression tests

Posted by Travis Vitek <tv...@quovadx.com>.
 
Martin Sebor wrote:
>
>I agree that it looks fragile, although I admit I am intrigued
>by the idea of programmatically invoking the compiler. I have
>been contemplating an API that would let us do this in general
>(i.e., build programs or even libraries, either using stdcxx
>or the native C++ Standard Library, or even C programs) but so
>far it's just an idea.
>
>Despite the fragility, I do see the appeal of this approach:
>unlike computing the constants directly in the test that I was
>at first inclined to suggest, it guarantees to yield the exact
>same values as the C library. (I assume that's why you chose
>it?) I suggest we keep the test in Jira and revisit it, maybe
>along withe the whole compiler API idea, when we have more
>time after the release.
>
>Some observations about the formatting code in the test:
>
>First, there's a macro for the "template <>" syntax to deal with
>outdated compiler: _RWSTD_SPECIALIZED_FUNCTION. We have been
>talking about dropping support for these kinds of workarounds at
>some point in the (near) future but we haven't actually done it
>yet. So until we do, we should continue to use these macros (if
>nothing else, it'll help us all appreciate the extent of these
>workarounds and decide which one we want to get rid of and which
>one we might want to keep).
>

Thanks, that may be useful for future reference.

>
>Second, this kind of type-based formatting is unnecessary. It would
>be sufficient to convert each constant to long and use %li or %lu
>to format them all. 
>

Yup, I realized that and wrote a new version of the test Friday night.
That simplifies things quite a bit.

>
>The formatting code is also slightly buggy
>because it doesn't account for integer promotion and sign extension.
>

I'm not sure what you mean by this. Can you give an example of how this
would be a problem?

Re: design of regression tests

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
[...]
> I'm onboard with reducing the scope of this test to just verify
> MB_LEN_MAX and creating a new test for verifying all of the required
> limits.

Okay, let's do that then.

> 
> That said, I'm actually hoping to get feedback an the guts of the test
> itself. It feels a bit fragile to me because it compiles a separate
> executable to emulate getconf [for the necessary constants only], and to
> do this I had to hardcode the compiler names and flags into the test.
> I'm just hoping that this isn't something that will cause a bunch of
> trouble in the future. If it looks fragile to you guys, then maybe it
> would be best to just pass for windows builds and invoke the system
> getconf as you suggested earlier on other platforms.

I agree that it looks fragile, although I admit I am intrigued
by the idea of programmatically invoking the compiler. I have
been contemplating an API that would let us do this in general
(i.e., build programs or even libraries, either using stdcxx
or the native C++ Standard Library, or even C programs) but so
far it's just an idea.

Despite the fragility, I do see the appeal of this approach:
unlike computing the constants directly in the test that I was
at first inclined to suggest, it guarantees to yield the exact
same values as the C library. (I assume that's why you chose
it?) I suggest we keep the test in Jira and revisit it, maybe
along withe the whole compiler API idea, when we have more
time after the release.

Some observations about the formatting code in the test:

First, there's a macro for the "template <>" syntax to deal with
outdated compiler: _RWSTD_SPECIALIZED_FUNCTION. We have been
talking about dropping support for these kinds of workarounds at
some point in the (near) future but we haven't actually done it
yet. So until we do, we should continue to use these macros (if
nothing else, it'll help us all appreciate the extent of these
workarounds and decide which one we want to get rid of and which
one we might want to keep).

Second, this kind of type-based formatting is unnecessary. It would
be sufficient to convert each constant to long and use %li or %lu
to format them all. The formatting code is also slightly buggy
because it doesn't account for integer promotion and sign extension.
At least one of the conversion specifications (%hc) is also not
defined.

Martin

RE: design of regression tests (was: Re: [jira] Updated: (STDCXX-436) [Linux] MB_LEN_MAX incorrect)

Posted by Travis Vitek <tv...@quovadx.com>.
 

Martin Sebor wrote:
>
>Travis, all,
>
>I'm trying to decide how we should treat the latest regression test
>for STDCXX-436:
>
>http://issues.apache.org/jira/secure/attachment/12365789/18.limits.stdc
xx-436.cpp
>
>Up until now our approach to "designing" regression tests has been
>to simply copy the test case from each issue into the test suite
>with only a minimum of changes. The test you've added goes far
>beyond that, which makes it valuable because none of the other
>macros is being tested, but at the same time it marks quite the
>radical departure from the approach taken in all the other tests
>which I hesitate to make without bringing it for discussion first.
>

Yes. I noticed that none of the other macros were being tested of this
same issue, so I just lumped them all in there to avoid writing a test
that was just a copy/paste of this one.

>
>On the one hand, when an issue comes in that points out a problem
>with a feature so closely related to one or more others it makes
>perfect sense to make sure that (and add tests for) the other
>related features aren't broken, too. On the other hand, the name
>of each regression test clearly indicates which bug it is designed
>to exercise and when it should fail for some other reason (e.g.,
>a regression in one of the other related macros) it would mislead
>one into thinking that there's a problem with MB_LEN_MAX.
>

Agreed.

>
>I suppose that my view on this is in cases like this, when the
>bug report highlights a whole slew of features that aren't being
>tested we should add an ordinary (unit) test for the whole area
>and, if possible, also a regression test just for the bug. My
>rationale for keeping the two separate (even at the cost of
>duplicating some tested functionality) is that the bigger unit
>test is more likely to be enhanced or tweaked in the future and
>thus more likely to be subject to accidentally removing the test
>for the bug (or otherwise "regressing"), while the regression
>test is much more likely to be left  alone and consequently less
>prone to such accidental regressions.
>
>Opinions? Comments? Thoughts?
>

I'm onboard with reducing the scope of this test to just verify
MB_LEN_MAX and creating a new test for verifying all of the required
limits.

That said, I'm actually hoping to get feedback an the guts of the test
itself. It feels a bit fragile to me because it compiles a separate
executable to emulate getconf [for the necessary constants only], and to
do this I had to hardcode the compiler names and flags into the test.
I'm just hoping that this isn't something that will cause a bunch of
trouble in the future. If it looks fragile to you guys, then maybe it
would be best to just pass for windows builds and invoke the system
getconf as you suggested earlier on other platforms.

Travis

Travis