stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Lemings" <Eric.Lemi...@roguewave.com>
Subject RE: structure of tuple tests ([Fwd: Re: svn commit: r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h include/tuple tests/utilities/20.tuple.cnstr.cpp tests/utilities/20.tuple.creation.cpp tests/utilities/20.tuple.h tests/utilities/20.tuple.helpers
Date Thu, 17 Jul 2008 14:47:09 GMT
 

> -----Original Message-----
> From: Martin Sebor [mailto:msebor@gmail.com] On Behalf Of Martin Sebor
> Sent: Wednesday, July 16, 2008 11:11 PM
> To: dev@stdcxx.apache.org
> Subject: Re: structure of tuple tests ([Fwd: Re: svn commit: 
> r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h 
> include/tuple tests/utilities/20.tuple.cnstr.cpp 
> tests/utilities/20.tuple.creation.cpp 
> tests/utilities/20.tuple.h tests/utilities/20.tuple.helpers
> 
> Eric Lemings wrote:
> >  
> > 
> >> -----Original Message-----
> >> From: Martin Sebor [mailto:msebor@gmail.com] On Behalf Of 
> Martin Sebor
> >> Sent: Friday, July 11, 2008 1:06 PM
> >> To: dev@stdcxx.apache.org
> >> Subject: Re: structure of tuple tests ([Fwd: Re: svn commit: 
> >> r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h 
> >> include/tuple tests/utilities/20.tuple.cnstr.cpp 
> >> tests/utilities/20.tuple.creation.cpp 
> >> tests/utilities/20.tuple.h tests/utilities/20.tuple.helpers
> >>
> > ...
> >> You might want to consider extending the UserClass class defined
> >> in tests/include/rw_value.h, or at least borrowing code or ideas
> >> from it.
> > 
> > I generalized the overall test pattern for tuples with the latest
> > revision 677458.  It might not be exactly what you're after but I
> > think it's much closer than the previous version.  For instance, I
> > lot of the tests could be wrapped in TEST() function macros.
> > 
> > Have a look if you find time.
> 
> Okay, I looked :) Just briefly, but I do have a few high-level
> comments and suggestions for you.
> 
> First, let's avoid using one library component to test another.
> Especially avoid using ostream except in tests exercising I/O
> (those should be separate from all others). We shouldn't be
> #including <ostream> or <string> in 20.tuple.h.

It's not currently used.  I can remove it.

> Also avoid
> using std::rand() and, ideally, any randomization at all. It
> makes it hard to reliably reproduce the same test results.

Not in this case.

> For <type_traits>, if all tuple needs is decay, it would be
> nice if we could come up with a way to do without it.

Yeah I could add FMT_SPEC() for each combination of cv-qualifier
and reference types for all types already specified.  I'd rather
not though and just rely on decay since that's what its meant
to do.

> In 20.tuple.cnstr.cpp you might also be able to get rid of
> <cstring> by using rw_strncmp(). You should also be able
> to use rw_equal() instead of defining a special helper (if
> rw_equal() doesn't do something we need to do in the tuple
> tests maybe we could extend it?)

Was not aware of those.  I'll try to reuse them.

> Another point: most people expect headers to be #included at
> the top of a file. It's surprising to see them included in
> the middle, and it can have unexpected effects in template
> code. Please move all #include directives to the top of the
> 20.tuple.cnstr.cpp test.

It's not used either.  I can just remove it.

> Third, I would rather strongly recommend removing the global
> tuple typedefs from the common header and defining them in
> each test function as necessary. AFAICS, they don't save us
> anything (they're all onle-liners), and it's far from clear
> from their names what each of them represents. Having their
> exact types spelled out at the point of their use will make
> the tests more readable, and much easier to change. The
> current setup suggests that to add a single test case for
> a new tuple we will have to edit the 20.tuple.h header shared
> across all tests, exposing all of them to the fallout from
> our edits.

I can do that.  It will make the code much more verbose but
also more explicit, as you say.
 
> Also, it's generally a good idea to write headers in a way
> that lets them be #included in multiple translation units of
> the same program. That means avoiding data definitions (except
> for those with internal linkage). I realize that 20.tuple.h
> isn't meant to be #included by multiple TUs, but that might
> change, especially if we decide to move the header into
> tests/include like all other common headers, and compile
> its contents into the test driver.

You probably wouldn't want to do that since it would add symbols
to the RWTest library that would get linked into every test even
though they are only used in a handful of tests, namely the
tuple tests.

Regardless, its good practice so I have no problem doing this
either.

Brad.

Mime
View raw message