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

[David Blaikie]

On Thu, Jan 17, 2013 at 10:51 AM, Daniel Dunbar <daniel at zuster.org> wrote:
> 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.

SGTM - though if we are going to go with the plan/features you've
outlined, it might not be so unreasonable to, rather than tunnelling
the log files through the lit output & splitting them back out, do the
work to actually gather those log files directly (it's a bit more work
in the buildbot configuration - enough that I didn't think I could
justify it given Dmitri's original proposal - but seems like it would
simplify this change & leave us roughly where I was discussing earlier
(though your suggestion of generalizing this over all of lit, rather
than just FileCheck is a step beyond what I'd proposed - it sounds
good/right though))

I asked about how to do this in the freenode buildbot channel & they
mentioned that it is possible to name log files dynamically to be
retrieved from the slave - I haven't looked into it in detail because
it did sound a bit more complicated, but should be achievable.

The drawback to your approach is that we'd have to enable this feature
unconditionally - rather than having the optimization advantage of
only dumping files on failure (see my question earlier in the thread,
Eli's concern that dumping output would be expensive, and Dmitri's
response that we'd only be dumping on failure anyway). Given that it
seems the vast majority of our failures aren't flakey, we could have
lit setup to rerun failures in a "create all the temporary files"
mode, though missing flakes would be unfortunate.

>
>  - 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>