llvm dev - Jan 2013 - [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 print
Does 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 Thu, Jan 17, 2013 at 1:23 AM, Evgeniy Stepanov
<eugeni.stepanov at gmail.com> wrote:
> 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
print
>
> Does 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.
Well, yes.  David requested that and I agreed that it is a good idea.
Are you strongly opposed to it?

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>*/

On Thu, Jan 17, 2013 at 3:31 AM, Dmitri Gribenko <gribozavr at gmail.com>
wrote:
> On Thu, Jan 17, 2013 at 1:23 AM, Evgeniy Stepanov
> <eugeni.stepanov at gmail.com> wrote:
>> 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
print
>>
>> Does 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.
>
> Well, yes.  David requested that and I agreed that it is a good idea.
> Are you strongly opposed to it?
Not really, I doubt it can break something for someone...

But if we only need this behaviour on the buildbot, and you are going
to update all tests anyway, why not make it conditional on a flag?

Note that as far as places to put temporary files, the right place to put
them is alongside the other test outputs in the test output "sandbox"
directory.

Somewhat orthogonal, but we should also fix up lit to purge those sandboxes
before it starts a new test run.

 - Daniel


On Wed, Jan 16, 2013 at 3:31 PM, Dmitri Gribenko <gribozavr at
gmail.com>wrote:
> On Thu, Jan 17, 2013 at 1:23 AM, Evgeniy Stepanov
> <eugeni.stepanov at gmail.com> wrote:
> > 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
print
> >
> > Does 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.
>
> Well, yes.  David requested that and I agreed that it is a good idea.
> Are you strongly opposed to it?
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130117/02d27f26/attachment.html>