Bug 13454 – Unit tests should be compiled in a module, where they are declared

http://forum.dlang.org/post/[email protected] There's no way to disable unittests compiled as part of templates, instantiated from another library. Error: --- template A(T) { int a; unittest{ f(); } } version(unittest) void f(){} --- If this module is compiled without unittests, `f` function will not be compiled, then if another module is compiled with unittests and instantiates the template, the template's unittest is compiled and it will link with `f` function, which wasn't compiled, so linking will fail? There will be no way to compile the importing module with unittests. Performance: --- import core.stdc.stdio:puts; template A(T) { shared int a; unittest { puts("test"); } } int main() { A!(int).a=0; A!(long).a=0; return 0; } --- test test --- The same test was executed twice (for each instance of a template). It's questionable that duplicating tests for each instantiation of a template is intended.

I'd say that each template instance should come with it's own unittest, but there should be a way to disable unittests for non-root modules, i.e. modules that are imported, but not part of the compilation.

Also see https://github.com/D-Programming-Language/druntime/pull/990

It is illegal to build D projects with non-consistent version flags exactly for this reason. version(unittest) is no different.

In practice template unittests are often independent of their template instance and simply result in code duplication. (In reply to Dicebot from comment #3) > It is illegal to build D projects with non-consistent version flags exactly > for this reason. version(unittest) is no different. Phobos is compiled without unittests, that will prohibit user code from being compiled with unittests.

I would argue that it only makes sense to compile in unittest blocks or version(unittest) code when directly compiling a module with -unittest and not when importing it. Otherwise, you'll end up getting stuff like Phobos' unit test code in your application when you compile it with -unittest, and the unit test code in libraries like Phobos would then potentially affect your program, causing fun problems like linker errors just because you imported a 3rd party module inside of your own module that you're unit testing.

> Otherwise, you'll end up getting stuff like Phobos' unit test code in your application when you compile it with -unittest And this is _exactly_ what I want by default. > Phobos is compiled without unittests, that will prohibit user code from being compiled with unittests It never uses version(unittest) in a way mentioned in the first post. Actually I do remember major removals of version(unittest) from Phobos completely because it caused too many issues with random imports. Looking at current master there are still many though :(

When designing unit tests for templates, a decision must be made: 1. are the unit tests to be run for specified instantiations only? 2. are the unit tests to be run for all instantiations? For (1), put them outside the template body. For (2), put the unit tests inside the template body. In the example code, the unit test code is in both places and both are expected to exist, and as you discovered, such does not work. The solution is either put all the unit test code outside the template for option (1), or all inside for option (2). I almost always go with (1) for my template code, and it works fine. > It's questionable that duplicating tests for each instantiation of a template is intended. Right, that's the (1) or (2) decision above, and only the programmer can make that decision. > too many issues with random imports -------- bad practice ---------- version (unittest) import std.stdio; unittest { writeln("hello"); } -------- better practice --------- unittest { import std.stdio; writeln("hello"); }

(In reply to Walter Bright from comment #7) > Right, that's the (1) or (2) decision above, and only the programmer can > make that decision. Is it a workaround or resolution?

(In reply to Walter Bright from comment #7) > When designing unit tests for templates, a decision must be made: > > 1. are the unit tests to be run for specified instantiations only? > 2. are the unit tests to be run for all instantiations? > > For (1), put them outside the template body. For (2), put the unit tests > inside the template body. In the example code, the unit test code is in both > places and both are expected to exist, and as you discovered, such does not > work. This is, inconvenient, if not downright horrid. Putting a unit test far away from the code it tests makes for some very difficult reading. It would be much more palatable if there was a way to say inside the tmeplate "here's a unit test, but it's not part of the template". But I can't think of any good reason why unit tests should be compiled when importing. If you want to build the unit tests, build the module that contains them. The author wrote the unit tests to run when he built that module, not when someone else imported them. They probably aren't meant for other consumption. All that being said, putting unit tests outside the template is a valid workaround, even if it sucks.

Note, I wouldn't be opposed to disallowing unit tests inside templates. That would at least avoid inadvertent errors. It's nearly impossible to create a universal unit test for a template.

It would certainly be nice to have a way to indicate that a unittest inside a template is not actually part of the template, since it would allow you to put the test for a function right after, which you can't do right now, which is definitely ugly and harder to maintain, but I don't know how reasonable that is from the standpoint of how templates work and how their implementation works. As for compiling unit tests while importing, if I understand what Walter is saying, he's saying that if you have the unit tests in the template, and they're compiled into each instantiation when -unittest is used, then every instantiation gets tested even if it's with template arguments that the template's author never thought of, and that if you didn't want to test the template with every instantiation, why did you put in the template in the first place? Now, that being said, while I can understand that point of view, as a user, I don't think that I'd ever want a template that I imported to include unit tests when I import it. I'd want the author to have tested it appropriately, and I don't want to have the extra overhead in my app (even if it's just when unit testing) of having those tests from a 3rd party compiled and run in my app. Sure, it might catch some bugs, but just as likely, the tests weren't written in a robust enough manner to work with all template arguments that the template actually works with. And even if the tests work perfectly, I just don't want that overhead. That belongs in the unit test app of the author of that code - not in mine. And as the author of a template, there are cases where I'd like to able to put a test inside of the template and have it tested with all of the instantations that I have in my unit test app and be able to have the unittest blocks right next to what they're testing, and I don't want that code ending up in a user's app. I don't want to worry about whether I wrote the tests robustly enough to work with _all_ template arguments. Even if the tests are reasonably well written, implicit conversions and the like can easily make it so that you end up with a compilation error in the tests with a stray type that you didn't test with and yet still works with the template just fine. So, having unittest blocks be compiled in when compiling anything other than the module that that template is in seems highly negative to me. I'd much rather argue that it's up to the author to make sure that they test it with a large enough range of template arguments rather than have those tests end up in a user's code. And I'd hate to have to move unittest blocks out of a template just so that they don't end up in user code. Honestly, I think that the situation with templates and unit tests sucks. Once in a while, I'm able to put unit tests directly in the template, because they'll work with most or all template instantations, but if that's going to end up in user code, then I'm _never_ putting them in there, which sucks. And it sucks that in most cases, I have to put the tests outside of the template, because I need to write tests which are for specific types, and you can't put those inside of the template unless you do something like version each of them so that they get compiled in with the instantiation that they go with. So, in most cases, unit tests for templated types end up outside the template itself, which harms maintainability. And given that I now know that tests within the template end up in user code (something that I would never have expected), I don't _ever_ intend to put unit tests inside of a templated type. That behavior is completely undesirable to me. So, the current behavior is just plain bad for maintainability IMHO. I would love it if you could mark tests within a template as existing outside the template - that would definitely improve maintainability of templated types - and I think that for unit tests in templates to be worth using, they really need to only be compiled in with the module that they're in. So, while I can understand Walter's position from a flexibility and correctness standpoint, from a usability and maintainability standpoint, I completely disagree with it.

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17682 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB