openbmc/telemetry: First complaint of unresponsiveness

[Ambrozewicz, Adrian]

On 20.12.2023 17:56, Patrick Williams wrote:
> On Wed, Dec 20, 2023 at 05:03:21PM +0100, Ambrozewicz, Adrian wrote:
>> What I mean are functional tests on target, with real BMC, board, and
>> sensors.
> 
> openbmc/telemetry doesn't interact with any of these things directly.
> sensors are covered by dbus-sensors, etc.
> 
> What you are describing is an integration test that belongs in
> openbmc-test-automation with all the other integration / functional
> tests.
> 
> If you want to make sure that openbmc/telemetry reacts to the dbus model
> correctly (which is the only thing that belongs in a repository-level
> test) you should mock that out as a unit test.

Agreed and this is done in unit tests.

>> Could you point me to examples where it's documented or done in
>> automated CI for other subprojects?
> 
> See openbmc-test-automation test cases that currently run on QEMU as part of
> Romulus and you can talk to Andrew about the hardware tests that run on
> Witherspoon (which are non-blocking but he keeps on top of the
> failure reports).
> 

Thanks.

>> Or perhaps do you meen that maintainer stalling the change until he
>> makes sure it doesn't break various configurations is unacceptable? Then
>> IMHO I won't aggree, as code compiled within unit tests is just one
>> arch, yet alone separated from rest of the system.
> 
> There are many problems with your position from an open source
> perspective.  These are the ones that are top of mind to me:
> 
> 1. You have no idea if the breakage is due to the submitters proposed
>     change or because of a change in another repository.

Nobody said that tests results are going to be a blind go/no go. That's 
not the case.

> 2. You've made up your own test framework that nobody else has
>     visibility to.  That means you're effectively treating
>     openbmc/telemetry as your own sandbox and are not collaborating with
>     the rest of the project.

That's a bit harsh statement, considering years of development put into 
the code itself and contributions made by team in other related areas 
(sensors, entity-manager, sdbusplus etc.).

> 3. Your delay in integrating changes can have velocity impacts to the
>     rest of the project when we _require_ repository-level changes in
>     order to integrate a Yocto update, etc.

Like Andrew stated - this is an really nasty edge case, a total f-up on 
our side. I believe delay caused by our plain oversight is than how we 
assess changes before merging. I could be running my own instance of 
static analysis tool paid from my pocket for all I care. Does it mean 
it's wrong? We also strive to shorten the timelines but quality comes first.

> 4. Your position is that you will reject someone's commit because it
>     breaks your internal test.  They have no visibility to this test,
>     they have no way to know that they are "breaking you", and they have
>     no possible course of action to recover from any issue you report.

This is similar to #1 and I don't agree - I've never stated that. You're 
talking to a guy who often debugs _and_ fixes external bug himself while 
root-causing. We're on the same side!

> If you want to have your own tests to test integration of
> openbmc/telemetry (along with other open source components) with your own
> project you are more than welcome to do that (even though I think it would be
> far more beneficial to enhance openbmc-test-automation).  That should
> probably be happening when you import openbmc/openbmc into your own
> internal tree, but if you want to give yourself early signal to these
> failures, that is okay.
> 
> My gripe is that you should not be holding up the open-source project for
> your own unpublished, undocumented, non-aligned tests.

I see your point and understand your concerns. Thanks for pointing out 
clearly your arguments.

I would prefer to not dwell into further discussion here as certain 
topics are far beyond my control. I'll just assure you we're trying to 
do the best we can with resources available. Whenever similar related 
issues or decisions to make arise in the future I will be surely better 
equipped to make informed decisions aligned with general open source 
consensus.

Regards,
Adrian

>> W dniu 20.12.2023 o 16:22, Patrick Williams pisze:
>>> On Wed, Dec 20, 2023 at 03:18:38PM +0100, Ambrozewicz, Adrian wrote:
>>>>
>>>> Currently we've integrated proposed changes and wait for feedback from
>>>> automated regression test suite.
>>>
>>> Hi Adrian,
>>>
>>> What did you mean by this?  Commits go through CI when they are
>>> submitted.  I don't see any change in Gerrit for any of the 3 commits.
>>>
>>> If you have an internal test suite you haven't contributed upstream and you
>>> are holding off merging commits until you run them through a private test suite,
>>> this is unacceptable for the open source project.
>>>
>