[PATCH] D51561: [CMake] Add support for unittests that have input files

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

> We may call these "unit" tests, but they're really gtests, and developers everywhere use gtest for more than just unit tests. We already have tests that create temp files. I don't see why reaching back into the source directory for inputs is too heavyweight for us.

That's definitely true at Google because it's the only hammer we have there. In LLVM land, we have lit, and gtests really tend to be unit tests there.

> Also there’s not really that many of these files, and they’re very small.
>  Making it opt-in seems more troue than it’s worth because there’s not going
>  to be a measurable impact of writing the files.

It's one per test binary, so about 60. I agree that writing 60 files won't have a big time cost, but writing 60 files because a single gtest wants to access a file in the source tree seems questionable to me.

Furthermore, it's not a move that has good follow-up moves:

- If this doesn't get widely adopted (as he commit message hopes: "and we just encourage people to use this sparingly") then we have this weird magical setup that's used by one file.

- If it does get widely adopted, then now we have lots of tests reading stuff from the disk, where writing a binary running under lit would almost certainly lead to a better long term design.

So I'd encourage you to think about the question "if this CL here wasn't a possible approach, how could I still do what I want?" and do that instead.

> Fwiw this pattern is also prevalent in lldb with minidump tests.

I'm not sure if "lldb has it, so it's a good pattern" is necessarily a good argument.


Repository:
  rL LLVM

https://reviews.llvm.org/D51561