Hi;

On 11/15/19 8:41 PM, Mark Janes wrote:
Michel Dänzer <mic...@daenzer.net> writes:

On 2019-11-15 4:02 p.m., Mark Janes wrote:
Michel Dänzer <mic...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to