Quit running foreign unittests >_<

[Ivan Kazmenko via Digitalmars-d]

On Monday, 27 April 2015 at 15:29:05 UTC, Steven Schveighoffer 
wrote:
> On 4/27/15 10:30 AM, Ivan Kazmenko wrote:
>> On Monday, 27 April 2015 at 11:30:04 UTC, Steven Schveighoffer 
>> wrote:
>>> The problem is as follows:
>>>
>>> 1. Unit tests for some library are written for that library. 
>>> They are
>>> written to run tests during unit tests of that library only 
>>> (possibly
>>> with certain requirements of environment, including build 
>>> lines, or
>>> expectations of system resource availability).
>>
>> By the way, a unittest-related issue still stands in DMD 
>> 2.067.1 for
>> RedBlackTree:
>> https://issues.dlang.org/show_bug.cgi?id=12246
>>
>> A similar matter got resolved quickly for BinaryHeap:
>> https://issues.dlang.org/show_bug.cgi?id=12245
>>
>> BinaryHeap's case was handled by putting the container's 
>> unittests into
>> "debug(BinaryHeap)".
>>
>> RedBlackTree's case is controlled via "debug(RBDoChecks)" 
>> (formerly
>> "version(RBDoChecks)").  The difference is the presence of a
>> "version(unittest) debug = RBDoChecks;" line.  This looks 
>> inconsistent.
>>
>> For RedBlackTree, compile the following with or without 
>> -unittest option
>> and run for a visible difference in speed (runs momentarily or 
>> for a few
>> seconds):
>> -----
>> import std.container;
>> void main() {
>>     auto t = redBlackTree!int;
>>     foreach (i; 0..3000) t.insert(i);
>> }
>> -----
>>
>> Ivan Kazmenko.
>
> It's an anecdotal fix. I remember arguing over the change to 
> debug, that was done for purity (pure functions need debug mode 
> to print out something, which rbdochecks will do if there is an 
> issue), but I can't find the conversation.
>
> But someone else will complain that when they try to debug 
> their code, adding -debug to the command line debugs RBTree's 
> algorithm (similarly to how they complained BinaryHeap was 
> doing this).

I am doing just that, right now =) .  But it's -unittest, not 
-debug, which triggers RBTree's expensive checks.

> This really should only EVER run during phobos unit tests.

A choice would be even better.  When a user's code is wrong, for 
example, a comparison function does not define a comparison in 
mathematical sense, the user might benefit from container's 
sanity checks.  From personal experience, when I implement my own 
sorted container and some complex logic using it, I do benefit 
from the container's checks sometimes.

> I don't know how to fix this properly without something like I 
> outlined above, or without doing some global 
> version(PhobosUnitTests) hack.
>
> In fact, I don't agree with the BinaryHeap change. At this 
> point, phobos unit tests are NOT testing the binary heap 
> structure. Sure that makes user code run faster, but at the 
> cost of never testing it even when it should be tested.
>
> -Steve

Right now, it is possible to test binary heap integrity with an 
explicit "-debug=BinaryHeap", and it is not tested by default.

And it is mandatory to test red-black tree integrity when running 
any unittests involving a red-black tree, as it is tested by 
default.

So, from a user's point of view, I like the binary heap situation 
better, since it gives me a choice with a reasonable default.

How Phobos' unittests should be handled is another matter which, 
in my opinion, should not take away choices, and reasonable 
defaults too, from the user.

To me, the proposed version(PhobosUnitTests) hack looks good for 
the particular case.  But I don't see whether such trick scales 
well for the whole ecosystem of libraries.

Ivan Kazmenko.