<div dir="ltr">IMO having to disable 2/3 of the tests means the patch isn't ready yet.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 28, 2015 at 9:31 AM, Jason Kim <span dir="ltr"><<a href="mailto:jasonk@codeaurora.org" target="_blank">jasonk@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
> -----Original Message-----<br>
> From: Renato Golin [mailto:<a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>]<br>
> > TESTS!<br>
> > Currently, about 2/3 tests for tsan fail/flake on android+aarch64.<br>
> > They aren't xfailed, because some of them are flaky, and thus<br>
> unexpectedly pass at times.<br>
> > So I decided to mark them as unsupported on android+aarch64, and thus<br>
> is "green".<br>
><br>
> So, if they currently pass on the production buildbots, you can't mark them<br>
> as unstable because of your patch, as that will mean your patch is making<br>
> them unstable, and we can't just make dozens of tests unstable for any<br>
> amount of features.<br>
<br>
</span>[ jasonk --> ] The tests, AFAIK, currently "pass" for a somewhat squiggly definition of "pass".<br>
I get occasional fails on a local x86_64 build, w/o my patches.<br>
<span class=""><br>
> If your patch introduces the instability, you'll have to fix them before<br>
<br>
</span>[ jasonk --> ] The instabilities were there to begin with, AFAIK. Some of the tests are run on a "deflake" regimen where they are repeated 10x, looking for a specific output.<br>
<span class=""><br>
> commit. That's why I suggested splitting it into many small chunks, none of<br>
> which will leave the tree in an unstable state, because you'll have time to<br>
> look at each instability individually, and fix them before commit. The bots<br>
> will also be happier, and every one wins.<br>
<br>
</span>[ jasonk --> ] Sorry, but I don't understand what you want here. The tests I touched are marked unsupported (i.e. won't be run at all) ONLY for aarch64-linux-androideabi. Is there something else you want? It should not impact any other configuration of TSAN/compiler-rt as-is. I also guard TSAN with a CMAKE variable so that it only gets built for aarch64 iff COMPILER_RT_FORCE_TSAN_AARCH64=1<br>
<br>
<br>
<br>
> cheers,<br>
> -renato<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888">-Jason<br>
<br>
<br>
</font></span></blockquote></div><br></div>