dub icon indicating copy to clipboard operation
dub copied to clipboard

Support for unit tests in package.d

Open mihails-strasuns opened this issue 10 years ago • 11 comments

I have recently learned that dub ignored package.d files when generating automated unittest target. This sounds like a blocking issue to me because practice of putting package-wide unit tests into package.d is not uncommon and quite convenient. Right now those seem to be silently ignored.

@s-ludwig mentions the issue with imports of package.d that prevent identifier look-up in other, non-package imports. This may need more explanation as I don't see why identifier look-up is needed at all from generated files (if import is present, runtime can find all test blocks anyway).

@s-ludwig can you explain your position on topic a bit more in details?

mihails-strasuns avatar Sep 25 '14 23:09 mihails-strasuns

I consider this a very serious issue. You basically have no control over which identifiers are available as "static imports", especially when compiling multiple files at once. And even if you do, it severely affects the ability to write generic code. People are also free to put anything they like into a package.d. If only public imports were allowed, DUB's approach to ignore package.d would at least always be valid, but as it stands this is a mess. The implemented solution is simply a very bad design that hasn't been thought through beforehand, IMO.


The problem is that the unit tester tries to iterate over all modules. See the generating code. It creates a TypeTuple of all modules and then iterates over them. However, this fails as soon as a package.d is involved.

s-ludwig avatar Sep 26 '14 07:09 s-ludwig

Any reason why you you have chosen this over runtime reflection to iterate all modules/tests? It does not seem to use any unittest UDA as far as I can see.

As for package.d - it was intended to behave as module at the import site. This has created import clashes because identifier resolution system did not take into account possibility of conflicting module paths. This is indeed a problem for generic code and something needs to be done about it but does not justify calling package.d broken feature and not supporting at all. Not in official release at least.

If you can list important criteria to keep in mind I can try re-writing dub test target generator to add at least basic support for it.

mihails-strasuns avatar Sep 26 '14 11:09 mihails-strasuns

It's used here to optionally pass the list of modules to a custom unit test runner.

but does not justify calling package.d broken feature and not supporting at all

Call it as you like, but to me, in it's current form, it's nothing more than a hack. It get's the job done and more or less works for the intended use case, but everything around that is a catastrophe. Leaving out a proper solution for symbol resolution for a new feature is IMO unacceptable at this stage of the language's development. But I'm getting back into a bad mood WRT D, so let's better try to stop this here and look for constructive solutions instead ;)

Anyway, do you have an idea how it's possible to work around this and to __traits(getUnitTests) on both the package.d module and sub modules?

s-ludwig avatar Sep 26 '14 11:09 s-ludwig

Anyway, do you have an idea how it's possible to work around this and to __traits(getUnitTests)

This is exactly what I am asking : is there any reason __traits(getUnitTests is necessary right now as opposed to running ModuleInfo.unitTest()? (https://github.com/D-Programming-Language/druntime/blob/master/src/object_.d#L1704)

It will be an important building block to provide any fancier framework with UDA on top of the unittest blocks but for simply running all tests it shouldn't be necessary.

mihails-strasuns avatar Sep 28 '14 12:09 mihails-strasuns

Yes, the test runner does use UDAs: https://github.com/rejectedsoftware/tested/blob/master/source/tested.d#L267

s-ludwig avatar Oct 07 '14 09:10 s-ludwig

Now I even more lost. What is relation between tested and dub?

mihails-strasuns avatar Oct 07 '14 17:10 mihails-strasuns

It's just that tested will be automatically invoked, if available. But it's also possible to specify a custom test main module that works the same way, so it doesn't really matter.

s-ludwig avatar Oct 07 '14 18:10 s-ludwig

Thanks for the explanation. I need to think what can less intrusive way to fix this, such external dependency complicates it.

mihails-strasuns avatar Oct 09 '14 15:10 mihails-strasuns

I have finally got to it and was surprised to find out that contrary to your old statement, tests inside package.d seem to work:

$ find source/
source/
source/mypkg
source/mypkg/package.d
source/mypkg/mod1.d
source/app.d
$ ../../dub/bin/dub test
Generating test runner configuration '__test__library__' for 'library' (library).
Building test ~master configuration "__test__library__", build type unittest.
Compiling using dmd...
Linking...
Running ./__test__library__ 
mypkg.mod1
mypkg
All unit tests have been run successfully.

Of course there are still issues when trying to use any relfection-based test library on top of it but basic "run all tests funcionality" works as expected. Has something been fixed about it during last months or it has been misunderstanding since the very beginning? @s-ludwig ?

mihails-strasuns avatar Dec 12 '14 05:12 mihails-strasuns

Ping

mihails-strasuns avatar Dec 18 '14 10:12 mihails-strasuns

Looks like https://issues.dlang.org/show_bug.cgi?id=11847 has been fixed in the meantime. dub still excludes package.d from tests. Probably this could be fixed now.

thaven avatar Aug 12 '21 11:08 thaven