www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - version(assert) and Phobos release unittest build

reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
Hello all,

Some unittests I've been writing for Phobos involve using 
assertThrown!AssertError to check that assert failures are indeed triggered 
where they should be.

Surprise surprise, this fails on the release unittest build, because of course 
the assertion statements are stripped out.  I assumed that using
version(assert) 
{ ... } to surround the assertThrown statement would fix this, but it doesn't
-- 
the assertThrown keeps being called and keeps claiming that no AssertError was 
thrown.

I guess I could do version(release) {} { assertThrown(...); } but that seems 
kind of ugly.  Any alternatives?

Thanks & best wishes,

     -- Joe
Nov 28 2013
parent reply "Dicebot" <public dicebot.lv> writes:
I think using assert for unittests is a mistake if you ever going 
to test release builds. `std.exception.enforce` or something 
similar is a better match.
Nov 28 2013
next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 16:48, Dicebot wrote:

 I think using assert for unittests is a mistake if you ever going to test
 release builds. `std.exception.enforce` or something similar is a better match.


I'm testing an in-contract, as it happens.

The point is that code inside version(assert) should only be included if assert 
expressions are enabled, as described at http://dlang.org/version.html.  But 
even with version(assert) conditionals in place, the assertThrown is still
called.
Nov 28 2013
prev sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 16:58, Joseph Rushton Wakeling wrote:

 But even with version(assert) conditionals in place, the assertThrown is still
called.


Come to that, even if I do:

     version(release) {} else { assertThrown(...); }

then the assertThrown is _still_ called even in release mode.
Nov 28 2013
parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 28 November 2013 at 16:01:06 UTC, Joseph Rushton 
Wakeling wrote:

 On 28/11/13 16:58, Joseph Rushton Wakeling wrote:

 But even with version(assert) conditionals in place, the 
 assertThrown is still called.


 Come to that, even if I do:

     version(release) {} else { assertThrown(...); }

 then the assertThrown is _still_ called even in release mode.


Oh, oops, I thought your issue is exactly other way around, 
sorry. That looks like an issue in assertThrown implementation. 
probably worth a bugzilla entry.
Nov 28 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 17:04, Dicebot wrote:

 Oh, oops, I thought your issue is exactly other way around, sorry. That looks
 like an issue in assertThrown implementation. probably worth a bugzilla entry.


It's a bit mysterious.  Consider this minimal example:

     unittest
     {
         import std.exception, core.exception;
         assertThrown!AssertError(assert(false));
     }

... which should be fine in non-release mode and (potentially) problematic in 
release mode.  In fact it makes no difference whether you call:

     rdmd -main -unittest -cov assert.d

or

     rdmd -main -unittest -cov -release assert.d

... either works without complaint, which is not what you'd expect: you'd
expect 
assertThrown to report an error in the case of -release mode.

Now, replace the assert(false) in the above with assert(true).  This time, in 
both cases, you get:

     core.exception.AssertError assert.d(4): assertThrown failed: No
AssertError 
was thrown.

... which is kind of in line with expectations, I suppose.

Now, wrap the line in question in version(assert) { ... } -- still fails in
both 
cases.

Now wrap it in version(release) {} else { ... } -- again, still fails with both 
build/run options.

It's this last one which is really mysterious to me, because it suggests that 
neither version(assert) nor version(release) is being respected properly.
Nov 28 2013
parent reply "Dicebot" <public dicebot.lv> writes:
assert(false) is special and never removed, don't use it for 
testing assert code : 
http://dlang.org/expression.html#AssertExpression
Nov 28 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 17:20, Dicebot wrote:

 assert(false) is special and never removed, don't use it for testing assert
code
 : http://dlang.org/expression.html#AssertExpression


Seems to make no difference if I replace it with

     int i = 1;
     assertThrown!AssertError(assert(i == 3));

... and tweak the assert accordingly (i.e. replace it with assert(i == 1) where 
previously I used assert(true)).
Nov 28 2013
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
I have quickly tested it and looks like asserts are not removed 
from unittest blocks in release builds, only from normal code. 
Which does not seem to be documented on dlang.org but was 
probably introduced exactly to prevent existing tests from 
breaking in release mode. I have just learned something new :)
Nov 28 2013
next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 17:40, Dicebot wrote:

 I have quickly tested it and looks like asserts are not removed from unittest
 blocks in release builds, only from normal code. Which does not seem to be
 documented on dlang.org but was probably introduced exactly to prevent existing
 tests from breaking in release mode. I have just learned something new :)


Snap, I just came to the same conclusion after finding that version(assert) 
seemed to be working fine in a main() function but that it continued to be 
ignored inside a unittest scope :-)
Nov 28 2013
prev sibling next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 17:40, Dicebot wrote:

 I have quickly tested it and looks like asserts are not removed from unittest
 blocks in release builds, only from normal code. Which does not seem to be
 documented on dlang.org but was probably introduced exactly to prevent existing
 tests from breaking in release mode. I have just learned something new :)


Both version(assert) and version(release) are ignored inside unittest blocks. 
Consider the attached code: running with rdmd -unittest -release results in
output:

     I don't respect release mode inside unittest blocks
     Release mode is disabled.

Which is interesting but irritating to say the least, because it means things 
like testing contracts are more difficult to unittest effectively.

OK, OK, I could do

     version(assert)
     {
         unittest { ... }
     }

... but that makes for an annoying separation out of unittests.
Nov 28 2013
parent reply "Dicebot" <public dicebot.lv> writes:
version(assert) is negated by -unittest at all actually:

import std.stdio;

void main()
{
     version ( assert )
     {
         writeln("errr");
     }
}

$ rdmd -release test.d
$ rdmd -release -unittest test.d
errr

That is so bad >_<
Nov 28 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 17:59, Dicebot wrote:

 version(assert) is negated by -unittest at all actually:

 import std.stdio;

 void main()
 {
      version ( assert )
      {
          writeln("errr");
      }
 }

 $ rdmd -release test.d
 $ rdmd -release -unittest test.d
 errr

 That is so bad >_<


Aaaaaack!! :-\

And to add to the confusion and inconsistency, it _does_ still strip in- and 
out-contracts of functions -- see e.g.:

     import std.stdio;

     void foo(int i)
     in
     {
         writeln("In contract enabled.");
     }
     out
     {
         writeln("Out contract enabled.");
     }
     body
     {
         writeln("This is the body of foo.");
     }

     void main()
     {
         foo(4);
     }

which will output all 3 messages in normal mode, but only the "body" one when 
compiled with -release.

This is the reason for my original help request -- an in-contract was
triggering 
an AssertError in regular unittests, but not in release-mode unittests.
Nov 28 2013
parent reply "Dicebot" <public dicebot.lv> writes:
I don't know what is the proper way to handle this but current 
situation is clearly worth a bugzilla entry. It is just weird.
Nov 28 2013
parent reply "Gary Willoughby" <dev nomad.so> writes:
On Thursday, 28 November 2013 at 17:22:20 UTC, Dicebot wrote:

 I don't know what is the proper way to handle this but current 
 situation is clearly worth a bugzilla entry. It is just weird.


Maybe this is intended behaviour to handle testing AssertErrors 
in unit tests? How else could you test them.
Nov 28 2013
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 28 November 2013 at 17:28:56 UTC, Gary Willoughby 
wrote:

 On Thursday, 28 November 2013 at 17:22:20 UTC, Dicebot wrote:

 I don't know what is the proper way to handle this but current 
 situation is clearly worth a bugzilla entry. It is just weird.


 Maybe this is intended behaviour to handle testing AssertErrors 
 in unit tests? How else could you test them.


It is tricky.

a) by not using asserts
b) by compiling unittest blocks in separate pass from main app

(a) breaks many existing tests in release, (b) does not fit well 
into existing compilation model but can be worth considering

There is also (c) of course : document the fact that -unittest 
overrides -release with big red words everywhere :)

I don't know what is good way to handle this. I know for sure 
that current situation is damn obscure though.
Nov 28 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 19:01, Dicebot wrote:

 On Thursday, 28 November 2013 at 17:28:56 UTC, Gary Willoughby wrote:

 On Thursday, 28 November 2013 at 17:22:20 UTC, Dicebot wrote:

 I don't know what is the proper way to handle this but current situation is
 clearly worth a bugzilla entry. It is just weird.


 Maybe this is intended behaviour to handle testing AssertErrors in unit tests?
 How else could you test them.


 It is tricky.


The basic problem is this: if you compile something with -release -unittest, it 
will strip out all assert statements and contracts _in the main code_, while 
preserving assert statements etc. inside the unittest blocks.

Now, if you compile with -release -unittest, quite obviously you want the 
unittests to test the code _as compiled for release_, so the above is entirely 
correct behaviour.  It becomes problematic when you want to start testing 
behaviour that should take place in non-release mode but not in release mode
(or 
vice versa).

The _reason_ it's problematic is because inside the unittest blocks, the 
version(release) and version(assert) conditionals don't work as they do
outside. 
  So you can't use them inside the unittest { ... } blocks as conditionals for 
what tests get run.

Now, I can see the case here for version(assert) -- after all, asserts are 
enabled inside the unittest blocks -- but not so much for version(release). 
After all, if release mode is enabled, it's enabled.  (Presumably the actual 
fact is that release-mode-style compilation is also disabled inside the
unittest 
blocks.)


 a) by not using asserts
 b) by compiling unittest blocks in separate pass from main app

 (a) breaks many existing tests in release, (b) does not fit well into existing
 compilation model but can be worth considering

 There is also (c) of course : document the fact that -unittest overrides
 -release with big red words everywhere :)


I thought that it might also work to group together unittests that are 
conditional on version(release) or version(assert) or whatever, and put the 
unittest block _inside_ the version() { ... } scope.  But it turns out not.

I think it may actually be a bug in whether -release implies version(release). 
Try out the attached code.  (r)dmd -release release.d produces an executable 
that still reports that release mode is disabled.
Nov 28 2013
parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 28 November 2013 at 18:26:15 UTC, Joseph Rushton 
Wakeling wrote:

 I think it may actually be a bug in whether -release implies 
 version(release).
 Try out the attached code.  (r)dmd -release release.d produces 
 an executable
 that still reports that release mode is disabled.


There is no pre-defined version(release) in D according to spec.
Nov 28 2013
parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, November 28, 2013 19:53:48 Dicebot wrote:

 On Thursday, 28 November 2013 at 18:26:15 UTC, Joseph Rushton
 
 Wakeling wrote:

 I think it may actually be a bug in whether -release implies
 version(release).
 Try out the attached code.  (r)dmd -release release.d produces
 an executable
 that still reports that release mode is disabled.


 
 There is no pre-defined version(release) in D according to spec.


The closest you would get would be

version(assert) {}
else { /* release */ }

- Jonathan M Davis
Nov 28 2013
parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 28 November 2013 at 19:01:40 UTC, Jonathan M Davis 
wrote:

 The closest you would get would be

 version(assert) {}
 else { /* release */ }

 - Jonathan M Davis


Which does not work if you also have "-unittest" ;)
Nov 28 2013
next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 20:08, Dicebot wrote:

 Which does not work if you also have "-unittest" ;)


Plus, assert statements in the main body of code (not just the unittests) are 
enabled with -release if -unittest is turned on, but in- and out-contracts are 
still stripped.  An oversight, perhaps?

I don't know how I came up with the idea that version(release) was one of the 
predefined version conditions.  I do think it has some use, though.  Feature 
request? :-)
Nov 28 2013
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, November 28, 2013 20:08:34 Dicebot wrote:

 On Thursday, 28 November 2013 at 19:01:40 UTC, Jonathan M Davis
 
 wrote:

 The closest you would get would be
 
 version(assert) {}
 else { /* release */ }
 
 - Jonathan M Davis


 
 Which does not work if you also have "-unittest" ;)


Then you're out of luck for having a version block which is enabled with
-release if -unittest is also being used.

- Jonathan M Davis
Nov 28 2013
prev sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/11/13 20:25, Joseph Rushton Wakeling wrote:

 assert statements in the main body of code (not just the unittests) are
 enabled with -release if -unittest is turned on, but in- and out-contracts are
 still stripped.


After consideration, I think that this is the real issue here: it's not a 
problem that -unittest preserves assert() statements, but it _is_ a problem
that 
it doesn't preserve _all_ of them.

I've filed a bug report to this effect:
https://d.puremagic.com/issues/show_bug.cgi?id=11636
Nov 28 2013
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, November 28, 2013 18:28:55 Gary Willoughby wrote:

 On Thursday, 28 November 2013 at 17:22:20 UTC, Dicebot wrote:

 I don't know what is the proper way to handle this but current
 situation is clearly worth a bugzilla entry. It is just weird.


 
 Maybe this is intended behaviour to handle testing AssertErrors
 in unit tests? How else could you test them.


I wouldn't. I honestly don't think that testing contracts is an appropriate 
thing to do, much as I can see why you might want to do it. If you're passing 
something to a functon which violates that function's contract, then you're 
violating its contract, which is technically undefined behavior as far as DbC 
is concerned.

I think that testing that the correct exception is being thrown from a 
function makes sense, because in that case, it's something that the function 
is suppose to handle. But per DbC, a function should never have to deal with 
input which violates its contract and it's undefined behavior if it's given 
such input. So, you really need to make sure that you never give it incorrect 
input. The assertions in an in block are just insurance so that you can catch 
bugs when you screw it up.

But if you're absolutely determined to validate your contracts, I'd suggest 
simply creating another function which does the validation and then calling 
that from within the in block. e.g.

bool validateFoo(int i, int j)
{
    return i <= j;
}

auto foo(int i, int j)
in
{
    asser(validateFoo(i, j));
}
body
{
    ...
}

And using a validator function has the advantage that you can make it 
available to callers of your function if you want to so that they can validate 
the input.

Or if you want to, validateFoo could be purely for the contract and just 
assert rather than putting the assertion in the in block (though that's a 
_lot_ less efficient when unit testing, as thrown exceptions are very, very 
slow). But either way, by putting the actual validation code outside of the in 
block, you can test the validation code.

- Jonathan M Davis
Nov 28 2013
prev sibling next sibling parent "Gary Willoughby" <dev nomad.so> writes:
On Thursday, 28 November 2013 at 16:40:57 UTC, Dicebot wrote:

 I have quickly tested it and looks like asserts are not removed 
 from unittest blocks in release builds, only from normal code. 
 Which does not seem to be documented on dlang.org but was 
 probably introduced exactly to prevent existing tests from 
 breaking in release mode. I have just learned something new :)


I found this out too during the development of DUnit: 
https://github.com/nomad-software/dunit#notes

The asserts remain then the runtime just catches anything 
Throwable during unit test execution: 
https://github.com/D-Programming-Language/druntime/blob/master/src/core/runtime.d#L389-L419
Nov 28 2013
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, November 28, 2013 17:40:55 Dicebot wrote:

 I have quickly tested it and looks like asserts are not removed
 from unittest blocks in release builds, only from normal code.
 Which does not seem to be documented on dlang.org but was
 probably introduced exactly to prevent existing tests from
 breaking in release mode. I have just learned something new :)


Exactly. unittest block assertions are treated somewhat specially. They're 
always around with -unittest, but all the others (aside from the ones which 
can statically be determined to be false - e.g. assert(0)) get compiled out in 
-released. So, if you're relying on an assertion to go off outside of a -
unittes block in -release, you're in big trouble.

- Jonathan M Davis
Nov 28 2013
prev sibling parent "Dicebot" <public dicebot.lv> writes:
P.S. version(assert) does not seem to be working at all
Nov 28 2013