[lldb-dev] New test summary results formatter

Thu Dec 3 08:39:16 PST 2015

On Thu, Dec 3, 2015 at 1:41 AM, Pavel Labath <labath at google.com> wrote:

> There is already code that enforces unique names (see dotest.py:1255).
>

How long ago was that added?  I literally hit this within the last 6-8
weeks on something internal.

> 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.
>
>
I really have no problem with this being forced.  With regards to other
repos, this is no different than using a shared library where a base class
may (in the future) add a method name that perturbs a downstream user
(subclass).  Removal of confusion trumps inconvenience in this case.  I'm
sold.

>
> As for the results formatter, I like the new output a lot.

Glad to hear that.  I'm looking forward to making it better.

> 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.
>
>
Yep, that and timeouts.  For us, we catch both on Jenkins and the Xunit
formatter only because the Xunit results gatherer also checks the output,
where the legacy summary support displays this information.  (I have the
new summary output wiring in the timeout data, but not the exceptional exit
(i.e. signal-based exit)) bits.

I'll be addressing both of those in the extremely near future.

I'll be depending on the test event infrastructure's ability to handle
re-runs to do the low-load, single-worker test pass, so I'm working on this
test summary bit first.  (And it's nice to have!).

>
> I would welcome shortening of the test names, but that is of secondary
> significance for me.
>
>
Ultimately I think it's fair to say "hey just put one test case class in a
file", and with a "test file base names must be unique" policy, I think we
can even get away with "test_method_name (relative filename)" to identify
the test method.  That should be even shorter than what Zachary and I were
discussing last night.

> great job,
> pl
>
>
Thanks for the feedback!

Hopefully this serves you as well, Dawn?

>
> 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
> >
>

-- 
-Todd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151203/bc1bee16/attachment-0001.html>