llvm dev - Jan 2013 - [LLVMdev] [llvm-commits] [PATCH] A "very verbose" mode for FileCheck

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

On Wed, Jan 16, 2013 at 10:43 PM, Chris Lattner <clattner at apple.com>
wrote:
> 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.
That was a suggestion/request from me - my thinking was that, pretty
much whenever that happens, you end up wanting to look at the output
anyway. My usual process involves copy/pasting the command (if I can
tell which one it is, since lit dumps all the RUN lines out, I think)
& running it to look at the output to see what went wrong. It seems
having that output already available would skip that step - is this
less common for other developers? Is there some other solution that
might address this?

(I suggested this in part in the hopes that we could find a unified
solution for local and buildbot execution, but on consideration I
couldn't find a particularly easy way to gather the extra log file
back to the build master so Dmitri's solution still seemed
necessary/convenient - making my suggestion perhaps orthogonal to his
current proposal)
>
>> 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

On Thu, Jan 17, 2013 at 8:43 AM, Chris Lattner <clattner at apple.com>
wrote:
> 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?
Hello Daniel & llvm-commits,

We are implementing a "very verbose" for FileCheck, so that FileCheck
will dump the input to stderr in case of mismatch.  This will make it
easier to debug failures on remote buildbots.

This will be a separate mode in FileCheck, activated by "FileCheck
--dump-input-on-error".  lit should add that option to FileCheck
invocations on buildbots (lit can see if it is running on a buildbot
by checking an environment variable).

We have to options:
(a) replace 'FileCheck' with '%FileCheck' in all tests, and
teach
'lit' to replace '%FileCheck' with 'FileCheck
--dump-input-on-error';

(b) teach 'lit' to replace a plain 'FileCheck'.

The first approach seems cleaner to developers who read and write
tests (it suggests that they are invoking some "macro" -- but does
that matter?)  The second approach is much easier to implement since
tests will be unchanged.

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 8:36 AM, Dmitri Gribenko <gribozavr at gmail.com>
wrote:
> We have to options:
> (a) replace 'FileCheck' with '%FileCheck' in all tests, and
teach
> 'lit' to replace '%FileCheck' with 'FileCheck
--dump-input-on-error';
>
> (b) teach 'lit' to replace a plain 'FileCheck'.
>
> The first approach seems cleaner to developers who read and write
> tests (it suggests that they are invoking some "macro" -- but
does
> that matter?)  The second approach is much easier to implement since
> tests will be unchanged.
IMO the biggest issue with (a) is that developers will continue to use
`FileCheck` instead of `%FileCheck`. So IMO (a) should only be
implemented if simultaneously there is a change that makes just plain
`FileCheck` an error.

Another approach would be to have a script or symlink
`FileCheck-dump-input-on-error` which lives in build/bin/ alongside
FileCheck, and which just invokes FileCheck in a way that causes it to
go into the "dump input on error" mode (e.g. it checks argv[0], or a
magic environment variable, or whatever). On the build bots,
FileCheck-dump-input-on-error and regular FileCheck could be swapped
by a configure option. That's kind of ugly, but some variation of it
may be more palatable.

-- Sean Silva

Hi Dimitri,

This has long been on my list of nice things to solve, thanks for tackling
it.

I actually would prefer to solve this problem in a completely different
way, however, that is not specific to FileCheck.

This is how I would like this to work:

1. We extend 'lit' to support associating files with test results. This
is
something that is good to have for other reasons, anyway.

2. We provide a mode to lit which captures verbose test output. In those
mode, it does several things:

a. Any pipes are replaced with temporary files.
b. When tests fail, any temporary files (either from pipes or from %t
markers) are included with the test results.

3. We extend the 'lit' test output to have a clear way to delineate
additional output files, and include them in the test output (maybe with
some size limits).

4. We extend the buildbot 'lit' test runner to be smart enough to
extract
these files out of the 'lit' test output, and attach them to the
buildbot
results.

While this proposal probably involves more work than the yours, here are
the reasons I prefer it:

1. It extends to all 'lit' uses, not just FileCheck.

2. It doesn't require modifying the tests.

3. Some of the extra work (like extending lit to associate files with test
results) is useful infrastructure for future extensions.

What do you think?

 - Daniel



On Thu, Jan 17, 2013 at 5:36 AM, Dmitri Gribenko <gribozavr at
gmail.com>wrote:
> On Thu, Jan 17, 2013 at 8:43 AM, Chris Lattner <clattner at
apple.com> wrote:
> > 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?
>
> Hello Daniel & llvm-commits,
>
> We are implementing a "very verbose" for FileCheck, so that
FileCheck
> will dump the input to stderr in case of mismatch.  This will make it
> easier to debug failures on remote buildbots.
>
> This will be a separate mode in FileCheck, activated by "FileCheck
> --dump-input-on-error".  lit should add that option to FileCheck
> invocations on buildbots (lit can see if it is running on a buildbot
> by checking an environment variable).
>
> We have to options:
> (a) replace 'FileCheck' with '%FileCheck' in all tests, and
teach
> 'lit' to replace '%FileCheck' with 'FileCheck
--dump-input-on-error';
>
> (b) teach 'lit' to replace a plain 'FileCheck'.
>
> The first approach seems cleaner to developers who read and write
> tests (it suggests that they are invoking some "macro" -- but
does
> that matter?)  The second approach is much easier to implement since
> tests will be unchanged.
>
> 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 --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130117/b9f886f4/attachment.html>