[PATCH] D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally

[Ying Yi via Phabricator via llvm-commits]

MaggieYi added a comment.



> Update: I actually do see where you're testing that they work, but I'm not convinced that the tests are sufficient.  For several reasons.
> 
> 1. it doesn't test that the builtin command is actually run versus the OS command.   It only tests that whatever command is run doesn't fail.
> 2. It doesn't test that the result of the command is as expected, it only tests that it doesn't return an error.  For a somewhat unrealistic illustration of this, imagine that all the commands just returned success without actually doing anything.  The test as it is written now would pass.

Good points. I am working on this issue now.


https://reviews.llvm.org/D39567