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.
