sourceLibrary package's unittest config should be used when testing dependee

[joseph-wakeling-frequenz]

System information

I don't think this issue is dependent on version, but FWIW:

• dub version: 1.15.0
• OS Platform and distribution: Ubuntu 18.04
• compiler version dmd-2.086.0

Bug Description

When crafting a library, some dependencies may only be needed for unittests (e.g. unit-threaded). It is possible to separate them from other required dependencies by using a custom unittest configuration. However, this means that the dependencies do not propagate to unittest builds of downstream apps/libs, which can cause build failures.

How to reproduce?

The following DUB project should illustrate the issue. It involves a project with a library dependency that uses unit-threaded.

The upstream lib is placed in the upstream directory:

upstream/dub.json:

{
    "name": "upstr",
    "targetType": "sourceLibrary",
    "configurations": [
        {
            "name": "sourceLibrary"
        },
        {
            "name": "unittest",
            "dependencies": { "unit-threaded:assertions": ">=0.8.0 <0.10.0" }
        }
    ]
}

upstream/src/lib.d:

int twice (int x)
{
    return 2 * x;
}

unittest
{
    import unit_threaded.assertions : should;

    twice(2).should.not == 4;
}

The main project, depending on upstr, is defined as follows:

dub.json:

{
    "name": "udeps",
    "targetType": "executable",
    "dependencies": { "upstr": { "path": "upstream" } }
}

src/app.d:

void main ()
{
    import lib : twice;
    import std.stdio : writefln;
    writefln!"Twice %s is %s"(5, twice(5));
}

Running dub test results in a build failure:

upstream/src/lib.d(8,12): Error: module `assertions` is in file 'unit_threaded/assertions.d' which cannot be read

Expected Behavior

DUB should recognize that the source library has extra dependencies for unittests, and include them in the downstream unittest build.

Note that the important concern here is that there be some way to specify some dependencies as unittest-only, and have them picked up by downstream builds. I'm not wedded to the specific need for this to be done via a custom unittest configuration; I am concerned that downstream projects should not need to manually deal with dependencies of their own dependencies' unittests.

This appears to only be an issue for sourceLibrary dependencies. When the upstream is a library then its unittests are not run anyway (which seems like an issue in its own right, but one to discuss separately).

[andre2007]

This appears to only be an issue for sourceLibrary dependencies. When the upstream is a library then its unittests are not run anyway (which seems like an issue in its own right, but one to discuss separately).

By setting targetType to sourceLibrary the modules (*.d) are added to the compiler as if they were contained in your main project. Therefore the unit tests run in this case.
If you use the targetType library, while your main project is compiled in unittest mode, the library is not . This is intended. You do not want to run the unittests of other libraries while running your tests.

[joseph-wakeling-frequenz]

By setting targetType to sourceLibrary the modules (*.d) are added to the compiler as if they were contained in your main project.

Yes, I know. This is expected behaviour.

Therefore the unit tests run in this case.

Indeed, and I was counting on this happening.

If you use the targetType library, while your main project is compiled in unittest mode, the library is not . This is intended. You do not want to run the unittests of other libraries while running your tests.

Er ... I don't?

I'd take quite the opposite attitude -- I want to run tests for the whole project, including dependencies. After all, how else can I verify the whole thing works together in the environment that I'm building it for?

I actually find it quite problematic that unittests aren't run when the dependency is a library -- and that there is no way to override that.

In any case, the fact is that for sourceLibrary dependencies, the unittests are run as part of a downstream project's build. So it's a problem that unittest-only dependencies are not propagated in this case.

[andre2007]

An example: there is this great library which provides terminal, http client and server and a lot more functionality.
But it had a major drawback. The unittests were interactive (waiting on user input). Due to another circumstance, the author was forced to use sourceLibrary as targetType. The consequence: unit testing of my application was not possible, because the unittest of the library always run also and the user input was not happening. The interactive thing even caused more issues in my IDE.

In the meantime the author was able to switch to targetType library, which solved the problem.

[joseph-wakeling-frequenz]

You've given a great example of a special case where a downstream might want to avoid running unittests for a specific dependency. I'm fine with the idea that one might want to, as a choice, exclude specific (or all) dependencies when running unittests. However, that is not a good reason to avoid running dependencies' unittests by default, or to have no way to run those unittests when the dependency is a library.

However, in my case, the problem is not that the unittests run. The problem is that dependencies required (and stated) for those unittests don't propagate. I'd appreciate it if the focus could be put on this problem, please.

[andre2007]

With the current knowledge I have, I agree with your opinion. The dependencies section in combination with targetType sourceLibrary seems to be, with the current behavior, dead information. It just serves as info which dependencies an app developer has to manually add to his application dub.json.

[John-Colvin]

cc @atilaneves w.r.t. unittests running in dependencies

[atilaneves]

@John-Colvin I think that, if someone needs it, they should be able to run dependencies' unit tests, but never by default. However, in the context of this particular issue I agree with @joseph-wakeling-frequenz

[joseph-wakeling-frequenz]

I think that, if someone needs it, they should be able to run dependencies' unit tests, but never by default.

Can you explain your rationale for that a little bit more? I'm not sure I mind (in the grand scheme of things) whether running dependencies' unittests is opt-in or opt-out, but personally I'd have favoured opt-out as the best way to ensure that, by default, any given project is as fully validated as possible.

I would be inclined to say that this result should be cached (just like library builds), so that library dependencies' unittests don't re-run (once done) unless --force is used, or compiler/build flags change.

[atilaneves]

Can you explain your rationale for that a little bit more?

Sure. It's the job of the dependencies to test themselves and have a green build badge. Downstream users can always write integration tests. Imagine if we ran dmd, druntime and phobos tests as part of every D project CI!

[joseph-wakeling-frequenz]

It's the job of the dependencies to test themselves and have a green build badge. Downstream users can always write integration tests. Imagine if we ran dmd, druntime and phobos tests as part of every D project CI!

Testing EVERYTHING would indeed be a bit excessive for a default, but is it unreasonable to expect that unittests might reasonably be run for every module included in a project, whether an internal module, or an upstream?

That's narrower than a full run of all dependency unittests (you pay only for what you use), but it does ensure that project code is unittested in its entirety.

That would also ensure consistency of default dub test behaviour between library and sourceLibrary dependencies.

[andre2007]

I fully agree with @atilaneves. If you really want to test all dependencies, you need to test phobos and DRuntime too.
It is the task of the library provider to test their libraries in different environments and give a go decision for these environments.

As a app developer I want to write 3 lines of code, run my tests, write the next 3 lines, run the tests and do this over and over again.
If running the tests would include the library tests, this would slow down the development process.

I really see your point @joseph-wakeling-frequenz but I think there is a better solution. Maybe we can run the tests of a library when it is build. Then you can be sure it is working fine in your environment n

[joseph-wakeling-frequenz]

@andre2007 we're risking getting a bit off-topic here, as the specific issue is about unittests of sourceLibrary dependencies, rather than the general issue of dependency testing.

However, I'll give an answer now, and we can move it to another issue if that seems appropriate. With respect to:

I fully agree with @atilaneves. If you really want to test all dependencies, you need to test phobos and DRuntime too.

See my earlier remark:

is it unreasonable to expect that unittests might reasonably be run for every module included in a project, whether an internal module, or an upstream?

I absolutely want unittests to be run for phobos and druntime modules that are included in the project in question.

As a app developer I want to write 3 lines of code, run my tests, write the next 3 lines, run the tests and do this over and over again.
If running the tests would include the library tests, this would slow down the development process.

Fair enough. That's your use case, and it's reasonable that DUB should support you being able to do that.

So, here's mine. I'm writing high-throughput distributed applications where each node must be able to run indefinitely (i.e. no upper limit on how long it runs for), and handle failures gracefully. The number of events per second is sufficiently high that "rare" failure cases are in practice very common.

What that means is that there is a VERY strong requirement on being able to validate all the code that is being built. The time taken to run those dependencies' unittests is small compared to the value of confirming that they have been validated in my precise build environment, and I cannot afford to assume that the upstream knows what they are doing and has run adequate CI.

As a point of comparison, take three prominent D library projects, and look how many merged PRs actually have failing CI:
https://github.com/libmir/mir-algorithm/pulls?q=is%3Apr+is%3Aclosed
https://github.com/vibe-d/eventcore/pulls?q=is%3Apr+is%3Aclosed
https://github.com/vibe-d/vibe.d/pulls?q=is%3Apr+is%3Aclosed

Now, I'm sure that most (hopefully all) of that is innocent, and is down to those projects' maintainers knowing well which CI failures really matter and which do not. However, it gives a good reason to want to validate things for myself, in my own build environment (you'd be amazed how readily a small difference in the choice of compiler flags or some other environment setting can expose a bug that wasn't spotted upstream).

One way to do that is to have all dependencies provided as source-library submodules, so that there's an inherent "pay for what you use" inclusion of unittests. See e.g. https://github.com/sociomantic-tsunami/dhtnode for an example of a high-performing application using that approach (without using DUB).

However, DUB gives me no comparable way to validate the dependencies that I am using, even though it is building all of them, regardless of whether they are marked library or sourceLibrary.

Now, we clearly agree that it should provide the ability to do that:

Maybe we can run the tests of a library when it is build. Then you can be sure it is working fine in your environment

... so any disagreement really comes down to what the default behaviour should be, and what should be available as an option.

So let's try to enumerate options for what one might want to do:

1. run unittests for a single module, or maybe even a single (named?) unittest block

2. run unittests just in a project's own code

3. run unittests for every module included in a project (both its own, and modules imported from dependencies, ideally including druntime and phobos)

4. run comprehensive unittests both for the project itself and for every one of its dependencies (i.e. for everything that DUB is building, again ideally including what it's building from druntime and phobos), possibly caching results for library dependencies so they only get re-tested if something changes or if --force is used

5. for either of the previous two cases, include or exclude individual dependencies based on a whitelist or a blacklist

Now, (3) above is what will happen with sourceLibrary dependencies, and we currently have no way to avoid that. (2) is what currently happens for projects that only have library dependencies, and we have no way to avoid that either. There is no way that I know of for DUB to support either (1), (4) or (5), even though all are reasonable use cases.

It sounds like we can probably get agreement that cases 1-4 should be supported (maybe with some quibbles over whether we bring druntime and phobos into the mix), and really the only question is whether we want (2) or (3) to be default behaviour.

I'd favour (3) as a default on safety-first grounds, with a dub test flag to switch to (2) for anyone who cares about the time saving. However, I'd understand if the preference was for (2) as default.

(5) is a little tricky because we'd need to make sure that it was always in an individual project's control which dependencies are whitelisted or blacklisted for testing (e.g. you probably don't want an upstream's choice of white/blacklist to propagate).

Assuming any and all of the above make sense as features, what would be the best way to move forwards? With one or more new issues here, or something else?

[andre2007]

For the main issue: as far as I understand we all agree, sourceLibrary dependencies should be propagated. The issue is, there is not really a main developer for Dub at the moment. Bug fixes and enhancements are done by the developers who need the fix/feature.

For the "offtopic issue": I suggested a solution similar to point 4. When the dependencies are built, one part of the build process is to execute the tests. If the tests are failing, the built process fails.
This implicit means, when the dependencies are changing, tests are executed again during the new build process. Also --force automatically triggers this behaviour.

We could introduce a new config value for the config json. S.th. like buildAndTest.

[joseph-wakeling-frequenz]

For the "offtopic issue": I suggested a solution similar to point 4.

I do recognize that (maybe I didn't communicate clearly enough that my point (4) was intended to fulfil your suggestion). But I think we should consider all the other use-cases as well.

BTW as an aside: I think the sourceLibrary/library distinction is something of a problematic one, inasmuch as the choice really should be for the downstream to decide, rather than the upstream. After all, any library can in principle be treated as a sourceLibrary if that's the preferred usage. There's a difference between saying "can this be built as a static lib?" (which obviously doesn't make sense for e.g. libraries where everything is templated), versus "do I want to use it that way as part of my build?" (which you can ask in every case).

But then again, that's a general issue with DUB's design, that it makes it too easy for upstreams to impose decisions on downstreams, and doesn't give enough control in the other direction.

[andre2007]

As explanation: when running the unittests during the build phase, in no case running the tests later should give you another result. That means even if you run the library tests on every application test run, they won't give you another result.

The only possibility that the test result changes is when the tests are flawed by relying on external resources like time/network/file system. But these are buggy unittests.

But if you really want to run the library unittests with your application tests, the proposed config buildAndTest would support you. Just switch it on and build your application with --force.

PS. As work around you can still use any dub library as source library by copying the source code into your application folder and include the source code.

[joseph-wakeling-frequenz]

As explanation: when running the unittests during the build phase, in no case running the tests later should give you another result.

I don't understand what this specific sentence is in response to.

[andre2007]

It was an explanation why I think not all scenarios make sense, because some scenarios handle cases which cannot occur (in theory)

[joseph-wakeling-frequenz]

some scenarios handle cases which cannot occur (in theory)

I don't follow why you think any of the scenarios outlined are problematic. I'm afraid that I find your posts difficult to understand, because it often seems like you are writing an incomplete summary of your actual thoughts.

(1), (2) and (3) are intended as convenience options -- allowing ways to not test more than the developer wants or needs to test at any given time. They reduce the total time required, and in particular (3) should test everything necessary to validate the project in its entirety (while not testing unused parts of dependencies).

(4) is a useful and necessary option to have but is probably overkill for most usage scenarios. For example, if I have a dependency on vibe-d, why should I care about running the unittests for vibe:data if I don't actually use that subset of the library?

However, all this is beyond the scope of the current issue. I recommend that we pause here and take such bigger-picture discussions to another context.

[schveiguy]

Just chiming in to say that I just had a similar problem, though in a different way. I have a library (mysql-native) that I wanted to run unittests using a separate harness. The main reason being that I did not want to poison any other projects that include mysql-native with the dependencies required for testing (e.g. vibe.d, unit-threaded). But I couldn't figure out how to make the unittests run correctly. I had no idea about the sourceLibrary type, which was a perfect fit for me (@Geod24 pointed me at this issue for inspiration). See relevant PR here

In any case, what I found is that all I needed to do when compiling unittests is to specify that the unittest configuration in the main library was of "sourceLibrary" mode. Then in the integration test application, I needed to use that configuration. It worked perfectly. Would this be a solution here?

relevant parts of my dub.sdl:

configuration "library" {
}

configuration "unittest" {
	debugVersions "MYSQLN_TESTS"
	targetType "sourceLibrary"
}

Then in the integration test dub.sdl, I just have to specify the subconfiguration of "unittest" for mysql-native, and now I can run its unittests. This might be the option folks are looking for (by default, don't run unittests, but allow running them if selected).