[llvm] r210597 - lit: warn when passed invalid pathname
Hans Wennborg
hans at chromium.org
Tue Jun 17 10:48:30 PDT 2014
On Mon, Jun 16, 2014 at 1:27 PM, Hans Wennborg <hans at chromium.org> wrote:
> On Mon, Jun 16, 2014 at 11:20 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>> Sorry, I should have caught that in review.
>>
>> Adam is right, the reason why this code didn't do this previously is because
>> of the "virtual test discovery" mechanism that lets you name tests inside
>> the "test exec root".
>>
>> Hans, I think it is probably necessary to put this patch later, after the
>> actual discovery is done.
I took a second look at this. It would be easy to move the check until
after the discovery is done, but then my new error message is
misleading:
If we say "'foo/bar/' doesn't exist", that might not be the actual
problem, because maybe we were also searching in ../../test, where
foo/bar does exist - it just didn't contain any tests. I don't
understand all the magic around test discovery, so I'm not going to
pursue this further.
I would like to simplify this code while I'm here though:
--- a/utils/lit/lit/discovery.py
+++ b/utils/lit/lit/discovery.py
@@ -200,9 +200,7 @@ def find_tests_for_inputs(lit_config, inputs):
# Expand '@...' form in inputs.
actual_inputs = []
for input in inputs:
- if os.path.exists(input) or not input.startswith('@'):
- actual_inputs.append(input)
- else:
+ if input.startswith('@'):
f = open(input[1:])
try:
for ln in f:
@@ -211,6 +209,8 @@ def find_tests_for_inputs(lit_config, inputs):
actual_inputs.append(ln)
finally:
f.close()
+ else:
+ actual_inputs.append(input)
I think this reflects the intent of the code better - add all files to
actual_inputs, except files starting with @ which are treated as
response files. (Supporting the case of test inputs start with @
doesn't seem important.) What do you think?
Thanks,
Hans
More information about the llvm-commits
mailing list