[Lldb-commits] [PATCH] D12587: Roll dosep.py parallel test runner into dotest.py command line

[Todd Fiala via lldb-commits]

tfiala added inline comments.

================
Comment at: test/CMakeLists.txt:65
@@ -64,3 +64,3 @@
   ${LLDB_SOURCE_DIR}/test/dotest.py
-  "${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS}"
+  "--no-multiprocess;${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS}"
   "Testing LLDB with args: ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS}"
----------------
zturner wrote:
> Is this necessary?  dosep already takes a -t argument which specifies the number of threads, and if the number of threads is 1 it disables the thread pool.  Is it possible to delete this option and just pass -t=1 instead?
Right now there's an implementation difference between the two:

--no-multiprocess literally switches from using the dosep parallel test runner logic to using the old dotest.py test runner logic.  Specifying -t/--threads  THREADCOUNT where THREADCOUNT is 1 will use the multiprocess test runner with a single process queue.  So, if somebody wanted to have the extra test subdir filtering of the dosep logic (the test-subdir arg) but for some reason wanted to debug or otherwise run it serially, they'd be out of luck if we adopted a -t=1 == --no-multiprocess stance.

What I'd recommend is we keep the two options for now since there is behavior we would otherwise lose at the moment.  But, when we do the refactor to test runners, we get rid of all the bits that are special to the multi test runner (e.g. the test-subdir and the --output-on-success flags), at which point your idea that "--no-multiprocess is unnecessary" should be true, and we can remove it then.

Sound reasonable?


http://reviews.llvm.org/D12587