[Issue 18488] test_extractor misses version(unittest) blocks, causing `Deprecation: X is not visible from Y`

[d-bugmail--- via Digitalmars-d-bugs]

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

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

--

[Issue 18488] test_extractor misses version(unittest) blocks, causing `Deprecation: X is not visible from Y`

[d-bugmail--- via Digitalmars-d-bugs]

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

greenify  changed:

   What|Removed |Added

 CC||greeen...@gmail.com

--- Comment #5 from greenify  ---
(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)

--

[Issue 18488] test_extractor misses version(unittest) blocks, causing `Deprecation: X is not visible from Y`

[d-bugmail--- via Digitalmars-d-bugs]

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

--- Comment #4 from Timothee Cour  ---
> 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) ?

--

[Issue 18488] test_extractor misses version(unittest) blocks, causing `Deprecation: X is not visible from Y`

[d-bugmail--- via Digitalmars-d-bugs]

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

--- Comment #3 from Seb  ---
> 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).

--

[Issue 18488] test_extractor misses version(unittest) blocks, causing `Deprecation: X is not visible from Y`

[d-bugmail--- via Digitalmars-d-bugs]

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

Timothee Cour  changed:

   What|Removed |Added

 CC||timothee.co...@gmail.com

--- Comment #2 from Timothee Cour  ---
> 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

--

[Issue 18488] test_extractor misses version(unittest) blocks, causing `Deprecation: X is not visible from Y`

[d-bugmail--- via Digitalmars-d-bugs]

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

Seb  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 CC||greensunn...@gmail.com
 Resolution|--- |WONTFIX

--- Comment #1 from Seb  ---
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];
}
---

--

[Issue 18488] test_extractor misses version(unittest) blocks, causing `Deprecation: X is not visible from Y`

[d-bugmail--- via Digitalmars-d-bugs]

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

Jack Stouffer  changed:

   What|Removed |Added

 CC||j...@jackstouffer.com
   Hardware|x86 |All
 OS|Mac OS X|All

--