[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