[Piglit] [PATCH 00/35] Serialize profiles into XML at build time

[Matt Turner]

On Tue, May 8, 2018 at 7:07 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 2018-05-07 06:49 PM, Michel Dänzer wrote:
>> On 2018-05-07 06:44 PM, Dylan Baker wrote:
>>> Quoting Tomi Sarvela (2018-05-07 01:20:46)
>>>>
>>>> piglit/framework$ diff -c profile.py.orig profile.py
>>>> *** profile.py.orig     2018-05-07 19:11:37.649994643 +0300
>>>> --- profile.py  2018-05-07 19:11:46.880994608 +0300
>>>> ***************
>>>> *** 584,591 ****
>>>>                # more code, and adding side-effects
>>>>                test_list = (x for x in test_list if filterby(x))
>>>>
>>>> !         pool.imap(lambda pair: test(pair[0], pair[1], profile, pool),
>>>> !                   test_list, chunksize)
>>>>
>>>>        def run_profile(profile, test_list):
>>>>            """Run an individual profile."""
>>>> --- 584,591 ----
>>>>                # more code, and adding side-effects
>>>>                test_list = (x for x in test_list if filterby(x))
>>>>
>>>> !         pool.map(lambda pair: test(pair[0], pair[1], profile, pool),
>>>> !                  test_list, chunksize)
>>>>
>>>>        def run_profile(profile, test_list):
>>>>            """Run an individual profile."""
>>>>
>>>>
>>>> Tomi
>>>
>>> Juan, can you test this patch and see if it resolves your issue as well? I'm not
>>> sure why this is fixing things, but if it does I'm happy to merge it and deal
>>> with any performance problems it introduces later.
>>
>> FWIW, this patch doesn't fix the gpu profile running a lot fewer tests
>> now than it did before 9461d92301e72807eba4776a16a05207e3a16477. I'm
>> also using -x.
>
> I just bisected another problem to
> 9461d92301e72807eba4776a16a05207e3a16477: The xts-render profile doesn't
> work anymore. Most of the time, it doesn't even start:
>
> [000/480]
> Traceback (most recent call last):
>   File "./piglit", line 178, in <module>
>     main()
>   File "./piglit", line 174, in main
>     sys.exit(runner(args))
>   File "/home/daenzer/src/piglit-git/piglit/framework/exceptions.py", line 51, in _inner
>     func(*args, **kwargs)
>   File "/home/daenzer/src/piglit-git/piglit/framework/programs/run.py", line 370, in run
>     backend.finalize({'time_elapsed': time_elapsed.to_json()})
>   File "/home/daenzer/src/piglit-git/piglit/framework/backends/json.py", line 163, in finalize
>     assert data['tests']
> AssertionError
>
> Sometimes, it doesn't fail like this, but only runs between 0 and another
> number < 480 of tests. Very rarely, it manages to run all tests.
>
> (I'm using python 3.6 now)
>
>
> Dylan, since a number of issues have been reported to have started with
> this commit, and you don't seem to have an idea what's wrong with it,
> can you revert it and anything depending on it for the time being? I'll
> be happy to test against the issues I've run into when you're ready to
> try again.

Do you think that is a good workflow? (Serious question)

Dylan's patches were on the list for three weeks and I think only one
person (Rafael) tested them. It doesn't make sense to me to
significantly increase the burden on the person writing the code (by
reverting all the patches when a problem is found) in exchange for a
promise to test the patches... which you or anyone else could have
done during the three weeks Dylan was practically begging for testers.

It's frustrating for me, just as an observer, to see that not even the
people who have so loudly complained about the lack of this very
feature could be bothered to try it out.