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

[Andrzej Warzynski via Phabricator via flang-commits]

awarzynski added inline comments.


================
Comment at: flang/CMakeLists.txt:132
          "Generate build targets for the Flang unit tests."
-         ON)
-  add_custom_target(check-all DEPENDS check-flang)
+         ${LLVM_INCLUDE_TESTS})
+
----------------
sameeranjoshi wrote:
> isuruf wrote:
> > sameeranjoshi wrote:
> > > isuruf wrote:
> > > > Is the variable `LLVM_INCLUDE_TESTS` set in a out-of-tree build?
> > > For an out-of-tree build of flang the user is supposed to set `FLANG_INCLUDE_TESTS` if he wants a local toggle for the unit tests of sub project(flang in this case).
> > > 
> > > Now `LLVM_INCLUDE_TESTS` depends on how LLVM was built, but by default `LLVM_INCLUDE_TESTS` is `ON`.
> > > In case, if `LLVM_INCLUDE_TESTS` was `OFF` still the user of out-of-tree build of flang can get unit-tests using the local toggle of `FLANG_INCLUDE_TESTS`. 
> > > 
> > > Can read more on this on my older revision[1]
> > > [1] https://reviews.llvm.org/D80377#2054070
> > > 
> > `LLVM_INCLUDE_TESTS` is not set to any value in an out-of-tree build. It's only set if you are doing an in-tree build.
> so @isuruf what should be the take on this?
> Do you want me to set it to `ON` always for out-of-tree?
> 
This patch seems to be ready for in-tree testing, which is great! We are keen to start adding unit tests - could the discussion about out-of-tree testing be moved to a separate patch? Ta!


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

https://reviews.llvm.org/D80377