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

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 6 13:46:13 PDT 2018


Doesn’t mean it’s a good or bad pattern, just that “only 1 file will use
it” is already false, because we can make lldb use it as a followup and
that will get many more.

I did think about alternative solutions, If this CL wasn’t here, the
alternatives i came up would be to either not write this test, or spend
several days/weeks writing a tool whose only purpose was to make it
possible to write this test.
On Thu, Sep 6, 2018 at 1:32 PM Nico Weber via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180906/c8c5c7f8/attachment.html>


More information about the llvm-commits mailing list