Add changelog entry for StdUnittest

[wilzbach]

@timotheecour asked me to write a short changelog entry, so here it is.

See also:

• Add a StdUnittest version #5927
• Replaced version(unittest) blocks with version(StdUnittest) #6107

[dlang-bot]

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

[JackStouffer]

I don't understand why users need to know this Phobos specific implementation detail.

At most, this would be a short note saying Std prefixed versions are reserved for Phobos.

[wilzbach]

People complained about this e.g. @timotheecour @FeepingCreature
I think the problem is that

1. Some people are used to test Phobos module with just rdmd -unittest -main foo.d
2. People are packaging Phobos in their own ways (distros, company repos, etc.)

Even just (2) is reason enough for this to be in the changelog.

[FeepingCreature]

When building with -deps -unittest, DMD will recurse into Phobos unittests, even when building something unrelated. However, since StdUnittest is not set, the Phobos unittests are semantically invalid.

[JackStouffer]

Wow, that worse than I originally understood. That means people's builds are broken because of the change to StdUnittest right?

If so we need to either revert until the change @jmdavis was making (only running tests for compiled files) is in. Or, version out the effected unittests a-la

version(StdUnittest) unittest
{
}

[FeepingCreature]

Indeed, though note that luckily this only seems to happen on master so far. What worries me is that there doesn't seem to be a build step to detect this issue.

In the medium term, it would probably be useful to have a command line flag to prevent -deps from recursing into certain packages, such as those that are linked in as libraries. Phobos could then be excluded via a default flag.

[jmdavis]

If so we need to either revert until the change @jmdavis was making (only running tests for compiled files) is in.

I'm doing no such thing. I suggested that it looked like we needed to go in that direction, and I suspect that a DIP is required. I'm not a compiler dev and have no idea how to go about implementing such a fix. This is almost certainly not something that is going to be fixed quickly. So, if version(StdUnittest) is causing problems, then either it needs to be given the axe, or all of the affected unittest blocks need to be marked with version(stdUnittest), as disgusting as that may be. However, unfortunately, this is exactly the sort of thing that we're not going to catch with the autotester, because it requires compiling other programs that use phobos rather than compiling phobos.

[timotheecour]

would something like this https://forum.dlang.org/post/mailman.3193.1518032575.9493.digitalmars-d@puremagic.com provide a better, less hacky and invasive solution? dmd -unittest=+foo.bar,+std,-std.stdio args... to specify unittests in select pkg/mod (analog to newly introduced -i=pkg syntax) it allows to sepcify what gets compiled into a unittest, eg: `dmd -unittest=-std main.d` will disable phobos unittests (ie things in std package)

…

On Thu, Feb 8, 2018 at 11:31 AM, Jonathan M Davis ***@***.***> wrote: If so we need to either revert until the change @jmdavis <https://github.com/jmdavis> was making (only running tests for compiled files) is in. I'm doing no such thing. I suggested that it looked like we needed to go in that direction, and I suspect that a DIP is required. I'm not a compiler dev and have no idea how to go about implementing such a fix. This is almost certainly not something that is going to be fixed quickly. So, if version(StdUnittest) is causing problems, then either it needs to be given the axe, or all of the affected unittest blocks need to be marked with version(stdUnittest), as disgusting as that may be. However, unfortunately, this is exactly the sort of thing that we're not going to catch with the autotester, because it requires compiling other programs that use phobos rather than compiling phobos. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6142 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACF9YEaSVzFfgQ2hZRv5y69oZA8otz0-ks5tS0pmgaJpZM4R-SFH> .

[JackStouffer]

Sorry, meant to say that you suggested it as a solution.

Regardless, it looks like for the time being StdUnittest needs to be removed from Phobos. Thoughts @wilzbach?

It's a shame to remove this optimization, but it also sucks that we are slowing down peoples unittest builds by running our own unittests. That has to really suck for people who are using something like systime or parallelism, as those can take more than 10 seconds.

[wilzbach]

However, unfortunately, this is exactly the sort of thing that we're not going to catch with the autotester,

Yeah, but that's why we have been working hard to get the Project Tester up, running and moderately stable.

@FeepingCreature that's one of the advantages of open-sourcing your libraries.

What worries me is that there doesn't seem to be a build step to detect this issue.

It's simple, no one is using -deps in their projects. At least not the ~35 projects we test.
And without Bugzilla issue, it's not a regression (we don't monitor IRC for bug reports).

Or, version out the effected unittests a-la

If those aren't too many, that's imho the best way to go. version(StdUnittest) is solving another real problem.

[jmdavis]

would something like this https://forum.dlang.org/post/mailman.3193.1518032575.9493.digitalmars-d@puremagic.com provide a better, less hacky and invasive solution?

Honestly, I think that this needs to be automatic unless there's a technical reason why it can't be. It's ridiculous to be running unittest blocks in modules that were only imported. And as soon as you're turning unittest blocks on or off manually, there are going to be mistakes.

Maybe such a feature would be worthwhile for being able to selectively run tests, but I don't think that it's a great way to go for fixing the problem of unittest blocks from other projects being compiled in when they shouldn't be.

[FeepingCreature]

As I understand/conjecture it, the problem isn't even necessarily that they're compiled in so much as that they have to be processed in order to identify import statements that they may contain, so -deps is accurate. Hence why this problem only occurs in the -deps -unittest combination.

[n8sh]

So, if version(StdUnittest) is causing problems, then either it needs to be given the axe, or all of the affected unittest blocks need to be marked with version(stdUnittest)

FYI I've done that in #6159

[jpf91]

Honestly, I think that this needs to be automatic unless there's a technical reason why it can't be. It's ridiculous to be running unittest blocks in modules that were only imported. And as soon as you're turning unittest blocks on or off manually, there are going to be mistakes.

👍

However, simply not running unittests for modules not being compiled would break the 'keep unittests in external file'+single file compilation idiom. I think there's a bigger problem here manifesting in

• manually having to specify which modules can run unittests
• manually having to specify which modules need to be compiled (-i)
• manually having to specify which modules are dllexport/import (http://stuff.benjamin-thaut.de/D/getting_started.html)

Essentially, we need a way of telling the compiler

1. which modules are pre-compiled/available in an external library
2. which modules are part of the current build sources

Ideally, (2) can be inferred by collecting all imported modules (through import chains) and removing the modules in (1). If every library provided a mylib.dmodule file which simply mapped a list of D modules to library files, you could do dmd -dmylib main.d instead of dmd -lmylib -I /usr/include/d/mylib/ -i=+main,-mylib -unittest=+main,-mylib -import=mylib. We could even reuse the pkg-config files which meson already uses/generates for D libraries. I guess I should write a DIP?

[JackStouffer]

Any further discussion on this should be moved to the forums or slack.