On Thu, Aug 22, 2024 at 6:28 AM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Sat, 17 Aug 2024 at 11:26, Octavian Purdila <ta...@google.com> wrote:
> >
> > This patch set adds support for NXP's RT500 MCU [1] and the RT595
> > EVK[2]. More RT500 device models will be submitted in future patch sets.
> >
> > The goal of this first patch set is to provide a minimal set that
> > allows running the NXP MCU SDK hello world example[4].
> >
> > The patch set introduces a (python) tool that generates C header files
> > from ARM SVD files[3]. This significantly reduces the effort to write
> > a new device model by automatically generating: register definitions
> > and layout (including bit fields), register names for easier debugging
> > and tracing, reset register values, register write masks, etc.
> >
> > The generated files are commited and not generated at compile
> > time. Build targets are created so that they can be easily regenerated
> > if needed.
> >
> > It also introduces unit tests for device models. To allow accessing
> > registers from unit tests a system bus mock is created.
> >
> > This can potentially introduce maintainance issues, due to mocks or
> > unit tests getting outdated when code is refactored. However, I think
> > this is not an issue in this case because the APIs we mocked (system
> > bus MMIO access) or directly used (irq APIs, chardev APIs, clock tree
> > APIs) to interact with device models are stable at this
> > point. Anecdotally, our experience seems to confirm this: we only run
> > into one (trivially fixed) breaking upstream change (gpio getting
> > removed from hwcore) in the last three years.
>
> My main issue with the mocking is that it introduces a
> completely different way of testing devices that is
> not the same as what we use for any existing device.
> QEMU already has too many places where there are multiple
> different ways or styles of doing something, so adding a
> new one should be a high bar (e.g. "this lets us test XYZ
> that would be impossible in the old way") and preferably
> also have a transition plan for how we would be
> deprecating and dropping the old way of doing things.
>
> So my inclination here is to say "you said that you could
> do the testing of this device with qtest, so use qtest".

Looks like I missed some things when I looked at it, probably minor or
a matter of preference in the great scheme of things. It might still
be worth mentioning them here.

In patch 19 we are testing exposed clocks and AFAICS there are no
qtest APIs for that. We can probably add a qest API for that though.

In patch 9 we are using internal APIs exposed by the generic flexcom
device to check that device selection works as expected. I don't see
how we can reimplement that with qtest, this is a classic example of
what unit tests can enable vs functional testing.

There are also a few things that are a bit cumbersome to do with
qtest. In patch 13 and 16 we introduce i2c and spi echo test devices
to test i2c/spi transactions. I've noticed that device qtests use an
existing i2c peripheral for tests. We could add the i2c and spi test
devices to qemu and have them enabled by default. Or continue using
existing spi/i2c peripherals for testing spi/i2c controllers for
qtests which I personally don't like because it requires knowledge of
specific peripherals.

That being said, I'll go ahead and switch to qtests, if only to better
understand advantages / disadvantages and separate this patch from the
bigger device unit tests discussion.

> If we were designing a "test devices" framework from
> scratch, using mocks would probably be a strong candidate
> for that design, but we aren't starting from scratch.

I still think that writing device tests as unit tests is better:
 - devices can be tested independently
 - we can test more, including internal behaviour and APIs, error paths, etc.
 - enable sanitizers by default for device tests*

I wonder what people think and what would be a path to enable device
unit tests. What would we need to prove, besides converting existing
device qtests to unit tests, which I can certainly help with.

*I was surprised to see that sanitizers are not enabled by default on
unit tests when host dependencies are available, what is preventing
that?

Reply via email to