[Piglit] Allow junit to be used for summary generation

[Mark Janes]

Dylan Baker <baker.dylan.c at gmail.com> writes:

> On Thu, Oct 8, 2015 at 6:09 PM, Mark Janes <mark.a.janes at intel.com> wrote:
>
>> baker.dylan.c at gmail.com writes:
>>
>> > This series updates the junit backend to allow it to properly load junit
>> > and convert it back into piglit's internal representation, thus allowing
>> > it to be summarized using the piglit summary tools
>> >
>> > There is still some work that needs to be done beyond this, most of the
>> > platform metadata isn't stored yet and restored, but I have a plan for
>> > that. I have some other refactoring work that I think will make that
>> > easier, and I'd like to get there before landing that.
>> >
>> > This is enough to be able to compare junit and json results using the
>> > console and html summaries.
>>
>> I don't have a use case for comparing junit and json.  If anyone tried
>> to compare json to our junit, they would see lots of differences because
>> json does not respect the "expected-failures" filter.  That filter will
>> also be unusable for json, because of the same test name character
>> conversion.
>>
>
> I think that we should make the filter work for both. I'm not sure yet what
> the right solution is for that, but I think it's worth making happen, I
> think there are a lot of reasons for it, but the main one I see is that it
> would eliminate distinctions between the backends.
>
> I also have a series I'm working on that uses json for the intermediate
> atomic writes, and then having a function that just converts the
> TestrunResult object generated at the end of the run into the output we
> want (like json or junit, or nunit, or whatever else). This reduces code
> duplication a lot and reduces the number of code paths that aren't being
> tested by me a lot. This is advantageous in unify the expected
> failure/crash code to use the native piglit names rather than the junit
> names, since we could apply it before converting the tests and writing them
> out. That feels like the best solution to me, though it may mean some work
> converting our existing crash/fail lists.
>
>
>>
>> > There is a caveat here, and that's patch 3. To compare json and junit we
>> > need to be able to restore the names of the junit tests to *exactly*
>> > what they were before, and currently we don't have a way to reverse the
>> > '.' -> '_' conversion. My proposal is to change '.' into '___', which is
>> > very unlikely in a real test name (though we could change it to almost
>> > anything that would be unique). This may break some existing setup
>> > (Mark, I think this will probably break some of our expected fail/crash
>> > data).
>>
>> It seems to me that it will be simpler for everyone to disallow
>> junit/json comparisons.  I just need a way for users to visualize a
>> dozen junit test files for disparate platforms and test suites.
>>
>
> It might be, but I suspect that someone will try it at some point. I
> imagine a workflow like:
> 1) developer gets a report about breaking some system
> 2) developer makes changes, and runs subset of piglit on their system
> 3) developer wants to compare those changes to the original junit
>

For the example workflow, the developer needs the pass/fail
configuration files.  Without them, the local results will not
correspond to what the CI produces.  We can provide those files, but
will developers really want to use them for their test runs?  The
configurations change several times a day as bugs are fixed or
regressions are written up as bugs.

If you want to solve the general problem, I think the most sensible
thing is to place limits on test names:

 * xml compatible (eg '<\/>&')
 * json compatible (eg '{}:')
 * config file compatible (eg '[]=:;#')
 * jenkins compatible ("api","search")

 and possibly even
 * command-line friendly (eg '()> &*')

Then, alter all existing problematic test names and assert that all
future test names conform.  Munging test names just adds confusion:  to
bisect a test, I'll need to reverse the '.' -> '__' transformation
before using names in --include-tests.

Personally, I find it irritating that so many test names incorporate
exotic characters.  But is it really worth the effort to rename
everything?  Will developers agree to the restrictions?  What about
BuildBot or other CI restrictions?