www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Unit test practices in Phobos

reply "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
I just filed a bug report (3240) that describes a case where IFTI is 
used in Phobos, and where this causes errors when the function is used 
with a different type than the one used in the unittest. (The well known 
"IFTI doesn't work with implicit conversions" problem.) I have a strong 
suspicion that there are many other cases like this waiting to be 
discovered.

I have encountered such errors in my own code many times, and lately 
I've been trying to get into the habit of writing unittests for all (or 
at least more than one) types. Not full-fledged functionality tests, 
mind you -- something like this is usually sufficient:

   T foo(T)(T x) if (isFloatingPoint!T) { return x + 1.0; }

   unittest
   {
       // Test different types
       alias foo!float foo_float;
       alias foo!double foo_double;
       alias foo!real foo_real;

       // Test functionality
       assert (foo(2.0) == 3.0);
   }

For the cases where any type is allowed (or a lot of them, at least) 
even this can become a time-consuming task. In these cases it should at 
least be possible to make a representative selection of types to check.

I just wanted to recommend this as "good practice" to all, but 
especially to the Phobos authors. In my experience this catches a lot of 
bugs which are otherwise hard to spot.

-Lars
Aug 10 2009
next sibling parent reply Daniel Keep <daniel.keep.lists gmail.com> writes:
Lars T. Kyllingstad wrote:
 ...

Knocked this up in about two minutes. Might be handy. template TestInstantiateImpl(alias Tmpl, Ts...) { static if( Ts.length > 0 ) { alias Tmpl!(Ts[0]) test; alias TestInstantiateImpl!(Tmpl, Ts[1..$]).next next; } else { enum next = true; } } template TestInstantiate(alias Tmpl, Ts...) { alias TestInstantiateImpl!(Tmpl, Ts).next TestInstantiate; } template Blah(T) { pragma(msg, "Blah!("~T.stringof~")"); alias T Blah; } T nanFor(T)() { pragma(msg, "nanFor!("~T.stringof~")"); return T.nan; } unittest { static assert( TestInstantiate!(Blah, float, double, real) ); static assert( TestInstantiate!(nanFor, float, double, real) ); static assert( TestInstantiate!(Blah, int) ); static assert( TestInstantiate!(nanFor, int) ); } void main() { } $ dmd -unittest irc Blah!(float) Blah!(double) Blah!(real) nanFor!(float) nanFor!(double) nanFor!(real) Blah!(int) nanFor!(int) irc.d(34): Error: no property 'nan' for type 'int' irc.d(43): Error: template instance irc.TestInstantiate!(nanFor,int) error instantiating
Aug 10 2009
parent reply "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
Daniel Keep wrote:
 
 Lars T. Kyllingstad wrote:
 ...

Knocked this up in about two minutes. Might be handy. template TestInstantiateImpl(alias Tmpl, Ts...) { static if( Ts.length > 0 ) { alias Tmpl!(Ts[0]) test; alias TestInstantiateImpl!(Tmpl, Ts[1..$]).next next; } else { enum next = true; } } template TestInstantiate(alias Tmpl, Ts...) { alias TestInstantiateImpl!(Tmpl, Ts).next TestInstantiate; } template Blah(T) { pragma(msg, "Blah!("~T.stringof~")"); alias T Blah; } T nanFor(T)() { pragma(msg, "nanFor!("~T.stringof~")"); return T.nan; } unittest { static assert( TestInstantiate!(Blah, float, double, real) ); static assert( TestInstantiate!(nanFor, float, double, real) ); static assert( TestInstantiate!(Blah, int) ); static assert( TestInstantiate!(nanFor, int) ); } void main() { } $ dmd -unittest irc Blah!(float) Blah!(double) Blah!(real) nanFor!(float) nanFor!(double) nanFor!(real) Blah!(int) nanFor!(int) irc.d(34): Error: no property 'nan' for type 'int' irc.d(43): Error: template instance irc.TestInstantiate!(nanFor,int) error instantiating

Good idea. :) Without having tested it, I think your template also works with multi-parameter templates: template Foo(T, U, V) { ... } unittest { static assert (TestInstantiate!(Foo, Tuple!(int, real, real), Tuple!(uint, double, cdouble))); } If something like this was to go into a library, I'd remove the pragma(msg)s, though. They'd get pretty annoying after a while. ;) -Lars
Aug 10 2009
parent reply "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
Lars T. Kyllingstad wrote:
 Daniel Keep wrote:
 Lars T. Kyllingstad wrote:
 ...

Knocked this up in about two minutes. Might be handy. template TestInstantiateImpl(alias Tmpl, Ts...) { static if( Ts.length > 0 ) { alias Tmpl!(Ts[0]) test; alias TestInstantiateImpl!(Tmpl, Ts[1..$]).next next; } else { enum next = true; } } template TestInstantiate(alias Tmpl, Ts...) { alias TestInstantiateImpl!(Tmpl, Ts).next TestInstantiate; } template Blah(T) { pragma(msg, "Blah!("~T.stringof~")"); alias T Blah; } T nanFor(T)() { pragma(msg, "nanFor!("~T.stringof~")"); return T.nan; } unittest { static assert( TestInstantiate!(Blah, float, double, real) ); static assert( TestInstantiate!(nanFor, float, double, real) ); static assert( TestInstantiate!(Blah, int) ); static assert( TestInstantiate!(nanFor, int) ); } void main() { } $ dmd -unittest irc Blah!(float) Blah!(double) Blah!(real) nanFor!(float) nanFor!(double) nanFor!(real) Blah!(int) nanFor!(int) irc.d(34): Error: no property 'nan' for type 'int' irc.d(43): Error: template instance irc.TestInstantiate!(nanFor,int) error instantiating

If something like this was to go into a library, I'd remove the pragma(msg)s, though. They'd get pretty annoying after a while. ;)

...except the pragmas are defined in Blah and nanFor themselves, not in TestInstantiate... Note to self: Think before you post. -Lars
Aug 10 2009
parent Daniel Keep <daniel.keep.lists gmail.com> writes:
Lars T. Kyllingstad wrote:
 Lars T. Kyllingstad wrote:
 Daniel Keep wrote:
 Lars T. Kyllingstad wrote:
 ...

Knocked this up in about two minutes. Might be handy. template TestInstantiateImpl(alias Tmpl, Ts...) { static if( Ts.length > 0 ) { alias Tmpl!(Ts[0]) test; alias TestInstantiateImpl!(Tmpl, Ts[1..$]).next next; } else { enum next = true; } } template TestInstantiate(alias Tmpl, Ts...) { alias TestInstantiateImpl!(Tmpl, Ts).next TestInstantiate; } template Blah(T) { pragma(msg, "Blah!("~T.stringof~")"); alias T Blah; } T nanFor(T)() { pragma(msg, "nanFor!("~T.stringof~")"); return T.nan; } unittest { static assert( TestInstantiate!(Blah, float, double, real) ); static assert( TestInstantiate!(nanFor, float, double, real) ); static assert( TestInstantiate!(Blah, int) ); static assert( TestInstantiate!(nanFor, int) ); } void main() { } $ dmd -unittest irc Blah!(float) Blah!(double) Blah!(real) nanFor!(float) nanFor!(double) nanFor!(real) Blah!(int) nanFor!(int) irc.d(34): Error: no property 'nan' for type 'int' irc.d(43): Error: template instance irc.TestInstantiate!(nanFor,int) error instantiating

If something like this was to go into a library, I'd remove the pragma(msg)s, though. They'd get pretty annoying after a while. ;)

....except the pragmas are defined in Blah and nanFor themselves, not in TestInstantiate... Note to self: Think before you post. -Lars

Oh, it's worse than that: Tuples flatten so what you're actually doing is instantiating TestInstantiate with SIX parameters. TestInstantiate doesn't actually know how many arguments each template is supposed to get. You could get around this with a tuple struct type. The problem then is that you couldn't pass alias or value arguments. You might be able to fix THAT by having a second argument that specifies how many arguments the template takes, then slice the required number of elements from the tuple... But I'm really not sufficiently married to the idea to bother with all that. :P
Aug 10 2009
prev sibling parent reply Jeremie Pelletier <jeremiep gmail.com> writes:
Lars T. Kyllingstad Wrote:

 I just filed a bug report (3240) that describes a case where IFTI is 
 used in Phobos, and where this causes errors when the function is used 
 with a different type than the one used in the unittest. (The well known 
 "IFTI doesn't work with implicit conversions" problem.) I have a strong 
 suspicion that there are many other cases like this waiting to be 
 discovered.
 
 I have encountered such errors in my own code many times, and lately 
 I've been trying to get into the habit of writing unittests for all (or 
 at least more than one) types. Not full-fledged functionality tests, 
 mind you -- something like this is usually sufficient:
 
    T foo(T)(T x) if (isFloatingPoint!T) { return x + 1.0; }
 
    unittest
    {
        // Test different types
        alias foo!float foo_float;
        alias foo!double foo_double;
        alias foo!real foo_real;
 
        // Test functionality
        assert (foo(2.0) == 3.0);
    }
 
 For the cases where any type is allowed (or a lot of them, at least) 
 even this can become a time-consuming task. In these cases it should at 
 least be possible to make a representative selection of types to check.
 
 I just wanted to recommend this as "good practice" to all, but 
 especially to the Phobos authors. In my experience this catches a lot of 
 bugs which are otherwise hard to spot.
 
 -Lars

I just go with type tuples: T foo(T)(T x) if(isFloatingPoint!T) { return x + 1.0; } unittest { foreach(T; allFloatingPointTuple) assert(foo!T(1.0) == 2.0); }
Aug 10 2009
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
Jeremie Pelletier wrote:
 Lars T. Kyllingstad Wrote:
 
 I just filed a bug report (3240) that describes a case where IFTI is 
 used in Phobos, and where this causes errors when the function is used 
 with a different type than the one used in the unittest. (The well known 
 "IFTI doesn't work with implicit conversions" problem.) I have a strong 
 suspicion that there are many other cases like this waiting to be 
 discovered.

 I have encountered such errors in my own code many times, and lately 
 I've been trying to get into the habit of writing unittests for all (or 
 at least more than one) types. Not full-fledged functionality tests, 
 mind you -- something like this is usually sufficient:

    T foo(T)(T x) if (isFloatingPoint!T) { return x + 1.0; }

    unittest
    {
        // Test different types
        alias foo!float foo_float;
        alias foo!double foo_double;
        alias foo!real foo_real;

        // Test functionality
        assert (foo(2.0) == 3.0);
    }

 For the cases where any type is allowed (or a lot of them, at least) 
 even this can become a time-consuming task. In these cases it should at 
 least be possible to make a representative selection of types to check.

 I just wanted to recommend this as "good practice" to all, but 
 especially to the Phobos authors. In my experience this catches a lot of 
 bugs which are otherwise hard to spot.

 -Lars

I just go with type tuples: T foo(T)(T x) if(isFloatingPoint!T) { return x + 1.0; } unittest { foreach(T; allFloatingPointTuple) assert(foo!T(1.0) == 2.0); }

Yah, same here. I have unit tests in Phobos that have nested loops testing against so many types, the release build takes forever. Some edge case for the optimizer. I must disable them in release builds. Andrei
Aug 10 2009
parent reply Jeremie Pelletier <jeremiep gmail.com> writes:
Andrei Alexandrescu Wrote:

 Jeremie Pelletier wrote:
 Lars T. Kyllingstad Wrote:
 
 I just filed a bug report (3240) that describes a case where IFTI is 
 used in Phobos, and where this causes errors when the function is used 
 with a different type than the one used in the unittest. (The well known 
 "IFTI doesn't work with implicit conversions" problem.) I have a strong 
 suspicion that there are many other cases like this waiting to be 
 discovered.

 I have encountered such errors in my own code many times, and lately 
 I've been trying to get into the habit of writing unittests for all (or 
 at least more than one) types. Not full-fledged functionality tests, 
 mind you -- something like this is usually sufficient:

    T foo(T)(T x) if (isFloatingPoint!T) { return x + 1.0; }

    unittest
    {
        // Test different types
        alias foo!float foo_float;
        alias foo!double foo_double;
        alias foo!real foo_real;

        // Test functionality
        assert (foo(2.0) == 3.0);
    }

 For the cases where any type is allowed (or a lot of them, at least) 
 even this can become a time-consuming task. In these cases it should at 
 least be possible to make a representative selection of types to check.

 I just wanted to recommend this as "good practice" to all, but 
 especially to the Phobos authors. In my experience this catches a lot of 
 bugs which are otherwise hard to spot.

 -Lars

I just go with type tuples: T foo(T)(T x) if(isFloatingPoint!T) { return x + 1.0; } unittest { foreach(T; allFloatingPointTuple) assert(foo!T(1.0) == 2.0); }

Yah, same here. I have unit tests in Phobos that have nested loops testing against so many types, the release build takes forever. Some edge case for the optimizer. I must disable them in release builds. Andrei

Don't you disable unittests in release builds?
Aug 10 2009
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
Jeremie Pelletier wrote:
 Andrei Alexandrescu Wrote:
 
 Jeremie Pelletier wrote:
 Lars T. Kyllingstad Wrote:

 I just filed a bug report (3240) that describes a case where IFTI is 
 used in Phobos, and where this causes errors when the function is used 
 with a different type than the one used in the unittest. (The well known 
 "IFTI doesn't work with implicit conversions" problem.) I have a strong 
 suspicion that there are many other cases like this waiting to be 
 discovered.

 I have encountered such errors in my own code many times, and lately 
 I've been trying to get into the habit of writing unittests for all (or 
 at least more than one) types. Not full-fledged functionality tests, 
 mind you -- something like this is usually sufficient:

    T foo(T)(T x) if (isFloatingPoint!T) { return x + 1.0; }

    unittest
    {
        // Test different types
        alias foo!float foo_float;
        alias foo!double foo_double;
        alias foo!real foo_real;

        // Test functionality
        assert (foo(2.0) == 3.0);
    }

 For the cases where any type is allowed (or a lot of them, at least) 
 even this can become a time-consuming task. In these cases it should at 
 least be possible to make a representative selection of types to check.

 I just wanted to recommend this as "good practice" to all, but 
 especially to the Phobos authors. In my experience this catches a lot of 
 bugs which are otherwise hard to spot.

 -Lars

T foo(T)(T x) if(isFloatingPoint!T) { return x + 1.0; } unittest { foreach(T; allFloatingPointTuple) assert(foo!T(1.0) == 2.0); }

testing against so many types, the release build takes forever. Some edge case for the optimizer. I must disable them in release builds. Andrei

Don't you disable unittests in release builds?

Phobos has the builds: debug, release, unittest/debug, and unittest/release. Client apps use the release version. I like being able to unittest the release version to make sure that the optimizer etc. don't do shenanigans. Andrei
Aug 10 2009
parent Jeremie Pelletier <jeremiep gmail.com> writes:
Andrei Alexandrescu Wrote:

 Jeremie Pelletier wrote:
 Andrei Alexandrescu Wrote:
 
 Jeremie Pelletier wrote:
 Lars T. Kyllingstad Wrote:

 I just filed a bug report (3240) that describes a case where IFTI is 
 used in Phobos, and where this causes errors when the function is used 
 with a different type than the one used in the unittest. (The well known 
 "IFTI doesn't work with implicit conversions" problem.) I have a strong 
 suspicion that there are many other cases like this waiting to be 
 discovered.

 I have encountered such errors in my own code many times, and lately 
 I've been trying to get into the habit of writing unittests for all (or 
 at least more than one) types. Not full-fledged functionality tests, 
 mind you -- something like this is usually sufficient:

    T foo(T)(T x) if (isFloatingPoint!T) { return x + 1.0; }

    unittest
    {
        // Test different types
        alias foo!float foo_float;
        alias foo!double foo_double;
        alias foo!real foo_real;

        // Test functionality
        assert (foo(2.0) == 3.0);
    }

 For the cases where any type is allowed (or a lot of them, at least) 
 even this can become a time-consuming task. In these cases it should at 
 least be possible to make a representative selection of types to check.

 I just wanted to recommend this as "good practice" to all, but 
 especially to the Phobos authors. In my experience this catches a lot of 
 bugs which are otherwise hard to spot.

 -Lars

T foo(T)(T x) if(isFloatingPoint!T) { return x + 1.0; } unittest { foreach(T; allFloatingPointTuple) assert(foo!T(1.0) == 2.0); }

testing against so many types, the release build takes forever. Some edge case for the optimizer. I must disable them in release builds. Andrei

Don't you disable unittests in release builds?

Phobos has the builds: debug, release, unittest/debug, and unittest/release. Client apps use the release version. I like being able to unittest the release version to make sure that the optimizer etc. don't do shenanigans. Andrei

Oh yeah I just did a test compile with -O -release -inline -unittest, took about 30 seconds to compile. Still this is lightning fast when compared to C. Even using make with 40 jobs on my quad core a lot of programs take minutes to compile, DMD does it all in one big swoop, no jobs needed, no makefile needed.
Aug 10 2009