llvm dev - Jul 2013 - [LLVMdev] [RFC] Switching make check to use 'set -o pipefail'

We currently don't use pipefail when running test under make check.
This has the undesirable property that it is really easy for tests to
bitrot. For example, something like

llc %s | FileCheck %s

will still pass if llc crashes after printing what FileCheck was
looking for. It is also easy to break the tests when refactoring. I
have fixed tests that were doing

%clang_cc1 -a-driver-options ... | not grep

clearly the test was changed from %clang to %clang_cc1 and we missed
the fact that the option also had to be updated.

Currently to check a command output and that it doesn't crash we have to do

llc %s > %t
FileCheck %s < %t

I would like to switch to using pipefail instead. That would meant that a simple

llc %s | FileCheck %s

would check both llc return value and output. I have already cleared
all the tests, so all that is missing is changing lit itself. Any
objections to doing so?

Cheers,
Rafael

Hi Rafael,

I think you saw my other e-mail, but just in case you haven't, do you
have any thoughts about making this an option that could be easily
disabled on the command line without maintaing a patch to lit? I think
it would help out-of-tree target maintainers to transition, since I'm
sure there will be a lot of similarly broken tests to fix.

Stephen

On Thu, Jul 4, 2013 at 8:56 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> We currently don't use pipefail when running test under make check.
> This has the undesirable property that it is really easy for tests to
> bitrot. For example, something like
>
> llc %s | FileCheck %s
>
> will still pass if llc crashes after printing what FileCheck was
> looking for. It is also easy to break the tests when refactoring. I
> have fixed tests that were doing
>
> %clang_cc1 -a-driver-options ... | not grep
>
> clearly the test was changed from %clang to %clang_cc1 and we missed
> the fact that the option also had to be updated.
>
> Currently to check a command output and that it doesn't crash we have
to do
>
> llc %s > %t
> FileCheck %s < %t
>
> I would like to switch to using pipefail instead. That would meant that a
simple
>
> llc %s | FileCheck %s
>
> would check both llc return value and output. I have already cleared
> all the tests, so all that is missing is changing lit itself. Any
> objections to doing so?
>
> Cheers,
> Rafael
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On 5 July 2013 00:15, Stephen Lin <swlin at post.harvard.edu>
wrote:
> Hi Rafael,
>
> I think you saw my other e-mail, but just in case you haven't, do you
> have any thoughts about making this an option that could be easily
> disabled on the command line without maintaing a patch to lit? I think
> it would help out-of-tree target maintainers to transition, since I'm
> sure there will be a lot of similarly broken tests to fix.
I don't think it is all that many since it was less than one day of
work for the in tree ones. But if there is the desire for such an
option I can try to add it. What should I use? An environment
variable?
> Stephen
>
Cheers,
Rafael

On Thu, Jul 4, 2013 at 8:56 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> We currently don't use pipefail when running test under make check.
> This has the undesirable property that it is really easy for tests to
> bitrot.
Hi Rafael,

Did this discussion ever get a conclusion?  I support enabling
pipefail.  Fallout for out of tree users should be easy to fix.  As we
learned from LLVM tests, almost all tests that start to fail actually
indicate a real problem that was hidden.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at
gmail.com>*/

> Hi Rafael,
>
> Did this discussion ever get a conclusion?  I support enabling
> pipefail.  Fallout for out of tree users should be easy to fix.  As we
> learned from LLVM tests, almost all tests that start to fail actually
> indicate a real problem that was hidden.
So far I got some positive feedback, but no strong LGTM from someone
in the area :-(
> Dmitri
>

Cheers,
Rafael