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

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

First off, let me say this is very cool.  I look forward to the day when all commands are builtin, and we can switch every client to the builtin lit shell.

I have two primary thoughts upon reading the patch:

1. I don't see any tests for the case where the commands *succeed*.  Only for the case where they fail.  Shouldn't we have some tests that verify the commands actually work?

2. Now that we have upwards of 6 builtin commands, I'm starting to think they should be separated into their own file.  For example, `lit/test/ShellBuiltins.py`  We could test them via unittests (e.g. see how `BooleanExpression.py` implements its unit tests).

What do you think?


https://reviews.llvm.org/D39567