[libc-commits] [PATCH] D74665: [libc] [UnitTest] Create death tests

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Overall looks good. But, one comment about eliminating the platform specificity from the change as proposed.



================
Comment at: libc/utils/UnitTest/Test.cpp:14
 
+#include <sys/wait.h>
+#include <unistd.h>
----------------
Can we build a generic forking utility and put it in the `testutils` library I have proposed in my other patch? This will help us keep the unittest implementation from pulling in and using platform specific pieces.


================
Comment at: libc/utils/UnitTest/Test.h:41
 
+struct PolymorphicFunction {
+  virtual ~PolymorphicFunction() {}
----------------
Instead of putting them in the `internal` namespace, can we nest them inside of of the `Test` class? It seems a bit odd for an `internal` class to be part of the public API of the `Test` class.


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

https://reviews.llvm.org/D74665