[PATCH] D51561: [CMake] Add support for unittests that have input files
Nico Weber via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 14 06:04:37 PDT 2018
thakis added a comment.
In https://reviews.llvm.org/D51561#1227163, @thakis wrote:
> In https://reviews.llvm.org/D51561#1226583, @zturner wrote:
>
> > These aren't demangle tests fwiw. But that just complicates the cmake
> > logic, and I don't see the benefit of doing so. Writing 3k of data to the
> > disk is hardly going to be a noticeable part of the configure step.
>
>
> The benefits are:
>
> 1. Unit tests reading inputs are bad form; doing that should require an explicit opt-in.
> 2. While writing 3k of data to disk isn't causing measurable perf issues, writing a text file next to every test binary just because one test binary needs it is still plain ugly.
>
> And not doing it is super easy to do. I don't understand the pushback here.
^ zturner: Ping on this.
Unrelatedly, another problem with the current design is that llvm.srcdir.txt must contain an absolute path, which makes it impossible to build on one machine and then copy artifacts and test inputs to another machine and run tests on another machine. DebugInfoPDBTests is the only test in all llvm, lld, clang tests that has this restriction. Requiring some defined fixed pwd for unit tests with inputs would address this issue as well. (As would finding a way to not require reading a file off disk for this test.)
Repository:
rL LLVM
https://reviews.llvm.org/D51561
More information about the llvm-commits
mailing list