Dmitri Gribenko
2013-Jan-16 19:33 UTC
[LLVMdev] [PATCH] A "very verbose" mode for FileCheck
On Wed, Jan 16, 2013 at 9:24 PM, Chris Lattner <clattner at apple.com> wrote:> > On Jan 16, 2013, at 10:32 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote: > >> Hello, >> >> When someone breaks a FileCheck-based test on some buildbot, sometimes >> it may not be obvious *why* did it fail. If the failure can not be >> reproduced locally, it can be very hard to fix. >> >> I propose adding a "very verbose" mode to FileCheck. In this mode >> FileCheck will dump the input file in case of failure. This mode will >> be enabled by an environment variable "FILECHECK_VERY_VERBOSE". If we >> chose a command line option, we would have to edit all FileCheck-based >> tests to use %FileCheck. > > I think that this idea is good, but I'd prefer it be implemented a different way: > > - Filecheck should take a new flag -dump-input-on-error that causes it to... dump the input file on error. > - Lit should be the thing that checks the environment (or perhaps add a new option to lit), and adds the flag to FileCheck invocations. > > I don't like it when the behavior of such a low-level tool like this changes based on environment variables. It isn't discoverable in --help. If for some reason, it is bad for lit to implicitly pass the option, I'd rather have a standard FILECHECK_COMMANDLINE environment variable, and have filecheck parse arbitrary options out of it using the cl::ParseEnvironmentOptions function.I agree that a command line option would be better. But in that case all tests should be updated. It is not an issue for me -- it is mostly mechanical. So should I change tests to use %FileCheck? 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>*/
Dmitri Gribenko
2013-Jan-16 21:19 UTC
[LLVMdev] [PATCH] A "very verbose" mode for FileCheck
On Wed, Jan 16, 2013 at 9:33 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:> On Wed, Jan 16, 2013 at 9:24 PM, Chris Lattner <clattner at apple.com> wrote: >> >> On Jan 16, 2013, at 10:32 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote: >> >>> Hello, >>> >>> When someone breaks a FileCheck-based test on some buildbot, sometimes >>> it may not be obvious *why* did it fail. If the failure can not be >>> reproduced locally, it can be very hard to fix. >>> >>> I propose adding a "very verbose" mode to FileCheck. In this mode >>> FileCheck will dump the input file in case of failure. This mode will >>> be enabled by an environment variable "FILECHECK_VERY_VERBOSE". If we >>> chose a command line option, we would have to edit all FileCheck-based >>> tests to use %FileCheck. >> >> I think that this idea is good, but I'd prefer it be implemented a different way: >> >> - Filecheck should take a new flag -dump-input-on-error that causes it to... dump the input file on error. >> - Lit should be the thing that checks the environment (or perhaps add a new option to lit), and adds the flag to FileCheck invocations. >> >> I don't like it when the behavior of such a low-level tool like this changes based on environment variables. It isn't discoverable in --help. If for some reason, it is bad for lit to implicitly pass the option, I'd rather have a standard FILECHECK_COMMANDLINE environment variable, and have filecheck parse arbitrary options out of it using the cl::ParseEnvironmentOptions function. > > I agree that a command line option would be better. But in that case > all tests should be updated. It is not an issue for me -- it is > mostly mechanical. So should I change tests to use %FileCheck?Here's a third attempt. The new behavior is as follows: 1. In case of errors we always dump output to a temporary file and print Saving input file "<stdin>" to "/tmp/filecheck.txt-Jno73y" 2. If --dump-input-on-error option is passed, FileCheck will also dump input to stderr. 3. If FILECHECK_DUMP_INPUT_ON_ERROR env var is set, lit will replace "%FileCheck" with "FileCheck --dump-input-on-error". I will fix all tests in LLVM and Clang if we decide this is the way to go. 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>*/ -------------- next part -------------- A non-text attachment was scrubbed... Name: filecheck-very-verbose-v3.patch Type: application/octet-stream Size: 4865 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130116/ca9b626b/attachment.obj>
Evgeniy Stepanov
2013-Jan-16 23:23 UTC
[LLVMdev] [llvm-commits] [PATCH] A "very verbose" mode for FileCheck
On Thu, Jan 17, 2013 at 1:19 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:> On Wed, Jan 16, 2013 at 9:33 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote: >> On Wed, Jan 16, 2013 at 9:24 PM, Chris Lattner <clattner at apple.com> wrote: >>> >>> On Jan 16, 2013, at 10:32 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote: >>> >>>> Hello, >>>> >>>> When someone breaks a FileCheck-based test on some buildbot, sometimes >>>> it may not be obvious *why* did it fail. If the failure can not be >>>> reproduced locally, it can be very hard to fix. >>>> >>>> I propose adding a "very verbose" mode to FileCheck. In this mode >>>> FileCheck will dump the input file in case of failure. This mode will >>>> be enabled by an environment variable "FILECHECK_VERY_VERBOSE". If we >>>> chose a command line option, we would have to edit all FileCheck-based >>>> tests to use %FileCheck. >>> >>> I think that this idea is good, but I'd prefer it be implemented a different way: >>> >>> - Filecheck should take a new flag -dump-input-on-error that causes it to... dump the input file on error. >>> - Lit should be the thing that checks the environment (or perhaps add a new option to lit), and adds the flag to FileCheck invocations. >>> >>> I don't like it when the behavior of such a low-level tool like this changes based on environment variables. It isn't discoverable in --help. If for some reason, it is bad for lit to implicitly pass the option, I'd rather have a standard FILECHECK_COMMANDLINE environment variable, and have filecheck parse arbitrary options out of it using the cl::ParseEnvironmentOptions function. >> >> I agree that a command line option would be better. But in that case >> all tests should be updated. It is not an issue for me -- it is >> mostly mechanical. So should I change tests to use %FileCheck? > > Here's a third attempt. > > The new behavior is as follows: > > 1. In case of errors we always dump output to a temporary file and printDoes it mean we get one more file in /tmp every time a test fails, and it is not cleaned up automatically? I don't think this should happen in the "default" mode of the tool. Other than that, the idea sounds awesome, and I'd be happy to see any variation of it on the bots ASAP.> > Saving input file "<stdin>" to "/tmp/filecheck.txt-Jno73y" > > 2. If --dump-input-on-error option is passed, FileCheck will also dump > input to stderr. > > 3. If FILECHECK_DUMP_INPUT_ON_ERROR env var is set, lit will replace > "%FileCheck" with "FileCheck --dump-input-on-error". > > I will fix all tests in LLVM and Clang if we decide this is the way to go. > > 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>*/ > > _______________________________________________ > llvm-commits mailing list > llvm-commits at cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >
On Jan 16, 2013, at 1:19 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:>> I agree that a command line option would be better. But in that case >> all tests should be updated. It is not an issue for me -- it is >> mostly mechanical. So should I change tests to use %FileCheck? > > Here's a third attempt.Thanks in advance for driving this forward. I'm sorry that such a simple thing is so complicated.> The new behavior is as follows: > > 1. In case of errors we always dump output to a temporary file and print > > Saving input file "<stdin>" to "/tmp/filecheck.txt-Jno73y"This doesn't make sense to me. It's really common in a normal development scenario to do something, test and have stuff fail. It doesn't make sense to dump things into /tmp in this case.> 2. If --dump-input-on-error option is passed, FileCheck will also dump > input to stderr.This is fine.> 3. If FILECHECK_DUMP_INPUT_ON_ERROR env var is set, lit will replace > "%FileCheck" with "FileCheck --dump-input-on-error".Sounds good for lit.> I will fix all tests in LLVM and Clang if we decide this is the way to go.What tests need to be fixed? FileCheck -> %FileCheck? You should check with Daniel, but would it make sense to have lit just "know" FileCheck? -Chris
Seemingly Similar Threads
- [LLVMdev] [llvm-commits] [PATCH] A "very verbose" mode for FileCheck
- [LLVMdev] [llvm-commits] [PATCH] A "very verbose" mode for FileCheck
- [LLVMdev] [llvm-commits] [PATCH] A "very verbose" mode for FileCheck
- [LLVMdev] [PATCH] A "very verbose" mode for FileCheck
- [LLVMdev] [PATCH] A "very verbose" mode for FileCheck