[Development] Solutions for ensuring that changes in upstream modules are tested with downstream modules before merging

[David Skoland]

I would really be interested if there are other leaf modules that have this issue. If it's just qtquickcontrols2 then perhaps merging into qtdeclarative would be the easiest solution after all.

Taking a quick look through the qtquickcontrols2’s changes, it seems a large number(half?) of them are just Qt Submodule Update Bot and Qt Cherry-pick Bot changes. I would think that merging qqc2 into qtdeclarative could make things simpler, but I would like to hear if anyone knows about any potential downsides this may have. Is it a realistic move to make at this point?

On 7 Jun 2021, at 10:22, Mitch Curtis <mitch.curtis at qt.io<mailto:mitch.curtis at qt.io>> wrote:

-----Original Message-----
From: Volker Hilsheimer <volker.hilsheimer at qt.io<mailto:volker.hilsheimer at qt.io>>
Sent: Friday, 4 June 2021 4:51 PM
To: Mitch Curtis <mitch.curtis at qt.io<mailto:mitch.curtis at qt.io>>
Cc: development at qt-project.org<mailto:development at qt-project.org>
Subject: Re: [Development] Solutions for ensuring that changes in upstream
modules are tested with downstream modules before merging

On 4 Jun 2021, at 16:10, Mitch Curtis <mitch.curtis at qt.io<mailto:mitch.curtis at qt.io>> wrote:

-----Original Message-----
From: Volker Hilsheimer <volker.hilsheimer at qt.io<mailto:volker.hilsheimer at qt.io>>
Sent: Friday, 4 June 2021 2:45 PM
To: Mitch Curtis <mitch.curtis at qt.io<mailto:mitch.curtis at qt.io>>
Cc: development at qt-project.org<mailto:development at qt-project.org>
Subject: Re: [Development] Solutions for ensuring that changes in
upstream modules are tested with downstream modules before merging

On 4 Jun 2021, at 13:56, Mitch Curtis <mitch.curtis at qt.io<mailto:mitch.curtis at qt.io>> wrote:

Hi.

A common problem I see is that a change in say qtbase or
qtdeclarative can
cause test failures in modules that depend on them after that change
is merged. As a result, dependency updates for the downstream modules
can be blocked, requiring the module maintainer to look into the
failure, only to discover that it is caused by an upstream module.
This causes a lot of time and effort to be diverted away from regular
work. Both upstream and downstream developers would benefit from
having more immediate CI feedback on these kinds of changes.

https://bugreports.qt.io/browse/QTBUG-79454 was created to track
this
problem. So far the ideas suggested have been:

- Run tests of dependent modules when testing a change, in addition
to
the regular tests for that module
- Merge repositories of more tightly coupled modules (e.g. move
qtquickcontrols2 into qtdeclarative)

It would be beneficial to discuss the advantages and disadvantages
of
these and other solutions so that we can address this problem.

Cheers.


Excellent topic for the upcoming Qt Contributors Summit, Mitch, great
if you can add it to the list of topics:

https://wiki.qt.io/Qt_Contributors_Summit_2021_-_Program

Which doesn’t mean that we shouldn't start the discussion on the
list, and maybe we can then conclude it at the summit.


Cheers,
Volker

OK, added it to the list.


Thanks.


As for the topic:

I do think that we have cases where too many modules are lumped into a
single repo, and cases where we have too much granularity.

For the former: I see no immediate reason why QtNetwork needs to be in
qtbase (network tests failing is a major cause of unrelated integrations
failing), or why the Qt Positioning module needs to be in qtlocation.

For the latter: Qt Quick Controls 2 might be better off sharing a commit
history with QtQuick

However, while we can make adjustments, I also think that there’s no
perfect solution. It’s always a tradeoff between speed and autonomy on the
one, and integrity across dependencies on the other hand.

I agree.

Running more tests is a similar tradeoff, and I don’t think blocking an
integration into e.g. qtbase because it breaks a leaf module is a good solution
to the problem. We should aim for fast feedback, and short meantime to
recover such breakages, and making CI runs take even longer helps with
neither. In particular if we then can’t make any such changes even though a
follow up change in a leaf module is already available and just needs to wait
for the dependency update. Then we are in a deadlock, unless we keep the
code in qtbase working for both versions of e.g. qtdeclarative, which is not
always practical.

I could be wrong, but perhaps this is what Ossi's https://bugreports.qt.io/browse/COIN-133 was targeting. In any case, I can't really speak for qtbase as I rarely work on it.

The problem is perhaps not that we break things, but that we don’t notice
for too long that we broke things? And once we do notice, we often seem to
be unwilling to revert the change that did cause the breakage, if things don’t
get fixed quickly.

This might be the case for qtbase and its leaf modules, but not for qtdeclarative and qtquickcontrols2. The typical situation there is: qtquickcontrols2 dependency update fails. I immediately cannot think of any relevant changes that would cause such a failure, so I quickly look through git log. If I can't find anything there, I need to start bisecting with qtdeclarative (and this assumes that the bisect doesn't result in build errors). It's not a productive use of time.

On the other hand, a CI run, while taking extra time if having to run the tests of two or more modules, shouldn't block someone from doing work. There is always another task to work on while waiting for CI. Flakiness will of course make the feedback loop take way longer, but that has always been the case.

To know about breakage quicker, a nighly run of all tests against “HEAD of
everything” could help. Our dependency updates give us some of that, and
we usually respond swiftly when an update fails. It shouldn’t be on the leaf
module maintainer alone to follow up on things, and I do have the
impression that people do take ownership, at least once they see that their
base module change broke stuff. I’m sure we can improve there as well.
But now that Qt 6 is out, I’d expect that a “everything at HEAD” toplevel build
should work most of the time. My local worktrees are usually at HEAD of
everything, and it’s rare that there are build problems at least.

For qtdeclarative and qtquickcontrols2 I'm not sure how this would improve anything. It would say that a test failed, but as you said, that's what the dependency update already tells us. It does sound like a nice tradeoff (of running tests for all modules on every change and not running them at all) for modules where the dependency updates don't give any signs of a breakage, though.

Lastly, I do think that we could do more in encoding assumptions made
across modules in unit tests. If a change in qtdeclarative breaks an
assumption made in qqc2, then ideally there’s a unit test in qtdeclarative that
would fail. We use private APIs a lot across some modules, but we don’t have
unit tests for them in the base module.

Agreed, and the qtdeclarative team are great in doing this once they're aware of it, but there are too many use cases for them to have to be aware of in order to fully solve the problem. It's also a slow process (in terms of the rate of breakages across modules) in adding tests upstream to catch issues as they occur downstream.

I would really be interested if there are other leaf modules that have this issue. If it's just qtquickcontrols2 then perhaps merging into qtdeclarative would be the easiest solution after all.
_______________________________________________
Development mailing list
Development at qt-project.org<mailto:Development at qt-project.org>
https://lists.qt-project.org/listinfo/development

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20210607/922389df/attachment-0001.html>