[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    [PATCH] D56116: [gn build] Make `ninja check-clang` also run Clang's unit tests
From:       Nico Weber via Phabricator via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2018-12-30 23:38:43
Message-ID: 2731b2997f99560c689f98f2757494d1 () localhost ! localdomain
[Download RAW message or body]

thakis added a comment.

Thanks for the fast turnaround!



================
Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:34
+    # out/gn/obj/clang/unittests/Format/FormatTests, which seems fine.
+    output_dir = target_out_dir
+    deps += [
----------------
phosek wrote:
> What if someone explicitly sets `output_dir` in the invoker? We should either \
> preserve that value or assert with an error message that overriding `output_dir` in \
> `unittest`s is unsupported.
Good point, done.


================
Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:35
+    output_dir = target_out_dir
+    deps += [
+      "//llvm/utils/unittest/UnitTestMain",
----------------
phosek wrote:
> You should check if `deps` is defined and set `deps = []` if not, otherwise the \
> attempt to add to non-existent list is going to fail.
I can't imagine that anyone would want a unit test that doesn't at least depend on \
the code under test. I agree in principle, but I think in practice this would be dead \
code 100% of the time. Might as well omit it then :-)


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

https://reviews.llvm.org/D56116



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]