[PATCH] D80377: [flang] Google test infrastructure support for unittests

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: flang/CMakeLists.txt:128
+  set(UNITTEST_DIR ${LLVM_BUILD_MAIN_SRC_DIR}/utils/unittest)
+  if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h)
+    if (TARGET gtest)
----------------
tskeith wrote:
> sameeranjoshi wrote:
> > tskeith wrote:
> > > Why do you have to check for `gtest.h`? Won't it always be there?
> > Not always because
> > 
> > > 
> > > The out-of-tree build can be configured against the CMAKE_INSTALL_PREFIX directory of llvm, rather than the build directory. 
> > > LLVM does not install gtest when installing itself, meaning that gtest might not be available.
> > > 
> > 
> > Also other subprojects as well follow something similar
> > [1]https://github.com/llvm/llvm-project/blob/8e058feae0b0d07cd86257f0aa3154acfa887fe0/clang/CMakeLists.txt#L188
> > [2]https://github.com/llvm/llvm-project/blob/82aac878beb48cd326b4684918b7ff2375fae439/polly/CMakeLists.txt#L29
> > [3]https://github.com/llvm/llvm-project/blob/82aac878beb48cd326b4684918b7ff2375fae439/lld/CMakeLists.txt#L109
> > 
> > 
> If the user requests unit tests and we can't run them it should be an error, not silently ignored.
If the user is running `make FlangUnitTests`, yes. The build target may not exist in this case. But failing on `make check-flang` would mean the build cannot be verified at all if configuring against a pre-installed LLVM. If not supported, I would question the point of a flang standalone build. 

Note that it is never possible to run all tests all the time since some tests have requirements, such as running on Windows. A warning about skipped tests might be useful though.


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

https://reviews.llvm.org/D80377