[PATCH] D56219: [gn build] Add build files for LLVM unittests with a custom main() function

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:53
+# //llvm/utils/unittest/UnitTestMain.
+template("unittest_with_custom_main") {
+  executable(target_name) {
----------------
thakis wrote:
> phosek wrote:
> > I'm not a big fan of the name, have you considered naming it e.g. `gtest`?
> `unittest_with_custom_gtest`? Or just `gtest`?
> 
> The "normal" unittest target is called `unittest`, and this one is a variant on that, so `unittest_foo` with some value for `foo` seems like a good name to me. The defining characteristic of these tests is that they contain a custom `main()` function, hence the current name. I'm very open for different naming suggestions, but imho this template should be named similarly to the existing `unittest` template. (Also, `unittest` is used like 50 times, this one only 3 times, so making it look familiar and self-consistent seems more important to me than making it short.)
> 
> Also, D56324 is another, similar patch that adds yet another similar template.
How about just adding an argument to `unittest` template, e.g. `has_main`? It seems like that's the only difference between the two templates so that also avoids the duplication.


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

https://reviews.llvm.org/D56219