www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Voting for std.experimental.testing

reply Robert burner Schadek <rburners gmail.com> writes:
This is the voting thread for inclusion of 
std.experimental.testing into phobos.

PR: https://github.com/D-Programming-Language/phobos/pull/3207
Dub: http://code.dlang.org/packages/unit-threaded
Doc: See CyberShadow/DAutoTest for up-to-date documentation build
Formal Review Thread: 
http://forum.dlang.org/post/stbdckpfsysjtppldmry forum.dlang.org
Previous Thread: 
http://forum.dlang.org/post/uzocokshugchescbawlj forum.dlang.org

Please respond to this post with a comment starting with a single 
"Yes"/"No" and optional explanation after that. Criteria you are 
expected to evaluate as part of "Yes":

- is this functionality needed in Phobos
- is basic design sound (some breaking changes are OK for 
std.experimental but not full redesign)

As usual, anyone can vote, but opinions of Phobos developers hold 
more value.

Voting ends in 2 weeks, on Oktober 22.
Oct 08 2015
next sibling parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 08/10/15 9:21 PM, Robert burner Schadek wrote:

 This is the voting thread for inclusion of std.experimental.testing into
 phobos.

 PR: https://github.com/D-Programming-Language/phobos/pull/3207
 Dub: http://code.dlang.org/packages/unit-threaded
 Doc: See CyberShadow/DAutoTest for up-to-date documentation build
 Formal Review Thread:
 http://forum.dlang.org/post/stbdckpfsysjtppldmry forum.dlang.org
 Previous Thread:
 http://forum.dlang.org/post/uzocokshugchescbawlj forum.dlang.org

 Please respond to this post with a comment starting with a single
 "Yes"/"No" and optional explanation after that. Criteria you are
 expected to evaluate as part of "Yes":

 - is this functionality needed in Phobos
 - is basic design sound (some breaking changes are OK for
 std.experimental but not full redesign)

 As usual, anyone can vote, but opinions of Phobos developers hold more
 value.

 Voting ends in 2 weeks, on Oktober 22.


Yes, but:

There is no way that could conflict with serializers/vibe.d's 
definitions. Let alone ORM's.

+struct name
+{
+    string value;
+}

std/experimental/testing/gen_ut_main_mixin.d
Why is it not package(std.experimental.testing) and more importantly not 
called internal? There is nothing there which the user should be touching.

Alright seriously?
+    /**
+     * Generate green coloured output on POSIX systems
+     */
+    string green(in string msg)  safe pure const
+    {
+        return escCode(Color.green) ~ msg ~ escCode(Color.cancel);
+    }

Somebody fix please: https://github.com/robik/consoled
Irk I don't like it being done like this. I want it done right or not at 
all pretty much.

Further thought about UDA's especially those with high conflict 
potential. Perhaps they should instead be moved out into e.g. 
std.stdudas. That way it is not locked into e.g. testing while being 
reusable.
Oct 08 2015
next sibling parent reply Per =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Thursday, 8 October 2015 at 08:52:04 UTC, Rikki Cattermole 
wrote:

 Alright seriously?
 +    /**
 +     * Generate green coloured output on POSIX systems
 +     */
 +    string green(in string msg)  safe pure const
 +    {
 +        return escCode(Color.green) ~ msg ~ 
 escCode(Color.cancel);
 +    }

 Somebody fix please: https://github.com/robik/consoled
 Irk I don't like it being done like this. I want it done right 
 or not at all pretty much.

 Further thought about UDA's especially those with high conflict 
 potential. Perhaps they should instead be moved out into e.g. 
 std.stdudas. That way it is not locked into e.g. testing while 
 being reusable.


What about using compile-time-only struct-wrappers or UDA's for 
Visual attributes such as color, boldness, etc? For a showcase 
see my pretty.d (which I plan to propose to put in 
std.experimental.pretty):

https://github.com/nordlow/justd/blob/master/pretty.d

It has bitrotten a bit lately but I'll fix it today if you want 
to have live showcase.
Oct 08 2015
parent Rikki Cattermole <alphaglosined gmail.com> writes:
On 09/10/15 1:12 AM, Per Nordlöw wrote:

 On Thursday, 8 October 2015 at 08:52:04 UTC, Rikki Cattermole wrote:

 Alright seriously?
 +    /**
 +     * Generate green coloured output on POSIX systems
 +     */
 +    string green(in string msg)  safe pure const
 +    {
 +        return escCode(Color.green) ~ msg ~ escCode(Color.cancel);
 +    }

 Somebody fix please: https://github.com/robik/consoled
 Irk I don't like it being done like this. I want it done right or not
 at all pretty much.

 Further thought about UDA's especially those with high conflict
 potential. Perhaps they should instead be moved out into e.g.
 std.stdudas. That way it is not locked into e.g. testing while being
 reusable.


 What about using compile-time-only struct-wrappers or UDA's for Visual
 attributes such as color, boldness, etc? For a showcase see my pretty.d
 (which I plan to propose to put in std.experimental.pretty):

 https://github.com/nordlow/justd/blob/master/pretty.d

 It has bitrotten a bit lately but I'll fix it today if you want to have
 live showcase.


I don't terribly care about that :) I just hate seeing something that we 
should have being done privately and in Phobos.
Oct 08 2015
prev sibling parent Atila Neves <atila.neves gmail.com> writes:
On Thursday, 8 October 2015 at 08:52:04 UTC, Rikki Cattermole 
wrote:

 On 08/10/15 9:21 PM, Robert burner Schadek wrote:

 [...]


 Yes, but:

 There is no way that could conflict with serializers/vibe.d's 
 definitions. Let alone ORM's.


?



 std/experimental/testing/gen_ut_main_mixin.d
 Why is it not package(std.experimental.testing) and more 
 importantly not called internal? There is nothing there which 
 the user should be touching.


This is supposed to be used by the user.



 Alright seriously?
 +    /**
 +     * Generate green coloured output on POSIX systems
 +     */
 +    string green(in string msg)  safe pure const
 +    {
 +        return escCode(Color.green) ~ msg ~ 
 escCode(Color.cancel);
 +    }

 Somebody fix please: https://github.com/robik/consoled


Get it into Phobos and I'll gladly use it.


 Further thought about UDA's especially those with high conflict 
 potential. Perhaps they should instead be moved out into e.g. 
 std.stdudas. That way it is not locked into e.g. testing while 
 being reusable.


If there's a conflict, that's what the package/module system is 
for:

 std.experimental.testing.name unittest { ... }

Atila
Oct 08 2015
prev sibling next sibling parent Per =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Thursday, 8 October 2015 at 08:21:58 UTC, Robert burner 
Schadek wrote:

 This is the voting thread for inclusion of 
 std.experimental.testing into phobos.
 Voting ends in 2 weeks, on Oktober 22.


Sorry for being late with this but I added two comments in the PR:

One more important:

https://github.com/D-Programming-Language/phobos/pull/3207/files#r41494484

One less important:

https://github.com/D-Programming-Language/phobos/pull/3207/files#r41494229
Oct 08 2015
prev sibling next sibling parent Orfeo <orfeo.davia gmail.com> writes:
On Thursday, 8 October 2015 at 08:21:58 UTC, Robert burner 
Schadek wrote:

 Please respond to this post with a comment starting with a 
 single "Yes"/"No" and optional explanation after that. Criteria 
 you are expected to evaluate as part of "Yes":


Yes
Oct 08 2015
prev sibling next sibling parent Dicebot <public dicebot.lv> writes:
No.

As I explained before I don't like proposed API and prefer to 
wait until language enables cleaner implementation as opposed to 
making this standard.
Oct 09 2015
prev sibling next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, 8 October 2015 at 08:21:58 UTC, Robert burner 
Schadek wrote:

 This is the voting thread for inclusion of 
 std.experimental.testing into phobos.

 PR: https://github.com/D-Programming-Language/phobos/pull/3207
 Dub: http://code.dlang.org/packages/unit-threaded
 Doc: See CyberShadow/DAutoTest for up-to-date documentation 
 build
 Formal Review Thread: 
 http://forum.dlang.org/post/stbdckpfsysjtppldmry forum.dlang.org
 Previous Thread: 
 http://forum.dlang.org/post/uzocokshugchescbawlj forum.dlang.org

 Please respond to this post with a comment starting with a 
 single "Yes"/"No" and optional explanation after that. Criteria 
 you are expected to evaluate as part of "Yes":

 - is this functionality needed in Phobos
 - is basic design sound (some breaking changes are OK for 
 std.experimental but not full redesign)

 As usual, anyone can vote, but opinions of Phobos developers 
 hold more value.

 Voting ends in 2 weeks, on Oktober 22.


No.

Given that unittest blocks already have function names (it's just 
that you can't name them directly), it's kind of ridiculous IMHO 
to use attributes for that. We should just make it possible to do 
something like unittest(funcName) {} to name the unittest blocks.

I also think that most of the functionality that 
std.expermental.testing provides is completely unnecessary - e.g. 
std.experimental.testing.should contains a bunch of functions 
that can almost always easily be done by simply using an 
assertion, and my experience with templated assertion functions 
like this is that they bloat things considerably, increasing 
compilation time and the amount of memory required. They're not 
worth the cost unless they significantly reduce the amount of 
code required to write a test (which these don't). If all that's 
needed is the ability to print out more information on failure, 
that's easily added by adding some writeln statements and 
rerunning the tests (on top of the fact that there's currently a 
DIP for how to improve assertion messages to solve that problem; 
and if that gets approved, then these functions make even less 
sense).

Being able to parallelize unit tests would be nice, but we should 
be able to add that to the existing test runner in druntime 
easily enough. And I definitely don't like the idea of adding 
another test runner into Phobos to replace the one in druntime. 
If there are some relatively simple changes that can be made to 
the one in druntime to get the improvements that we want, then 
great. We should add them there. But I don't think that we should 
be adding stuff to Phobos to replace the unit test stuff in 
druntime, and I think that if someone wants to have something 
that's definitely more complicated or works significantly 
differently from the unit test framework in druntime, then that 
should be handle by 3rd party libs and not included as part of 
the official libraries.

- Jonathan M Davis
Oct 11 2015
parent Atila Neves <atila.neves gmail.com> writes:
On Sunday, 11 October 2015 at 21:56:21 UTC, Jonathan M Davis 
wrote:

 On Thursday, 8 October 2015 at 08:21:58 UTC, Robert burner 
 Schadek wrote:

 This is the voting thread for inclusion of 
 std.experimental.testing into phobos.

 PR: https://github.com/D-Programming-Language/phobos/pull/3207
 Dub: http://code.dlang.org/packages/unit-threaded
 Doc: See CyberShadow/DAutoTest for up-to-date documentation 
 build
 Formal Review Thread: 
 http://forum.dlang.org/post/stbdckpfsysjtppldmry forum.dlang.org
 Previous Thread: 
 http://forum.dlang.org/post/uzocokshugchescbawlj forum.dlang.org

 Please respond to this post with a comment starting with a 
 single "Yes"/"No" and optional explanation after that. 
 Criteria you are expected to evaluate as part of "Yes":

 - is this functionality needed in Phobos
 - is basic design sound (some breaking changes are OK for 
 std.experimental but not full redesign)

 As usual, anyone can vote, but opinions of Phobos developers 
 hold more value.

 Voting ends in 2 weeks, on Oktober 22.


 No.

 Given that unittest blocks already have function names (it's 
 just that you can't name them directly), it's kind of 
 ridiculous IMHO to use attributes for that. We should just make 
 it possible to do something like unittest(funcName) {} to name 
 the unittest blocks.


That would indeed be good, and has been proposed before. 
Unfortunately, we don't have that now and the idea of doing it as 
a library instead (and the fact that it was considered a good 
idea) came from Andrei.
As for the names they have now, they're pretty much useless for 
anything whatsoever.


 I also think that most of the functionality that 
 std.expermental.testing provides is completely unnecessary - 
 e.g. std.experimental.testing.should contains a bunch of 
 functions that can almost always easily be done by simply using 
 an assertion, and my experience with templated assertion 
 functions like this is that they bloat things considerably, 
 increasing compilation time and the amount of memory required.


We'd have to measure to be sure of how much worse things get. If 
at all.


 They're not worth the cost unless they significantly reduce the 
 amount of code required to write a test (which these don't). If 
 all that's needed is the ability to print out more information 
 on failure, that's easily added by adding some writeln 
 statements and rerunning the tests (on top of the fact that 
 there's currently a DIP for how to improve assertion messages 
 to solve that problem; and if that gets approved, then these 
 functions make even less sense).


Suggesting using `writeln` for failing tests is like telling 
someone who's complaining about debugger support to "just use 
printf". And then we wonder why we get complaints about our tools.

As for the cost, reasonable people may beg to differ.


 Being able to parallelize unit tests would be nice, but we 
 should be able to add that to the existing test runner in 
 druntime easily enough. And I definitely don't like the idea of 
 adding another test runner into Phobos to replace the one in 
 druntime.


Why not? The druntime one is horrible.



 If there are some relatively simple changes that can be made to 
 the one in druntime to get the improvements that we want, then 
 great. We should add them there. But I don't think that we 
 should be adding stuff to Phobos to replace the unit test stuff 
 in druntime, and I think that if someone wants to have 
 something that's definitely more complicated or works 
 significantly differently from the unit test framework in 
 druntime, then that should be handle by 3rd party libs and not 
 included as part of the official libraries.


Fair enough. I don't mind using my own library, but as explained 
above, there was a suggestion for making this a part of Phobos 
and I included all the functionality I had, which not 
coincidentally is exactly all the functionality I wanted.

As this has been rejected, I'm already thinking of smaller 
simpler steps that could be made to make unit testing in D better 
than it is now.

Atila
Oct 26 2015
prev sibling next sibling parent reply stewart <growlercab gmail.com> writes:
On Thursday, 8 October 2015 at 08:21:58 UTC, Robert burner 
Schadek wrote:

 This is the voting thread for inclusion of 
 std.experimental.testing into phobos.

 PR: https://github.com/D-Programming-Language/phobos/pull/3207
 Dub: http://code.dlang.org/packages/unit-threaded
 Doc: See CyberShadow/DAutoTest for up-to-date documentation 
 build
 Formal Review Thread: 
 http://forum.dlang.org/post/stbdckpfsysjtppldmry forum.dlang.org
 Previous Thread: 
 http://forum.dlang.org/post/uzocokshugchescbawlj forum.dlang.org

 Please respond to this post with a comment starting with a 
 single "Yes"/"No" and optional explanation after that. Criteria 
 you are expected to evaluate as part of "Yes":

 - is this functionality needed in Phobos
 - is basic design sound (some breaking changes are OK for 
 std.experimental but not full redesign)

 As usual, anyone can vote, but opinions of Phobos developers 
 hold more value.

 Voting ends in 2 weeks, on Oktober 22.



No

a) Doesn't add enough value; I feel DIP83 (and also DIP82) are 
more important and this library should wait then build on DIP83.

b) There'd be more bang for buck if we had good backtrace support 
on all platforms when asserts fire.

c) Implements orthogonal functionality, as per Rikki's comments, 
coupling a testing framework with implementation of coloured 
output and report generation. Pretty output should wait until 
available from other modules in Phobos.

d) API is clunky because lower-level components are not available.

e) Named unittests are nice but unless something like 
unittest(name) is accepted it isn't worth it.

__FUNCITON__ already provides a UID that includes line number 
information, which is 100x more useful than a name made up by Joe 
Developer. If we get b) above then unittest names really become 
unnecessary IMO.

bye,
stew
Oct 11 2015
parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 12/10/15 3:39 PM, stewart wrote:

 On Thursday, 8 October 2015 at 08:21:58 UTC, Robert burner Schadek wrote:

 This is the voting thread for inclusion of std.experimental.testing
 into phobos.

 PR: https://github.com/D-Programming-Language/phobos/pull/3207
 Dub: http://code.dlang.org/packages/unit-threaded
 Doc: See CyberShadow/DAutoTest for up-to-date documentation build
 Formal Review Thread:
 http://forum.dlang.org/post/stbdckpfsysjtppldmry forum.dlang.org
 Previous Thread:
 http://forum.dlang.org/post/uzocokshugchescbawlj forum.dlang.org

 Please respond to this post with a comment starting with a single
 "Yes"/"No" and optional explanation after that. Criteria you are
 expected to evaluate as part of "Yes":

 - is this functionality needed in Phobos
 - is basic design sound (some breaking changes are OK for
 std.experimental but not full redesign)

 As usual, anyone can vote, but opinions of Phobos developers hold more
 value.

 Voting ends in 2 weeks, on Oktober 22.



 No

 a) Doesn't add enough value; I feel DIP83 (and also DIP82) are more
 important and this library should wait then build on DIP83.

 b) There'd be more bang for buck if we had good backtrace support on all
 platforms when asserts fire.

 c) Implements orthogonal functionality, as per Rikki's comments,
 coupling a testing framework with implementation of coloured output and
 report generation. Pretty output should wait until available from other
 modules in Phobos.


That's not quite what I meant. It doesn't need to wait. As long as the 
implementation stays private and is not exposed in anyway, it is fine.
We can rewrite it to use new functionality of Phobos at a later point.
We just need to acknowledge that it will need to happen at this time.


 d) API is clunky because lower-level components are not available.

 e) Named unittests are nice but unless something like unittest(name) is
 accepted it isn't worth it.

 __FUNCITON__ already provides a UID that includes line number
 information, which is 100x more useful than a name made up by Joe
 Developer. If we get b) above then unittest names really become
 unnecessary IMO.

 bye,
 stew
Oct 11 2015
next sibling parent reply stewart <growlercab gmail.com> writes:
On Monday, 12 October 2015 at 02:50:37 UTC, Rikki Cattermole 
wrote:

[snip]

 That's not quite what I meant. It doesn't need to wait. As long 
 as the implementation stays private and is not exposed in 
 anyway, it is fine.
 We can rewrite it to use new functionality of Phobos at a later 
 point.
 We just need to acknowledge that it will need to happen at this


[snip]

Sorry for misquoting :)

I would still argue the additional maintenance and future work 
isn't worth it for coloured text. Plain text output is fine until 
Phobos has better colour support.

bye,
stewart
Oct 11 2015
parent Rikki Cattermole <alphaglosined gmail.com> writes:
On 12/10/15 4:11 PM, stewart wrote:

 On Monday, 12 October 2015 at 02:50:37 UTC, Rikki Cattermole wrote:

 [snip]

 That's not quite what I meant. It doesn't need to wait. As long as the
 implementation stays private and is not exposed in anyway, it is fine.
 We can rewrite it to use new functionality of Phobos at a later point.
 We just need to acknowledge that it will need to happen at this


 [snip]

 Sorry for misquoting :)

 I would still argue the additional maintenance and future work isn't
 worth it for coloured text. Plain text output is fine until Phobos has
 better colour support.

 bye,
 stewart


I'll leave that up to the author to decide :)
And anyway while its in std.experimental, the goal is to get the 
implementation up to what the abstraction is meant to be. So it doesn't 
have to be perfect, but it does need to be a good show case which this is.
Oct 11 2015
prev sibling parent Atila Neves <atila.neves gmail.com> writes:
On Monday, 12 October 2015 at 02:50:37 UTC, Rikki Cattermole 
wrote:

 c) Implements orthogonal functionality, as per Rikki's 
 comments,
 coupling a testing framework with implementation of coloured 
 output and
 report generation. Pretty output should wait until available 
 from other
 modules in Phobos.


 That's not quite what I meant. It doesn't need to wait. As long 
 as the implementation stays private and is not exposed in 
 anyway, it is fine.
 We can rewrite it to use new functionality of Phobos at a later 
 point.
 We just need to acknowledge that it will need to happen at this 
 time.


Those functions are `package`, not `public`.

Atila
Oct 12 2015
prev sibling next sibling parent Robert burner Schadek <rburners gmail.com> writes:
One week left!
Oct 15 2015
prev sibling parent Robert burner Schadek <rburners gmail.com> writes:
Voting Result:

Yes:             1
Conditional Yes: 1
No:              3

Therefore, std.experimental.testing is rejected in the current 
state.
Oct 23 2015