[PATCH] D81422: Change filecheck default to dump input on failure

Mon Jun 8 16:06:40 PDT 2020

jdenny added a comment.

In D81422#2080861 <https://reviews.llvm.org/D81422#2080861>, @mehdi_amini wrote:

> In D81422#2080758 <https://reviews.llvm.org/D81422#2080758>, @probinson wrote:
>
> > I don't remember the exact reasoning but I believe it had something to do with bot logs?  @jdenny or @thopre might remember.
>

The only problem I've seen is the extreme verbosity when there are many failures.  For example, I have experienced cases where that verbosity hung an IDE that was collecting the output.  But if people have problems, they can set `FILECHECK_OPTS=-dump-input=never`, so it's probably fine.

> It'd be interesting to hear about it: having the bot log including the input on failure could indeed help debugging when something fails in flaky way, or is dependent on the environment (like the path where the code is checked out, etc.). 
>  At least in the past it could have saved me trouble when only a single bot was failing in mysterious way and the bot owner had to be involved to help figure out.
> 
> (also right now we're inconsistent: many tests are adding the flag explicitly)

I imagined more and more bots would set this eventually via FILECHECK_OPTS.  Changing the default seems fine too.

================
Comment at: llvm/test/FileCheck/dump-input-enable.txt:78
 ; Check no -dump-input, which defaults to never.
 ;--------------------------------------------------

----------------
Are the tests in this section passing for you now that the default is fail?

================
Comment at: llvm/test/FileCheck/dump-input-enable.txt:127
+; Check -dump-input=fail
 ;--------------------------------------------------

----------------
The preceding section checks `-dump-input=fail`, so you can probably just remove this `-dump-input-on-failure` section.

================
Comment at: llvm/test/FileCheck/dump-input-enable.txt:142
+; RUN:               -match-full-lines  -v 2>&1 \
+; RUN: | FileCheck %s -dump-input=never -match-full-lines \
 ; RUN:                -check-prefixes=NOTRACE,ERR,DUMP-ERR,DUMP-ERR-V
----------------
Why is `-dump-input=never` needed here?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422