[PATCH] D56541: Fix FileCheck tests when `FILECHECK_DUMP_INPUT_ON_FAILURE` is set in the environment that llvm-lit is executed in.

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 10 07:33:07 PST 2019


delcypher added a comment.

In D56541#1352721 <https://reviews.llvm.org/D56541#1352721>, @jdenny wrote:

> I've been wanting to fix this too.  Thanks for working on it.  A few issues:
>
> - `FILECHECK_OPTS` should be cleared too.


Sure I can fix that one.

> 
> 
> - The last time I checked (but it's been a little while), the lit test suite had problems with these variables too.

LLVM and lit's test suites pass for me

  FILECHECK_DUMP_INPUT_ON_FAILURE=1 bin/llvm-lit -vs test/
  FILECHECK_DUMP_INPUT_ON_FAILURE=1 bin/llvm-lit -v utils/lit/



> 
> 
> - This approach makes these environment variables unhelpful for this test suite, but they don't have to be.  I think a better solution is to create a command, perhaps a lit substitution named FileCheckee, that clears these environment variables and then calls FileCheck.  Throughout this test suite, any FileCheck call under test would be replaced with FileCheckee.  Other FileCheck calls, whose job is to verify correct output from FileCheckee or from other commands, would still be affected by these environment variables.

Okay I get the idea. This is a much more invasive change however. Would it be okay to commit this (with `FILECHECK_OPTS` also stripped out) and do your suggestion as a follow up patch?
The reason I'd prefer to do this is because this change is intended to fix some internal build bots. Due to where we are in the release cycle the changes I make need to be simple (i.e. the smaller the patch the better) in order to be accepted by internal reviewers.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56541





More information about the llvm-commits mailing list