[PATCH 19/21] py/tests: test_efi_selftest: Add test for counting EFI netdevices

[Simon Glass]

Hi Tom,

On Thu, 23 Jan 2025 at 07:42, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Jan 23, 2025 at 07:37:26AM -0700, Simon Glass wrote:
> > Hi Adriano,
> >
> > On Wed, 22 Jan 2025 at 10:10, Adriano Cordova <adrianox at gmail.com> wrote:
> > >
> > > check that the number of ethernet udevices matches the number of simple
> > > network protocols
> > >
> > > Signed-off-by: Adriano Cordova <adriano.cordova at canonical.com>
> > > ---
> > >  test/py/tests/test_efi_selftest.py | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >
> > > diff --git a/test/py/tests/test_efi_selftest.py b/test/py/tests/test_efi_selftest.py
> > > index 310d8ed294..af9992a6f7 100644
> > > --- a/test/py/tests/test_efi_selftest.py
> > > +++ b/test/py/tests/test_efi_selftest.py
> > > @@ -195,3 +195,24 @@ def test_efi_selftest_tcg2(u_boot_console):
> > >      if u_boot_console.p.expect(['Summary: 0 failures', 'Press any key']):
> > >          raise Exception('Failures occurred during the EFI selftest')
> > >      u_boot_console.restart_uboot()
> > > +
> > > + at pytest.mark.buildconfigspec('cmd_bootefi_selftest')
> > > + at pytest.mark.buildconfigspec('cmd_dm')
> > > +def test_efi_selftest_count_netdevices(u_boot_console):
> > > +    """Test the EFI net device driver
> > > +
> > > +    u_boot_console -- U-Boot console
> > > +
> > > +    This function counts the number of ETH_UCLASS probed udevices
> > > +    calls the EFI net device selftest to check that the EFI driver
> > > +    sees the same.
> > > +    """
> > > +    u_boot_console.restart_uboot()
> >
> > We should avoid restarting U-Boot in a test unless there is a strong
> > need. It makes things a lot slower!
>
> Yes, if we can not reboot here that would be good.
>
> > > +    response = u_boot_console.run_command('dm tree')
> > > +    lines = response.splitlines()[2:]
> > > +    ethernet_count = sum(1 for line in lines if line.strip().startswith('ethernet') and '[ + ]' in line)
> >
> > This test should be written in C, e.g:
> >
> > struct udevice *dev;
> > int count;
> > uclass_foreach_dev(dev, UCLASS_ETH)
> >     if (device_active(dev))
> >          count++;
> >
> > > +
> > > +    u_boot_console.run_command(cmd='setenv efi_selftest netdevices')
> > > +    u_boot_console.run_command('bootefi selftest', wait_for_prompt=False)
> > > +    if u_boot_console.p.expect([f'Detected {ethernet_count} active EFI net devices']):
> > > +        raise Exception('Failures occurred during the EFI selftest')
> >
> > Again, this should be written in C and use run_command().
>
> No, if these are written in python already, we can leave them in python.

This is a new test. We should not be making things any worse.

> C is bad for strings.

ut_assert_skip_to_line("Detected %d active EFI net devices", count);

For C tests we don't even need 'dm tree'.

This matters. The C test is easier to understand and runs in a tiny
fraction of the time. Our CI tests have been getting slower and slower
and it is due to this kind of test. I went to the effort of writing
this up carefully [1].

So, NAK from me. Please use C tests where possible.

Regards,
Simon

[1] https://docs.u-boot.org/en/latest/develop/tests_writing.html