On Thursday, 21 March 2013 at 17:29:46 UTC, Jonathan M Davis wrote:
> Hmmm. I've been working on stuff which makes creating ranges to test range-
> based functions easier, but I've never thought about creating something to
> test conformance.
>
> - Jonathan M Davis

One of the things I've been trying to deploy is a trait that would help identify *why* a range is (statically) non-compliant.

For example, when I was still not 100% up to speed on what it takes for a range to conform to this and that, I'd forget something, and it would take a lot of effort to realize that "dang, my save is not a property, that's why my range is not a random access range".

I wrote some traits called things like "assertIsRandomAccessRange!R". Contrary to "assert(isRandomAccessRange!R)", my function would output: "Assertion failed: R is not a random access range because it does not provide the save property".

This was quite a helpful tool but...

...It was mostly just code duplication. It would have been an helpful feature, but I'm not sure the code maintenance it entailed in std.range would have made it worth it.

On Thu, Mar 21, 2013 at 07:52:32PM +0100, monarch_dodra wrote:
> On Thursday, 21 March 2013 at 17:29:46 UTC, Jonathan M Davis wrote:
> >Hmmm. I've been working on stuff which makes creating ranges to
> >test range-
> >based functions easier, but I've never thought about creating
> >something to
> >test conformance.
> >
> >- Jonathan M Davis
> 
> One of the things I've been trying to deploy is a trait that would help identify *why* a range is (statically) non-compliant.
> 
> For example, when I was still not 100% up to speed on what it takes for a range to conform to this and that, I'd forget something, and it would take a lot of effort to realize that "dang, my save is not a property, that's why my range is not a random access range".
> 
> I wrote some traits called things like "assertIsRandomAccessRange!R". Contrary to "assert(isRandomAccessRange!R)", my function would output: "Assertion failed: R is not a random access range because it does not provide the save property".
> 
> This was quite a helpful tool but...
> 
> ...It was mostly just code duplication. It would have been an helpful feature, but I'm not sure the code maintenance it entailed in std.range would have made it worth it.

I think this is worth doing if you already wrote the templates to do these checks.

Ranges are an extremely powerful concept in D, but they are also very obscure for a beginner, and the current error messages don't help. Improving the assertions' messages would be a big improvement, IMO.


T

-- 
People tell me I'm stubborn, but I refuse to accept it!

>> - Upon instantiating a new random access range, 'r.front == r[0]', and
>>   then after 'r.popFront()', 'r.front == r[1]', etc.
>
> This should be relatively easy to check.

Is that really the behaviour for RA ranges? I'd say that after calling
r.popFront(), then r.front == r[0].
The test could be:

assert(r has at least two elements);
auto nextOne = r[1];
r.popFront();
assert(r.front == nextOne);

> void testForwardRange(R, E)(R range, E[] content)
> {
>     // Deliberately not contraints, because this *is* a test, after all.
>     // *Or* maybe it just auto-detects range type and leaves it
>     // up to the user to check for "isForwardRange!R" or whatever.
>     static assert(isForwardRange!R);
>     static assert(is(ElementTypeOf!R == E));
>
>     static if(hasLength!R)
>         assert(range.length == content.length);
> 
>     foreach(i; 0...content.length)
>     {
>         assert(range.front == content[i]);
>         range.popFront();
>     }
>
>     // More tests
> }

Would it make sense to just name the function testRange and have it test conformance for any range primitives that the type supports? Your example already uses static "if(hasLenght!R)" for length, it may be a good idea to expand that.

On Thu, 21 Mar 2013 20:24:40 +0100
Philippe Sigaud <philippe.sigaud@gmail.com> wrote:

> >> - Upon instantiating a new random access range, 'r.front == r[0]', and then after 'r.popFront()', 'r.front == r[1]', etc.
> >
> > This should be relatively easy to check.
> 
> Is that really the behaviour for RA ranges? I'd say that after calling
> r.popFront(), then r.front == r[0].
> The test could be:
> 
> assert(r has at least two elements);
> auto nextOne = r[1];
> r.popFront();
> assert(r.front == nextOne);

You might be right.

See why such a tester would be helpful? ;)

On Thu, 21 Mar 2013 21:41:52 +0100
"jerro" <a@a.com> wrote:

> > void testForwardRange(R, E)(R range, E[] content)
> > {
> >     // Deliberately not contraints, because this *is* a test,
> > after all.
> >     // *Or* maybe it just auto-detects range type and leaves it
> >     // up to the user to check for "isForwardRange!R" or
> > whatever.
> >     static assert(isForwardRange!R);
> >     static assert(is(ElementTypeOf!R == E));
> >
> >     static if(hasLength!R)
> >         assert(range.length == content.length);
> > 
> >     foreach(i; 0...content.length)
> >     {
> >         assert(range.front == content[i]);
> >         range.popFront();
> >     }
> >
> >     // More tests
> > }
> 
> Would it make sense to just name the function testRange and have it test conformance for any range primitives that the type supports? Your example already uses static "if(hasLenght!R)" for length, it may be a good idea to expand that.

Possibly. After all, a person really should already be using "static assert(isBlahBlahRange!MyRange);" to make sure their range is the type they expect it to be anyway.

On Thursday, March 21, 2013 19:46:08 John Colvin wrote:
> On Thursday, 21 March 2013 at 18:19:00 UTC, Jonathan M Davis
> 
> wrote:
> > I could definitely see an argument for adopting the
> > policy of having popFront throw RangeError when the range is
> > empty.
> > 
> > - Jonathan M Davis
> 
> In release mode, assert expressions are removed. This would not be possible for throwing a RangeError, preventing a useful optimisation in well tested code.

A valid point as to why such a change in policy should not be made. Regardless, there's definitely no requirement right now that popFront throw RangeError when the range is empty. What Phobos normally does at this point is assert. Anything else would be a change that would have to be discussed, so specifically testing that a conformant range throws a RangeError from popFront when it's empty (as Nick was suggesting) would not be correct at this point.

- Jonathan M Davis

On Thu, 21 Mar 2013 21:55:10 -0400
"Jonathan M Davis" <jmdavisProg@gmx.com> wrote:

> Anything else would be a change that would
> have to be discussed, so specifically testing that a conformant range
> throws a RangeError from popFront when it's empty (as Nick was
> suggesting) would not be correct at this point.
> 

I wasn't suggesting that, merely trying to list a bunch of examples of testable things. Clearly I got a few of the examples wrong ;)

But that does kinda speak to the usefulness of having something standard that can be expected to actually do the *correct* tests, instead of all range authors just winging it on their own and potentially misunderstanding the rules.

>>  testing that a conformant range throws a RangeError from popFront when it's empty (as Nick was suggesting) would not be correct at this point

ok what about the following: should work regardless it's RangeError or an assert(0) in th eparticular implementation of a range.

----
bool isThrown(E)(lazy E expression){
        import std.exception;
        import core.exception : RangeError;
	try
		expression();
	catch(RangeError t)	return true;
	catch(Error t)	return true;
	return false;
}
unittest(){
    int[]x;
    assert(isThrown(x[1]));
}
----

On Friday, March 22, 2013 08:07:01 timotheecour wrote:
> >>  testing that a conformant range throws a RangeError from
> >> 
> >> popFront when it's empty (as Nick was suggesting) would not be
> >> correct at this point
> 
> ok what about the following: should work regardless it's RangeError or an assert(0) in th eparticular implementation of a range.
> 
> ----
> bool isThrown(E)(lazy E expression){
>          import std.exception;
>          import core.exception : RangeError;
> 	try
> 		expression();
> 	catch(RangeError t)	return true;
> 	catch(Error t)	return true;
> 	return false;
> }
> unittest(){
>      int[]x;
>      assert(isThrown(x[1]));
> }
> ----

Sure, you could do that, but I'd be completely against putting anything in Phobos to test ranges which tested anything which wasn't agreed upon as the correct way to do it. That being the case, it would have to be agreed upon whether assertions should be used or whether RangeError should be used (or that using either was valid, though that's a bad idea IMHO).

As for the implementation, it would make more sense to simply do

assertThrown!Error(x[1]);

if you don't care which type of Error is thrown, and if you do care but are allowing both (since you can only give one type to assertThrown), then you could use

auto e = collectException!Error(x[1]);
assert((cast(AssertError)e) !is null || (cast(RangeError)) !is null);

But if you're writing try-catch blocks in unit tests to test exceptions being thrown, odds are that you're not taking proper advantage of std.exception.

- Jonathan M Davis