[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 11:32:02 PDT 2018


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 would be nice to have a standalone executable that can be lit-run but
it’s a lot of work for little gain in some cases. Fwiw this pattern is also
prevalent in lldb with minidump tests. There too it would be nice to have a
dedicated test executable, but again it’s a lot of work. In this particular
case, it’s even questionable how appropriate that would be, because I’m
really testing an implementation detail, which is what unit tests are for),
but it requires an input file to exercise. Mocking up a “fake” pdb file
class is another option, but that’s also a lot of work and the solution
here is pretty straightforward and “just works”
On Thu, Sep 6, 2018 at 11:19 AM Reid Kleckner via Phabricator <
reviews at reviews.llvm.org> wrote:

> rnk added a comment.
>
> In https://reviews.llvm.org/D51561#1226155, @thakis wrote:
>
> > Sorry, didn't see this earlier. Writing a llvm.srcdir.txt next to every
> single unit test binary seems like a pretty roundabout way of doing this.
> How about instead passing the src dir in a runtime flag in
> http://llvm-cs.pcc.me.uk/utils/lit/lit/formats/googletest.py#107 if the
> lit config asks for it? Then you don't need to touch the disk to get the
> src dir path, it's not passed to _all_ binaries, and cmake doesn't have to
> write a file to disk for every test binary.
>
>
> To avoid CMake writing so many files, we could make this opt-in. A
> unittest could pass an option to `add_unittest` or call another cmake
> function to configure the text file. Or, CMake could test for the presence
> of $srcdir/Inputs before writing the text file.
>
> I don't think we want to get lit to pass a flag to gtest binaries. Gtest
> binaries have the nice property that you just run them and they work. It
> would be a shame to lose that.
>
> In https://reviews.llvm.org/D51561#1226160, @thakis wrote:
>
> > Also also, opening a file from a unit test kind of makes it not a unit
> test, since those are supposed to not hit the disk etc. Maybe that test
> wants to be its own binary instead that's called from a lit script, instead
> of being a unit test?
>
>
> 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.
>
> ---
>
> What are you most concerned about, files littering the build directory,
> cmake runtime, simplicity, or something else?
>
>
> 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/bcb5cc29/attachment.html>


More information about the llvm-commits mailing list