Rafael Espíndola
2013-Jul-05 03:56 UTC
[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
Stephen Lin
2013-Jul-05 04:15 UTC
[LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
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
Rafael Espíndola
2013-Jul-05 04:40 UTC
[LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
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
Dmitri Gribenko
2013-Jul-18 00:22 UTC
[LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
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>*/
Rafael Espíndola
2013-Jul-18 01:48 UTC
[LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
> 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
Reasonably Related Threads
- [LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
- [LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
- [LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
- [LLVMdev] [RFC] Switching make check to use 'set -o pipefail'
- [LLVMdev] Regression tests now using pipefail