<div>OK changes made and new patch attached.</div><div><br></div>Comments below:<br><br><div class="gmail_quote">On Tue, Dec 30, 2008 at 2:40 PM, Misha Brukman <span dir="ltr"><<a href="mailto:brukman@gmail.com">brukman@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">+++ unittests/TestMain.cpp (revision 0)<br>+//===--- unittest.cpp - unittest driver -----------------------------------===//<br>
<br>+++ unittests/ADT/DenseMapTest.cpp (revision 0)<br>+//===- llvm/unittest/DenseMapMap.h - DenseMap unit tests --------*- C++ -*-===//<br>
<br>You probably want to update these file headers to match their real names.<br><br><div class="gmail_quote">2008/12/30 Misha Brukman <span dir="ltr"><<a href="mailto:brukman@gmail.com" target="_blank">brukman@gmail.com</a>></span><br>
<blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">2008/12/30 Talin <span dir="ltr"><<a href="mailto:viridia@gmail.com" target="_blank">viridia@gmail.com</a>></span><br>
<div class="gmail_quote"><div><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
Here's the version of the unit test patch, incorporating the feedback I have received so far.<div></div></blockquote></div><div><br>Looks good, a few comments below.<br> </div><div><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
<div>Some notes on the patch:</div><div><ul><li>This patch doesn't include googletest itself, that will need to be checked in separately. The source distribution will live in llvm/utils/unittest/googletest. (The reason for the extra directory level is so that the LLVM-specific makefiles can live in llvm/utils/unittest, while the googletest directory itself can be the pristine, unmodified distribution.)</li>
</ul></div></blockquote></div><div>Are you going to send a separate patch for this, or should I do this?</div></div></blockquote></div></blockquote><div><br></div><div>Please go ahead and do this. It seems silly (to me anyway) to convert the source tarball into a patch file.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
<div class="gmail_quote"><div><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
<div><ul><li></li>
<li>The individual tests are in llvm/unittests (note plural).</li><li>The makefile target to build and run the tests is "make unittest" (I wasn't sure if you could have a synthetic target that was the same as the name of a directory, so I made it "unittest" instead of "unittests")</li>
</ul></div></blockquote></div><div>I don't think that's an issue -- to recurse into a directory, you have to say "make -C dir [target]". <br></div><div><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
<div><ul><li></li>
<li>There's a common Makefile and a common TestMain.cpp in llvm/unittests. The individual tests are in llvm/unittests/<dir> where <dir> is the name of an LLVM library such as "ADT". Each of these subdirs has a tiny makefile which sets TESTNAME=<name> and then includes the common Makefile.unittest. Each subdir under unittests creates a separate executable. (I didn't use the default googletest main because I figured at some point we might want to customize main().)</li>
<li>I updated the LICENSE.txt and mkpatch, but I haven't done the HTML docs yet because I am still thinking about what to write.</li><li>I probably made some mistakes in setting up the makefile rules - that is what took the most time - so it will merit heightened scrutiny.</li>
</ul></div></blockquote></div><div><br>+++ unittests/Makefile.unittest (revision 0)<br>[...]<br>+VPATH=$(PROJ_SRC_DIR) $(PROJ_SRC_ROOT)/unittests</div></div></blockquote></div></blockquote><div><br></div><div>That's two dirs: One for the current directory, and one for the directory one level above. ".." won't work here, so I have to create an absolute path so that it can find TestMain.cpp.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
<div class="gmail_quote"><div><br> <br>That doesn't look like a valid path to me.<br></div></div><br>
</blockquote></div><br>
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br></blockquote></div><br><br clear="all"><br>-- <br>-- Talin<br>