Skip to content

Conversation

@wilzbach
Copy link
Contributor

There's still quite a bit of stuff in Phobos, that shouldn't be parsed & executed when Phobos is imported by a user who runs his tests.

For the motivation of excluding unittest in templates, please see https://wiki.dlang.org/DIP82

(I don't have any preferences on the name of this version.)

@dlang-bot
Copy link
Contributor

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.

@CyberShadow
Copy link
Member

Did we not have this or something very similar to this before? Am I thinking of another component?

@CyberShadow
Copy link
Member

Oh, I was probably thinking of StdDdoc.

In which case maybe make the two consistent.

@JackStouffer
Copy link
Contributor

bikesheading: any version you use should probably have two underscores prepended to it to avoid naming collisions with any existing code.

@dnadlinger
Copy link
Contributor

dnadlinger commented Dec 15, 2017

I'd say let's just "reserve" all StdXyz identifiers by mentioning that in the Phobos docs, and then stick to that naming scheme.

@wilzbach
Copy link
Contributor Author

Oh, I was probably thinking of StdDdoc.
In which case maybe make the two consistent.

Renamed to StdUnittest, which @JackStouffer I think is unique enough and considering that this sadly will be used often in Phobos I would prefer to avoid anything with two ugly __.
We could add a changelog entry?

@wilzbach
Copy link
Contributor Author

I'd say let's just "reserve" all StdXyz identifiers by mentioning that in the Phobos docs, and then stick to that naming scheme.

Great idea!
-> dlang/dlang.org#1999

Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need here a version(unittest) - the same we have for version(assert).

import std.typecons : Ternary;
import std.typecons : tuple, Tuple;

version(StdUnittest)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this here? I'd assume if no unittest is conducted, unittests are ignored anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From DIP82:

At present, unittest blocks inside of a templated type are compiled into every instantation, which only works with tests that are generic, which most tests aren't, and it almost never works with examples and thus doesn't work for ddoc-ed unittest blocks.

  1. these unittest blocks might not even be run (I have a couple of these issues during the runnable examples story)
  2. these unittest blocks will be executed as part of a user's testsuite

@wilzbach
Copy link
Contributor Author

I think we need here a version(unittest)

This already exists, but it's defined during a user's test run too.

@wilzbach wilzbach changed the title [RFC] Add a UnittestPhobos version Add a StdUnittest version Dec 21, 2017
@wilzbach
Copy link
Contributor Author

FYI there have been two recent pull requests for which this identifier would have been useful:

So maybe we can simply move on with this? There are many other places in Phobos which should have used StdUnittest ...

@dlang-bot dlang-bot merged commit da00e8f into dlang:master Dec 21, 2017
@wilzbach wilzbach deleted the udflags branch December 21, 2017 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants