[lldb-dev] New test summary results formatter

[Pavel Labath via lldb-dev]

There is already code that enforces unique names (see dotest.py:1255).
I added this a while back because we were getting test flakyness
because of that. I don't remember the exact reason but I think it had
something to do with the log file names clashes, as all logs are
placed in the same folder.

I agree that it is possible to have multiple test files with the same
name, and still have the names meaningful, but then we need to make
sure that the it actually works reliably.


As for the results formatter, I like the new output a lot. For me, the
inability to recognize crashes is a blocker for using it, but once
that works, I would definitely be in favor of making the default.


I would welcome shortening of the test names, but that is of secondary
significance for me.

great job,
pl


On 3 December 2015 at 06:20, Todd Fiala via lldb-dev
<lldb-dev at lists.llvm.org> wrote:
>
>
> On Wed, Dec 2, 2015 at 9:48 PM, Zachary Turner <zturner at google.com> wrote:
>>
>>
>>
>> On Wed, Dec 2, 2015 at 9:44 PM Todd Fiala <todd.fiala at gmail.com> wrote:
>>>
>>>
>>>>
>>>> and the classname could be dropped (there's only one class per file
>>>> anyway, so the classname is just wasted space)
>>>
>>>
>>> Part of the reason I included that is I've hit several times where copy
>>> and paste errors lead to the same class name, method name or even file name
>>> being used for a test.  I think, though, that most of those are addressed by
>>> having the path (relative is fine) to the python test file.  I think we can
>>> probably get by with classname.methodname (relative test path).  (From your
>>> other email, I think you nuke the classname and keep the module name, but
>>> I'd probably do the reverse, keeping the class name and getting rid of the
>>> module name since it can be derived from the filename).
>>
>> I don't think the filename can be the same anymore, as things will break
>> if two filenames are the same.
>
>
> Maybe, but that wasn't my experience as of fairly recently.  When tracking
> failures sometime within the last month, I tracked something down in a
> downstream branch with two same-named files that (with the legacy output)
> made it hard to track down what was actually failing given the limited info
> of the legacy test summary output.  Maybe that has changed since then, but
> I'm not aware of anything that would have prohibited that.
>
>>
>>   We could go one step further and enforce this in the part where it scans
>> for all the tests.
>
>
> I think I can come up with a valid counterargument to doing that.  I could
> imagine some python .py files being organized hierarchically, where some of
> the context of what is being tested clearly comes from the directory
> structure.
>
> Something like (I'm making this up):
>
> lang/c/const/TestConst.py
> lang/c++/const/TestConst.py
>
> where it seems totally reasonable to me to have things testing const support
> (in this example) but being very different things for C and C++, being
> totally uniqued by path rather than the .py file.  I'd prefer not to require
> something like this to say:
> lang/c/const/TestConstC.py
> lang/c++/const/TestConstC++.py
>
> as it is redundant (at least via the path hierarchy).
>
> The other reason I could see avoiding that
> unique-test-basenames-across-test-suite restriction is that it can become
> somewhat of an unnecessary burden on downstream branches.  Imagine somebody
> has a branch and has a test that happens to be running fine, then somebody
> in llvm.org lldb adds a test with the same name.  Downstream breaks.  We
> could choose to not care about that, but given that a lot of our tests will
> revolve around language features accessed/provided by the debugger, and a
> number of language features pull out of a limited set of feature names (e.g.
> const above), I could see us sometimes hitting this.
>
> Just one take on it.  I'm not particularly wedded to it (I probably would
> avoid the confusion by doing something exactly like what I said above with
> regards to tacking on the language to the test name), but I have hit this in
> similar form across different language tests.
>
>>
>>   If it finds two test files with the same name we could just generate an
>> error.  I think that's a good idea anyway, because if two test files have
>> the same name, then the tests inside must be similar enough to warrant
>> merging them into the same file.
>
>
> Maybe, but not in the real cases I saw across different languages.  I think
> for other areas of the debugger, this isn't an issue.  So maybe language
> feature tests just have to know to append their language (whether it be C,
> C++, ObjC, etc.)
>
>>
>>
>> If no two filenames are the same, and if there's only 1 class per file,
>> then filename + method name should uniquely identify a single test, and so
>> you could omit the class name and show a relative path to the filename.
>
>
> I think we currently have some tests that have multiple test classes in the
> test file.  We could certainly verify that in TOT, and we could certainly
> undo that which seems reasonable.
>
> I'd be interested in what other people think here on restricting test names
> to be unique across the repo.  I could be convinced either way on allowing
> two tests with the same name, but I'd probably avoid layering on a
> restriction if it is entirely artificial and requires longer test names that
> are otherwise uniqued by path.
>
> Thanks for the feedback!
> --
> -Todd
>
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>