[Mesa-dev] How to merge Mesa changes which require corresponding piglit changes

[Tapani Pälli]

Hi;

On 11/15/19 8:41 PM, Mark Janes wrote:
> Michel Dänzer <michel at daenzer.net> writes:
> 
>> On 2019-11-15 4:02 p.m., Mark Janes wrote:
>>> Michel Dänzer <michel at daenzer.net> writes:
>>>
>>>> Now that the GitLab CI pipeline tests a snapshot of piglit with llvmpipe
>>>> (https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2468), the
>>>> question has come up how to deal with inter-dependent Mesa/piglit
>>>> changes (where merging only one or the other causes some piglit
>>>> regressions).
>>>>
>>>>
>>>> First of all, let it be clear that just merging the Mesa changes as-is
>>>> and breaking the GitLab CI pipeline is not acceptable.
>>>>
>>>>
>>>>  From the Mesa POV, the easiest solution is:
>>>>
>>>> 1. Merge the piglit changes
>>>> 2. In the Mesa MR (merge request), add a commit which updates piglit[0]
>>>> 3. If the CI pipeline is green, the MR can be merged
>>>>
>>>>
>>>> In case one wants to avoid alarms from external CI systems, another
>>>> possibility is:
>>>
>>> For the Intel CI, no alarm is generated if the piglit test is pushed
>>> first.  Normal development process includes writing a piglit test to
>>> illustrate the bug that is being fixed.
>>
>> Cool, but what if the piglit changes affect the results of existing
>> tests? That was the situation yesterday which prompted this thread.
> 
> We attribute the status change to piglit in the CI config, within a few
> hours.  The test shows up as a failure in CI until it is triaged.

I think we have a problem with current gitlab CI process.

Right now if someone needs to update piglit commit used by CI, he also 
ends up fixing and editing the .gitlab-ci/piglit/quick_gl.txt (and 
glslparser+quick_shader.txt) as CI reports numerous failures because of 
completely unrelated stuff as meanwhile people added other tests, 
removed tests and modified them. I think we should turn such warnings on 
only when we have more sophisticated algorithm to detect actual 
regression (not just 'state change', like additional test or removed test).

>>>> 1. In the Mesa MR, add a commit which disables the piglit tests broken
>>>> by the Mesa changes.
>>>> 2. If the CI pipeline is green, the MR can be merged
>>>> 3. Merge the piglit changes
>>>> 4. Create another Mesa MR which updates piglit[0] and re-enables the
>>>> tests disabled in step 1
>>>>
>>>> I hope that covers it, don't hesitate to ask questions if something's
>>>> still unclear.
>>>
>>> It might help developers if CI generated the patch to make their pipeline
>>> pass.
>>
>> It does for the test result list, if that's what you mean.
>>
>> However, that patch shouldn't be applied mechanically, but only after
>> confirming that all changes in test results are expected. Ideally,
>> whenever there are any new tests, the corresponding CI jobs should be
>> run several times to make sure the new results are stable, otherwise any
>> flaky tests should be excluded.
>>
>>
>> -- 
>> Earthling Michel Dänzer               |               https://redhat.com
>> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 


// Tapani