[PATCH] D55834: [libcxx] Make sure all experimental tests are disabled when enable_experimental=False

[Louis Dionne via Phabricator]

ldionne added a comment.

In D55834#1336438 <https://reviews.llvm.org/D55834#1336438>, @EricWF wrote:

> In D55834#1334932 <https://reviews.llvm.org/D55834#1334932>, @ldionne wrote:
>
> > In D55834#1334839 <https://reviews.llvm.org/D55834#1334839>, @EricWF wrote:
> >
> > > If the tests are header only, why disable them? We are losing code coverage on things people may be depending on.
> >
> >
> > I don't expect most people to be using `enable_experimental=False`, so that shouldn't actually reduce the test coverage. The problem I hit was running CI on older OS X where a test was failing (because of a problem with exceptions in the old dylib). The experimental part doesn't actually use anything in the dylib, but the test as a whole does, and enabling it when I explicitly say `enable_experimental=False` was surprising certainly not what I expected.
> >
> > In other words, I fully agree that we should strive to run experimental tests as often as possible (when it makes sense), and we do have the right default here. But when I explicitly say "don't give me experimental tests", I don't want them to run.
> >
> > If we'd rather not give that option at all, we could consider inferring the `c++experimental` LIT feature automatically based on stuff like whether `use_system_cxx_lib` was used and other things, but I don't think it makes a lot of sense.
> >
> > WDYT?
>
>
> I think you're missing the point of the feature. It's set by CMake when the user disables the experimental library build.


You're right. However, where I'm coming from is running the LIT test suite by itself and passing `enable_experimental=False`. For example, imagine an implementation that does not implement experimental features -- how would such an implementation use the test suite?

> Now we have no test coverage that we aren't regressing header only functionality.

I find this statement too strong. We do have coverage by default because the experimental library is built by default. This really only makes a difference if you're explicitly requesting for the experimental library _not_ to be built but would still want to test the code in headers. I find this use case questionable, and of smaller value than having the ability to turn off the experimental tests on their own.

Are you convinced?


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55834/new/

https://reviews.llvm.org/D55834