On 08/12/2013 13:12, Chandler Carruth wrote:> >> * Removing trailing \ will introduce the neat property that >> one RUN line corresponds precisely to one command that's >> executed. This is good for humans and will enable >> simplifications in the test runner. >> >> FWIW, I've never really had a problem that needed this. The RUN: >> forms a prefix of a shell script in my head, and I know how to >> read shell scripts including multiple lines. > > The transformations lit does are really too complex and there's at > least one known bug to do with closed pipes that's contributing to > no-op tests (think the discussion thread was on cfe-dev). > > In a nutshell, the script output lit forms right now is not likely > not the pipeline you had in your head ;-) > > > I understand that you think this is too complex, but I'm suggesting > that this particular aspect of lit does not seem too complex to at > least one other developer, and thus you shouldn't assume it to be true.It's great if we've made it all look simple to you. Unfortunately for the developers there's an ongoing problem with algorithmic complexity in lit hiding problems that lead to broken tests. In particular, there are constructs that would error out in a shell but get silently accepted by the lit runner. See Sean Silva's observation of one of these cases in the thread on no-op tests on cfe-dev: On 08/11/2013 06:48, Sean Silva wrote:> Although it doesn't eliminate the hassle of having to manually fix > this, it seems like at least one issue worth fixing in its own right > is the fact that RUN lines ending with a | are silently accepted by > our test infrastructure.This proposal is about resolving a class of these problems, and I'd like to see if we can get it done early in the 3.5 cycle given there's a degree of churn. It's ultimately about weighing up the cost/benefit of wrapped RUN lines. The benefit you pointed out so far has been visual wrapping in the editor, but the cost of that is very high. Perhaps your editor has a virtual wrapping mode? With one-to-one mapping, it becomes possible to use simple tools like grep to validate common mistakes like %clang / %clang_cc1 mixups, a missing -o flag and so on. Right now there's no obvious way to do those checks and we've ended up without an easy way to lint for broken tests as a result. Each broken test has a high cost so we need to continually look at ways to improve the situation. Alp. -- http://www.nuanti.com the browser experts
On Sun, Dec 8, 2013 at 6:04 AM, Alp Toker <alp at nuanti.com> wrote:> > On 08/12/2013 13:12, Chandler Carruth wrote: > >> >> * Removing trailing \ will introduce the neat property that >>> >>> one RUN line corresponds precisely to one command that's >>> executed. This is good for humans and will enable >>> simplifications in the test runner. >>> >>> FWIW, I've never really had a problem that needed this. The RUN: >>> forms a prefix of a shell script in my head, and I know how to >>> read shell scripts including multiple lines. >>> >> >> The transformations lit does are really too complex and there's at >> least one known bug to do with closed pipes that's contributing to >> no-op tests (think the discussion thread was on cfe-dev). >> >> In a nutshell, the script output lit forms right now is not likely >> not the pipeline you had in your head ;-) >> >> >> I understand that you think this is too complex, but I'm suggesting that >> this particular aspect of lit does not seem too complex to at least one >> other developer, and thus you shouldn't assume it to be true. >> > > > It's great if we've made it all look simple to you. Unfortunately for the > developers there's an ongoing problem with algorithmic complexity in lit > hiding problems that lead to broken tests. > > In particular, there are constructs that would error out in a shell but > get silently accepted by the lit runner. >Then we should just fix that bug. I would be very happy to see lit's shell support improved to more closely match the shell for validation.> > See Sean Silva's observation of one of these cases in the thread on no-op > tests on cfe-dev: > > On 08/11/2013 06:48, Sean Silva wrote: > >> Although it doesn't eliminate the hassle of having to manually fix this, >> it seems like at least one issue worth fixing in its own right is the fact >> that RUN lines ending with a | are silently accepted by our test >> infrastructure. >> > > This proposal is about resolving a class of these problems, and I'd like > to see if we can get it done early in the 3.5 cycle given there's a degree > of churn. > > It's ultimately about weighing up the cost/benefit of wrapped RUN lines. > The benefit you pointed out so far has been visual wrapping in the editor, > but the cost of that is very high. Perhaps your editor has a virtual > wrapping mode? > > With one-to-one mapping, it becomes possible to use simple tools like grep > to validate common mistakes like %clang / %clang_cc1 mixups, a missing -o > flag and so on. > > Right now there's no obvious way to do those checks and we've ended up > without an easy way to lint for broken tests as a result. Each broken test > has a high cost so we need to continually look at ways to improve the > situation.Can you elaborate on exactly what kind of checks you want to do? - Daniel> > > Alp. > > -- > http://www.nuanti.com > the browser experts > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131208/7d660db0/attachment.html>
On 08/12/2013 16:43, Daniel Dunbar wrote:> > With one-to-one mapping, it becomes possible to use simple tools > like grep to validate common mistakes like %clang / %clang_cc1 > mixups, a missing -o flag and so on. > > Right now there's no obvious way to do those checks and we've > ended up without an easy way to lint for broken tests as a result. > Each broken test has a high cost so we need to continually look at > ways to improve the situation. > > > Can you elaborate on exactly what kind of checks you want to do?Basically, it's for asking questions about what's currently tested.. e.g. /Find me all tests for c++11 exceptions/: |git grep 'RUN:.*c++11' -- test/ | grep exceptions| .. experimenting with new features or refactoring existing tests.. e.g. /How many tests could we add -fsyntax-only to?/ |giteach perl -pi -e 's/((RUN:.*%clang_cc1)(.*))$/$2 -fsyntax-only$3/g unless m/(-fsyntax-only|-analyze|-arcmt-check|-arcmt-migrate|-o|-emit-llvm|-E|-ast-print|-ast-dump|[|>]|\\$)/'| Answer, 405. Then git diff and page through to see if any are actionable. and fixing broken tests.. eg. /Let's find and fix %clang_cc1 substitution mistakes../ giteach perl -pi -e 's/(RUN:.*%clang_cc1)[^\s]+/$1/g' I committed the results of this run just a few minutes ago in r196729 and r196730. One was a test that had been disabled since 2007. Losing 80 columns is a small price to pay if it'll help make tests easier to write, understand and validate in my opinion. (The two editors I use, vim and XCode, both do a good job of virtual line wrapping so it turns out long RUN lines aren't that much of a big deal in practice. But I can see why it may be a bigger problem with editors that don't support wrapping. Is this still a problem in 2013?) Alp. -- http://www.nuanti.com the browser experts -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131208/d0474880/attachment.html>
On Sun, Dec 8, 2013 at 9:04 AM, Alp Toker <alp at nuanti.com> wrote:> > On 08/12/2013 13:12, Chandler Carruth wrote: > >> >> * Removing trailing \ will introduce the neat property that >>> >>> one RUN line corresponds precisely to one command that's >>> executed. This is good for humans and will enable >>> simplifications in the test runner. >>> >>> FWIW, I've never really had a problem that needed this. The RUN: >>> forms a prefix of a shell script in my head, and I know how to >>> read shell scripts including multiple lines. >>> >> >> The transformations lit does are really too complex and there's at >> least one known bug to do with closed pipes that's contributing to >> no-op tests (think the discussion thread was on cfe-dev). >> >> In a nutshell, the script output lit forms right now is not likely >> not the pipeline you had in your head ;-) >> >> >> I understand that you think this is too complex, but I'm suggesting that >> this particular aspect of lit does not seem too complex to at least one >> other developer, and thus you shouldn't assume it to be true. >> > > > It's great if we've made it all look simple to you. Unfortunately for the > developers there's an ongoing problem with algorithmic complexity in lit > hiding problems that lead to broken tests. > > In particular, there are constructs that would error out in a shell but > get silently accepted by the lit runner. > > See Sean Silva's observation of one of these cases in the thread on no-op > tests on cfe-dev: > > On 08/11/2013 06:48, Sean Silva wrote: > >> Although it doesn't eliminate the hassle of having to manually fix this, >> it seems like at least one issue worth fixing in its own right is the fact >> that RUN lines ending with a | are silently accepted by our test >> infrastructure. >> > > This proposal is about resolving a class of these problems, and I'd like > to see if we can get it done early in the 3.5 cycle given there's a degree > of churn. > > It's ultimately about weighing up the cost/benefit of wrapped RUN lines. > The benefit you pointed out so far has been visual wrapping in the editor, > but the cost of that is very high. Perhaps your editor has a virtual > wrapping mode? > > With one-to-one mapping, it becomes possible to use simple tools like grep > to validate common mistakes like %clang / %clang_cc1 mixups, a missing -o > flag and so on. > > Right now there's no obvious way to do those checks and we've ended up > without an easy way to lint for broken tests as a result. Each broken test > has a high cost so we need to continually look at ways to improve the > situation.The classic way to do this sort of checking is by hacking into the tool that actually interprets it (i.e. lit in this case). Considering that lit is Python, it should be pretty easy to insert an ad-hoc regex check (or even something substantially more sophisticated). E.g. insert code into TestRunner.py's parseIntegratedTestScript function. -- Sean Silva> > > Alp. > > -- > http://www.nuanti.com > the browser experts > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131209/eb88977e/attachment.html>
On 10/12/2013 04:20, Sean Silva wrote:> The classic way to do this sort of checking is by hacking into the > tool that actually interprets it (i.e. lit in this case). Considering > that lit is Python, it should be pretty easy to insert an ad-hoc regex > check (or even something substantially more sophisticated). E.g. > insert code into TestRunner.py's parseIntegratedTestScript function.Hi Sean, Let's try to work on that then. It'll be brilliant if we can make this work without churn in the test suite. I had some progress trying to get that working by building a source line table and moving substitutions to later in the processing pipeline so all the source information to map changes back are actually available now. The difficulty is in getting the regular expression engine to use that to preserve source locations so we can apply the changes back to the original source. I've been digging into the depths of the Python regex implementation and tried inserting line break markers together with the multiline option but it didn't work out -- if can help find a way to do that then I think all the information we need is there to map back to the original lines and apply changes.. Alp. -- http://www.nuanti.com the browser experts