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