Thread overview
phobos unit tests
Sep 03, 2016
Manu
Sep 03, 2016
Seb
Sep 03, 2016
Jonathan M Davis
Sep 03, 2016
ZombineDev
Sep 03, 2016
Jonathan M Davis
Sep 04, 2016
ZombineDev
September 03, 2016
This document: https://wiki.dlang.org/Contributing_to_Phobos

States: "Avoid unittest in templates (it will generate a new unittest for each instance) - put your tests outside"

Sounds reasonable, but then I realised that most of my unit tests are documenting unittests... this recommendation is in conflict with the documentation standards... who wins?
September 03, 2016
On Saturday, 3 September 2016 at 03:36:06 UTC, Manu wrote:
> This document: https://wiki.dlang.org/Contributing_to_Phobos
>
> States: "Avoid unittest in templates (it will generate a new unittest for each instance) - put your tests outside"
>
> Sounds reasonable, but then I realised that most of my unit tests are documenting unittests... this recommendation is in conflict with the documentation standards... who wins?

For the rationale see e.g. this thread:

https://forum.dlang.org/post/wnydmelyxprvhsxewums@forum.dlang.org

A trick around this is sth. like:

version(D_Ddoc)
{
    enum doUnittest = true;
}
else
{
    version(unittest)
    {
        enum doUnittest = true;
    }
    else
    {
        enum doUnittest = false;
    }
}

and then `static if (doUnittest)` the tests inside. For example ndslice uses a similar pattern.
September 03, 2016
On 9/3/16 5:36 AM, Manu via Digitalmars-d wrote:
> This document: https://wiki.dlang.org/Contributing_to_Phobos
>
> States: "Avoid unittest in templates (it will generate a new unittest
> for each instance) - put your tests outside"

Actually that's a good thing, sometimes you do want to run a unittest for each instantiation.

> Sounds reasonable, but then I realised that most of my unit tests are
> documenting unittests... this recommendation is in conflict with the
> documentation standards... who wins?

Just version it only for the documentation.


Andrei


September 03, 2016
On Saturday, September 03, 2016 15:06:33 Andrei Alexandrescu via Digitalmars-d wrote:
> On 9/3/16 5:36 AM, Manu via Digitalmars-d wrote:
> > This document: https://wiki.dlang.org/Contributing_to_Phobos
> >
> > States: "Avoid unittest in templates (it will generate a new unittest for each instance) - put your tests outside"
>
> Actually that's a good thing, sometimes you do want to run a unittest for each instantiation.

Sometimes, sure. But usually not.

> > Sounds reasonable, but then I realised that most of my unit tests are documenting unittests... this recommendation is in conflict with the documentation standards... who wins?
>
> Just version it only for the documentation.

Then the tests won't be run unless your documentation build is also your unittest build, and they frequently can't be the same - e.g. when something differs between operating systems, and you have a separate D_Ddoc or StdDdoc block so that you can have proper documentation without requiring that the documentation be built on all systems and potentially being able to document what exists on each system. For instance, the supported clock types vary quite a bit across operating systems, so the core.time.ClockType enum has different values on different systems and version(CoreDdoc) is used so that the documentation can list them all and list the consistently across systems.

You could add some sort of version identifier that's specific to your build that you use for your unittest builds (similar to how Phobos has StdDdoc separate from D_Ddoc), but even if you do that, you still have the problem of the non-generic unit tests within a template being compiled into every instantiation of the template that's part of your unittest build, and all of those tests will get run, increasing both the build times and the time it takes to run the unit tests.

- Jonathan M Davis

September 03, 2016
On Saturday, 3 September 2016 at 15:54:31 UTC, Jonathan M Davis wrote:
> On Saturday, September 03, 2016 15:06:33 Andrei Alexandrescu via Digitalmars-d wrote:
>> On 9/3/16 5:36 AM, Manu via Digitalmars-d wrote:
>> > This document: https://wiki.dlang.org/Contributing_to_Phobos
>> >
>> > States: "Avoid unittest in templates (it will generate a new unittest for each instance) - put your tests outside"
>>
>> Actually that's a good thing, sometimes you do want to run a unittest for each instantiation.
>
> Sometimes, sure. But usually not.
>
>> > Sounds reasonable, but then I realised that most of my unit tests are documenting unittests... this recommendation is in conflict with the documentation standards... who wins?
>>
>> Just version it only for the documentation.
>
> Then the tests won't be run unless your documentation build is also your unittest build, and they frequently can't be the same - e.g. when something differs between operating systems, and you have a separate D_Ddoc or StdDdoc block so that you can have proper documentation without requiring that the documentation be built on all systems and potentially being able to document what exists on each system. For instance, the supported clock types vary quite a bit across operating systems, so the core.time.ClockType enum has different values on different systems and version(CoreDdoc) is used so that the documentation can list them all and list the consistently across systems.
>
> You could add some sort of version identifier that's specific to your build that you use for your unittest builds (similar to how Phobos has StdDdoc separate from D_Ddoc), but even if you do that, you still have the problem of the non-generic unit tests within a template being compiled into every instantiation of the template that's part of your unittest build, and all of those tests will get run, increasing both the build times and the time it takes to run the unit tests.
>
> - Jonathan M Davis

You can just enable the unittests for a single instance that you know for sure that it will be used. For example:
1) https://github.com/dlang/phobos/blob/v2.071.2-b3/std/experimental/ndslice/slice.d#L808

2) https://github.com/dlang/phobos/blob/v2.071.2-b3/std/experimental/ndslice/slice.d#L947
September 03, 2016
On Saturday, September 03, 2016 16:56:08 ZombineDev via Digitalmars-d wrote:
> You can just enable the unittests for a single instance that you
> know for sure that it will be used. For example:
> 1)
> https://github.com/dlang/phobos/blob/v2.071.2-b3/std/experimental/ndslice/sl
> ice.d#L808
>
> 2) https://github.com/dlang/phobos/blob/v2.071.2-b3/std/experimental/ndslice/sl ice.d#L947

So, in order to avoid having the unit tests compiled into the code of anyone who uses your template, you have to create a special version identifier that you use with your unit test build and your documentation build, and then you version the tests within the template with that version. And in order to avoid having the unit test compiled into every instantiation of that template during your test, you have to also put it in a static if for a specific instantiation? Sure, that's feasible, but that's getting _ugly_.

- Jonathan M Davis

September 04, 2016
On Saturday, 3 September 2016 at 17:16:24 UTC, Jonathan M Davis wrote:
> On Saturday, September 03, 2016 16:56:08 ZombineDev via Digitalmars-d wrote:
>> You can just enable the unittests for a single instance that you
>> know for sure that it will be used. For example:
>> 1)
>> https://github.com/dlang/phobos/blob/v2.071.2-b3/std/experimental/ndslice/sl
>> ice.d#L808
>>
>> 2) https://github.com/dlang/phobos/blob/v2.071.2-b3/std/experimental/ndslice/sl ice.d#L947
>
> So, in order to avoid having the unit tests compiled into the code of anyone who uses your template, you have to create a special version identifier that you use with your unit test build and your documentation build, and then you version the tests within the template with that version. And in order to avoid having the unit test compiled into every instantiation of that template during your test, you have to also put it in a static if for a specific instantiation? Sure, that's feasible, but that's getting _ugly_.
>
> - Jonathan M Davis

I like and support your DIP about static unittests, I was just pointing out that there are workarounds, mainly for people like Manu, who need ways to get their job done now.

Personally, I think that putting a single `static if (doUnittests)` before each unittest is not too much, considering how flexible the doUnittests condition can be (e.g. check if ddoc or unittest build, check template parameters, etc.).