Named unittests: Introducing std.experimental.testing

[atilaneves]

Docs here

[rikkimax]

The most glaring obvious thing to me is definately comments. At Least have some Returns/Params for the public API.

[MartinNowak]

Nice, we just have a PR for asynclogger.

[atilaneves]

I hadn't even connected those this to asynclogger! I wrote this code more than a year ago, it's been the same for most of its life (I think, git might prove me wrong).

[MartinNowak]

Documentation preview.
https://dlang.dawg.eu/phobos-prerelease/std_experimental_testing.html

[atilaneves]

@rikkimax Could you give an example of where the comments aren't enough? I thought I'd added enough all over but clearly I was wrong.

@MartinNowak What would I have to do to update the documentation on that link?

[burner]

these four methods should be static.

[atilaneves]

They can't, _useEscCodes is an instance variable.

[burner]

commented to the wrong line! "yellow" and friends should be static as there is no way for the user AFAIK to change _escCodes

[atilaneves]

You're right, and I changed _escCodes to static immutable. However, yellow and friends all call escCode which in turn depends on _useEscCodes which is an instance variable.

[burner]

s/if(/if (/g

[burner]

Exceptions usually allow chaining. have a look at the exceptions in std.csv
file and line usually have default values of FILE and LINE

[atilaneves]

I'm not sure what you mean by the chaining here. I added defaults for file and line.

[burner]

usually the last (default) arg to a exception is Exception. aka Exception chaining!

[mihails-strasuns]

Main problem here is still requirement for explicit registration of modules. This does not feel robust enough to be proudly presented by Phobos. At the very least it needs to provide module for stub main generation that can be used as part of build system out of the box (via rdmd / dmd -run)

[mihails-strasuns]

I think all "verbose" I/O should be done via std.experimental.logger.

[atilaneves]

Yes, possibly. I wrote all of this long before std.experimental.logger existed, of course. I'm going to have to look into it.
It might just be that I need another logger backend for multithreaded applications first, though.

[burner]

the default logger is already thread-safe

[atilaneves]

So I finally took a look at std.experimental.logger. With @Dicebot's comment, I feel like the only thing to change is to remove writelnUt (I realised later it's pretty much useless, it has the same effect as using plain old writeln in a unit test). Or I could remove everything except the ANSI colours from the io.d file and completely rework the current IO to use std.experimental.logger. The only verbose IO supported by this library was when the -d option was used, and all that did was stop turning off stdio and stderr.

[mihails-strasuns]

Is there a reason why verbose shouldBeSomething was chosen over simpler and shorter operator-based versions? We use stuff like test!"is"(obj1, obj2) and it feels pretty good.

[atilaneves]

I was playing about with foo.should == bar and that works. Unfortunately, other operators don't so I abandoned it. I independently had the "string operator" idea as well, but wasn't sure how well it'd be received. AFAIC, I think I have no problem writing foo.should!">="(bar), but I'm not entirely sure. That exclamation mark would get confusing for should!"!=" for instance.
test!"is" should be checking for indentiy, not equality though, no?

[mihails-strasuns]

Yes, test!"op"(a, b) is similar to test(a op b) but with a more meaningful error message out of the box.

I have a very bad opinion of wordings like should, expect and all similar testing facilities that try to emulate human language. It only distracts from what tests do.

[atilaneves]

I guess that's a matter of taste. I prefer should to expect, and expect to test. I'd prefer it even more if all of these worked:

foo.should == 5;
foo.should != 4;
foo.should >= 3;
//etc

For some reason dmd is perfectly fine with an equality statement by itself but complains when it's !(a == b) (which is what a.should != b means) or opCmp. My workout in my experimental branch of unit-threaded is:

foo.should == 5;
foo.shouldNot == 4;

But then the other operators would have to be passed in as compile-time strings, so no consistency. In the end I kept things as they were for this PR.

[mihails-strasuns]

foo.should == 5 seeing that would be clear and straight "never use that testing library" sign for me

[atilaneves]

Why would foo.should == 5 be a bad sign for you? I don't mind shouldEqual or shouldBeEqual, but shouldBeGreaterThan is a bit of a mouthful. I really just want to use operators.

[atilaneves]

@Dicebot On the explicit registration, what would you propose beyond the runTestsMixin I added? Your idea of recursive compile-time reflection on modules doesn't currently work (I tried). The only other way I can think of and that was mentioned is to have a mixin in every module that wants to be tested, but that's intrusive and IMHO a worse solution than generating the file with the main function from the build system.

[mihails-strasuns]

All "good" solutions I can imagine require fixing language at least a bit :) For now I'd propose to provide separate module gentestroot.d or something like that which can be ran with shebang notation and generates the module with necessary mixins for each of D files found in provided folder path. That would at least allow to same script (which is part of stdlib) from different build systems.

[atilaneves]

@Dicebot Is your imagined gentestroot.d similar to this? That was my companion project to unit-threaded. I never wrote the main function manually, I'm a programmer and therefore lazy :)

[mihails-strasuns]

Why would foo.should == 5 be a bad sign for you?

As I have already mentioned, it is a very clear indicator that library author values "pretty" test definitions that increase visual complexity at no added value (which, in my opinion, is fundamental flaw of almost all modern test frameworks). I like D tests because they are very KISS and using both operator overloading and "spoken" language (I don't care what function "should" do, I simply test what it does!) smells as not KISS at all. Every abstraction needs to be justified with "what does that actually give you in return?".

Is your imagined gentestroot.d similar to this?

Yes, something very similar. Otherwise everyone will write the same thing of their own ;)

[jacob-carlborg]

We can't include an executable with phobos. Maybe dtools. What was settled was to have a file in phobos that users could use to generate their own ut executables with a main function

I guess that's acceptable as a start. But we need a proper tool for this sooner rather than later.

So, instead of a binary that runs the tests, there's a program that generates the program to run the tests, and then that program would be run in turn. I'll add documentation explaining properly.

So I need to run rdmd twice? Why can't the first file that is compiled and run also run the next one? You should be able to do something like this:

$ rdmd gen_ut_main.d foo/bar

Then gen_ut_main.d could be renamed to test.d and drop the .d extension in the command line, making it look like an action/subcommand for rdmd:

$ rdmd test foo/bar

The user should not need to know or care there's an intermediate file. In that case, you can generate a temporary file and skip the -f flag as well.

[atilaneves]

rdmd test would only work if run from $PHOBOS/std/experimental/testing. If anything I think what you're suggesting would be better accomplished by modifying rdmd to run that script from phobos. As for why generating and running being separate operations, I got so annoyed at dtest taking so long I stopped using it myself, but that was before I added a check to not overwrite the generated file if the file list hasn't changed. I'll have to think about it and play with the current state of affairs a bit.

I still think such a tool belongs in tools. I'm more inclined to remove gen_ut_main.d altogether and have just the mixin. But... the mixin could maybe also run the tests. Hmm.

[atilaneves]

Having thought a bit about gen_ut_main.d, here's my proposal: dtest should be part of the tools repository and leverage the code from the mixin in gen_ut_main_mixin and gen_ut_main.d would be deleted from this PR. Rationale:

1. We don't have runnable scripts / binaries in Phobos now
2. The binaries we ship that aren't the compiler are in tools
3. Having a runnable (even if just through rdmd) script in Phobos doesn't even help users much. On Arch Linux I'd have to rdmd /usr/include/dlang/dmd/std/experimental/testing/gen_ut_main.d which is a lot to type and read and not user-friendly in the slightest. Worse, it isn't even portable, if that command was in a Makefile it'd only build on Arch. It'd be better to package a separate dtest binary that worked as expected.
4. Even as a user myself, I'd rather have the functionality of writing the file that reflects on the modules to be tested in Phobos - it's easy to use just by mixing in to whatever file I want and including this as part of the build system
5. Mixing options for file generation and directories to search for with test-running options is messy.

I'm aware my proposal isn't ideal - but that's what's possible given the state of compile-time reflection in D now. @Dicebot, @jacob-carlborg : thoughts?

[jacob-carlborg]

I have given this some though as well. In an ideal world the build tool would handle this.

Alternative A:

1. The build tool compiles everything as a dynamic library
2. The test runner (dtest) loads the dynamic library and decides which tests should be run

The problems with this are:

1. Requires a build tool
2. Dynamic libraries are not supported on all platforms
3. UDA's are lost at runtime

It's possible to find the test methods at runtime by inspecting the symbol table (yes I know, it's a hack).

Alternative B:

1. The build tool compiles everything as a static library
2. The build tool generates a file with all the tests
3. The test runner is compiled together with the static library and the generated file

The problem with this is:

1. Requires a build tool

Alternative C:

1. The test runner (dtest) generates a temporary/hidden file with all tests
2. Compiles the generated file with the rest of the application and its tests
3. Runs the compiled tests

The problem with this are:

1. Need to compile twice

I guess Alternative C is what you have in mind. I'm not sure how dtest works now but you could always generate a file with all tests (regardless which ones should be executed) then at runtime select which test should be run. This way you could cache the intermediate file which only needs to be updated when tests are added or removed.

[atilaneves]

Libraries and compile-time reflection don't mix, so it's alternative C except that a hidden file isn't as useful as one that gets generated automatically by the build system / on demand when the list of modules to be compiled changes.
dtest just does what gen_ut_main.d here does, then runs the generated file.

[nordlow]

1. I strongly believe we should focus on improving the existing DMD diagnostics of assert and in turn assertThrown and alikes instead of adding a new suite should-functions. We should strive to do as much as possible with as little features/redundance as possible and fix/extend the existing ones instead adding a new different "overload set".
2. Further, the fact that the shouldXXX-functions throw exceptions will make it impossible to tag shouldXXX-calling unittests as nothrow and currently (because exceptions are currently allocated by the GC) also @nogc. This will lead to usage of both assert and should-way in cases where the unittests are used to verify that no exceptions are thrown in its functions calls.

Apart from those issues I warmly accept this module!

[atilaneves]

I agree it'd be better that the compiler assert do this work. But it's easier to do it in a library.

I don't see why unit tests need to be @nogc. Production code, sure, but why unit tests?

I added a link to the docs.

[nordlow]

The most appearant use would be to make sure that a specific Phobos algorithm/range doesn't accidentally use the GC nor throw exceptions for a specific set of element types. This is currently a hot topic because many Phobos algorithms accidentally make unnecessary use of the GC. We can't tag the template ranges/functions as @nogc nothrow because other (user-defined) element types should be allowed to be to use the GC and/or throw.

What if I fix DMD to make asserts have better diagonistics? :) What format do you want for a failing assert? Do you want to print the values of lhs and rhs aswell? Such a feature built into DMD has, at least, been discussed previously. Is toString always defined for a type in the same way that a struct always should be allowed to be default initialized?

[atilaneves]

@nordlow Knock yourself out ;) I've looked into changing assert in dmd myself; if it were easy I'd've already done it

[jmdavis]

I don't know what Walter would accept at this point, but after assertPred was rejected several years ago on the grounds that we should improve the built in assertions instead, the attempt to improve assert was ultimately rejected, because it cost too much:

https://issues.dlang.org/show_bug.cgi?id=5547

[nordlow]

What's costing memory? Extra AST-nodes in DMD?

[jmdavis]

Read the comments in the bug report I listed. That's all the information that I have on the details of why it would or wouldn't be a good idea. But according to Don, you have to copy the parameters, which can be expensive, and Walter was against the increased memory requirements.

If you can come up with something that makes assertion messages show more useful information and which gets past Walter and the rest of the compiler dev team, then great, but there was a previous attempt to make assert print out the values of of the variables involved - e.g. printing out the values of lhs and rhs in assert(lhs == rhs) - and it was rejected by Walter. Walter thought that it should be done with library functions instead. I don't know what Walter's stance on it would be at this point.

[nordlow]

What about adding a specific compiler flag (argument), say -unittest=verbose, which triggers pretty printing of reason for failing assert?

Typically added/modified by the developer by changing the flags to calls to rdmd/dub/scons etc upon failure with -unittest for a specific failing module. This will be more convenient than explicitly adding the prints of lhs and rhs in the failing unittest.

If DMD is called with -unittest=verbose it will then rewrite the AST to replace calls to

assert(lhs == rhs)

with

assertBinOp(A_FILE, A_LINE, A_COLUMN, "==")(lhs == rhs, lhs, rhs)

where

assertBinOp(string file, uint line, uint column, string op)(E, L, R)(lazy E expression, lazy L lhs, lazy R rhs)

and similarly for unary expressions.

The default implementation of assertBinOp (preferrably defined somewhere in druntime) would be some standard D code that mimics the current behaviour of assert(expression) by throwing an AssertExpression if cast(bool)expression is false (dont know about the behaviour of nothrow/@nogc discussed above though). The Phobos-developer can then do what he likes with the information he needs in the extra arguments in specific templated overloads of assertBinOp.

This specific behaviour could be extendable by adding (typically templated) overloads of assertBinOp for specific sets of types (concepts).

Then we could get the extendability we want in testing-frameworks such as this without adding a new assert-overload-set and without sacrifycing default memory usage in DMD/Phobos unittests.

Further with this solution we could add cool diagnostics behaviour in assertBinOp for failing array/range comparisons aswell in a format. This diagnostics could even have different pretty printing backends such as HTML.

For example a failing

assert([1,2,3] == [1,2,4]);

could fancy-print

([1,2,3][2] being 3) != ([1,2,4][2] being 4)

or for aggregates a failing

struct A { int x, y; }
auto a = A(1,2);
auto b = A(1,3);
assert(a == b);

could fancy-print

(a.y being 2) != (b.y being 3)

Thereby giving D yet another feature, I've not seen in any other language :)

If needed I'd be more than happy to help out implementing this solution in dmd/druntime/phobos. I'm guessing that parts of the solution list at

https://issues.dlang.org/show_bug.cgi?id=5547#c3

can be reused, right?

@WalterBright @andralex What do you think?

[andralex]

@nordlow sounds interesting - please write a DIP as I mentioned in the forum

[nordlow]

http://wiki.dlang.org/DIP83

[burner]

voting thread: http://forum.dlang.org/post/jnihyetudelpkvrxlabd@forum.dlang.org

[nordlow]

Why not Modules instead of uppercase?

[jmdavis]

All caps is in violation of the official style guide, so it really should be changed. And unless MODULES is a type (and it doesn't look like it is), it should be camelCased, not PascalCased, so it would then be modules, not Modules.

[nordlow]

I agree.

[JackStouffer]

This was rejected in the voting thread. Time to close?

[atilaneves]

I wanted to resubmit a simpler proposal, but I guess it makes sense to close this one as it is now.