[llvm-dev] phab unit tests + libcxx tests w/concurrency

[Brian Cain via llvm-dev]

I don't want to sound too picky but if the test is really "flaky" it
probably doesn't belong in the test suite.  But I appreciate that by
design, it's dependent on system load and possibly other factors.  So I
guess a flaky test by any other name would still smell as sweet ;)

It would be ideal if FLAKY_TEST were omitted from the `check-cxx` /
`check-cxxabi` target(s). Maybe it should be a lit feature and only passed
on when a new cmake option like LIBCXX_ENABLE_EXHAUSTIVE_TESTS or
LIBCXX_ENABLE_TIMING_TESTS is set?  Or if you are saying the FLAKY_TEST
annotation runs the test but then masks any failure then yes that probably
would suffice (but in that case why run the test at all?).

On Wed, Feb 26, 2020 at 4:24 PM Eric Fiselier via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> We do have a // FLAKY_TEST tag that is used to tell the test suite that a
> given test my fail for non-deterministic reasons.
> I'll add this annotation to more try_lock tests. In the past this has
> resolved LIT turning up actual flaky failures.
>
> Is this sufficient?
>
> /Eric
>
> On Tue, Feb 25, 2020 at 10:41 AM Brian Cain via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> I think it may make sense to segregate the libcxx lit tests that expect a
>> task to be completed in a particular threshold.  Either they should move to
>> the llvm test-suite or they should be under a feature guard that omits them
>> from the default test target.  These tests are sensitive to the load and/or
>> capability of the target on which they’re tested.  I appreciate that it’s
>> likely impossible to write a noninteractive test for these library
>> concurrency features that aren’t designed with some kind of thresholds.
>>
>>
>>
>> I have an “innocuous” change that was automagically tested (pre-merge!)
>> via phabricator --  https://reviews.llvm.org/D75085 -- but it triggered
>> a test failure in one of the “thread_mutex_class::try_lock.pass.cpp” tests.
>>
>>
>>
>> It’s great and super convenient to have this test facility and I’m pretty
>> sure I opted-in to it.  I think it would be/would have been nice for it to
>> be integrated with github PRs, but this seems functionally pretty close.
>> Having this pre-merge check helps buoy confidence in my change – that it’s
>> less likely to cause a buildbot to trigger.  We should strive to eliminate
>> false signals from this test suite.  Either the test suite should omit
>> check-cxx (not my preference) or the cxx tests must be descoped to reliably
>> passing ones.  Or maybe the test infrastructure could be
>> partitioned/dedicated such that it’s very unlikely to have false failures
>> like this.
>>
>>
>>
>> Also, thanks again to the teams who work on providing testing features
>> like this, it’s a step in the right direction.
>>
>>
>>
>> -Brian
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>


-- 
-Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200227/461c115c/attachment.html>