https://issues.dlang.org/show_bug.cgi?id=18488

Jack Stouffer <jack@jackstouffer.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jack@jackstouffer.com
           Hardware|x86                         |All
                 OS|Mac OS X                    |All

--

https://issues.dlang.org/show_bug.cgi?id=18488

Seb <greensunny12@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |greensunny12@gmail.com
         Resolution|---                         |WONTFIX

--- Comment #1 from Seb <greensunny12@gmail.com> ---
It's not a bug, but a feature.

1) ALL public examples need to be runnable locally and dlang.org
That's not optional, but a deep requirement from mistakes in the past and the
resulting bad image.

2) We don't use version(unittest) for future code and are about to weed out the
last usages in Phobos, because you essentially just ended up with
- accidentally exposing a public symbol
- adding different behavior for -unittest (i.e. the testsuites) - there have
been quite a few bugs where `version(unittest) { import std.stdio;}` led to
bugs in user code because of templates and their dependence on the
version(unittest) imports which obviously wasn't caught by the testsuite.

While I really like your enthusiasm and in general a lot of things in the D lang are old, same are quite new and have a reason for being there, so sometimes taking a moment to step back and check why there's a CI warning (and not directly opening a bug report + finding "workarounds") would save you save time and frustration.

> The solution would be for test_extractor to export `vesion(unittest)` declarations.

That wouldn't help and you can already do better today with a ConditionalDeclaration visitor, e.g. in pseudo-code:

---
override void visit(const ConditionalDeclaration decl){
   // if decl is unittest
    auto text = cast(immutable(char)[]) sourceCode[decl.startLocation
decl.endLocation];
}
---

--

https://issues.dlang.org/show_bug.cgi?id=18488

Timothee Cour <timothee.cour2@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |timothee.cour2@gmail.com

--- Comment #2 from Timothee Cour <timothee.cour2@gmail.com> ---
> We don't use version(unittest) for future code and are about to weed out the last usages in Phobos


this seems not true anymore, so `version(unittest)` is ok again, see:


> FYI: that was before StdUnittest was reverted: #6202
https://github.com/dlang/phobos/pull/6178#discussion_r174675283

--

https://issues.dlang.org/show_bug.cgi?id=18488

--- Comment #3 from Seb <greensunny12@gmail.com> ---
> this seems not true anymore, so `version(unittest)` is ok again, see:

Well, it's an open problem - it was reverted because with -deps ALL unittest
blocks are compiled (even the ones in Phobos).
See also:

https://github.com/dlang/phobos/pull/6202 https://github.com/dlang/phobos/pull/6159

Anyhow, as mentioned in my earlier comment we purposely don't include the version(unittest) symbols in the publicly documented tests as they aren't available for the user neither and wouldn't run on run.dlang.io or other places and we want to prevent such errors (people have rightfully complained a lot about examples from the docs not working locally in the past).

--

https://issues.dlang.org/show_bug.cgi?id=18488

--- Comment #4 from Timothee Cour <timothee.cour2@gmail.com> ---
> people have rightfully complained a lot about examples from the docs not working locally in the past

just curious, doesn't auto-tester make sure that the extracted documented
unittests (produced by pipeline that extracts them from documented unittests)
can be run (and don't generate failures) ?

--

https://issues.dlang.org/show_bug.cgi?id=18488

greenify <greeenify@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |greeenify@gmail.com

--- Comment #5 from greenify <greeenify@gmail.com> ---
(In reply to Timothee Cour from comment #4)
> > people have rightfully complained a lot about examples from the docs not working locally in the past
> 
> just curious, doesn't auto-tester make sure that the extracted documented
> unittests (produced by pipeline that extracts them from documented
> unittests) can be run (and don't generate failures) ?

No the extraction is only run on CircleCi. This isn't a huge problem as it's
almost always just missing imports or other accessibility issues.
(I was talking about the past before this pipeline was introduced)

--

https://issues.dlang.org/show_bug.cgi?id=18488

--- Comment #6 from Timothee Cour <timothee.cour2@gmail.com> ---
ok filed a bug for that so we don't forget: https://issues.dlang.org/show_bug.cgi?id=18619

--