[PATCH v5 4/4] test/py: efi_capsule: test for FMP versioning

[Simon Glass]

Hi Masahisa,

On Mon, 10 Apr 2023 at 03:07, Masahisa Kojima
<masahisa.kojima at linaro.org> wrote:
>
> This test covers FMP versioning for both raw and FIT image,
> and both signed and non-signed capsule update.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> ---
> Changes in v5:
> - get aligned to the device tree based versioning
>
> Newly created in v4
>
>  test/py/tests/test_efi_capsule/conftest.py    |  73 +++++++
>  .../test_capsule_firmware_fit.py              | 187 ++++++++++++++++
>  .../test_capsule_firmware_raw.py              | 201 ++++++++++++++++++
>  .../test_capsule_firmware_signed_fit.py       | 165 ++++++++++++++
>  .../test_capsule_firmware_signed_raw.py       | 169 +++++++++++++++
>  test/py/tests/test_efi_capsule/version.dts    |  27 +++
>  6 files changed, 822 insertions(+)
>  create mode 100644 test/py/tests/test_efi_capsule/version.dts

I hate to say this, but we must fix the reboot stuff here before
adding more code.

The test needs to tell U-Boot to do the update (e.g. via a command),
then continue. It should not reboot sandbox.I have said this before
but the feedback was not understood or taken. If you need help
designing this, please let me know. In fact I am happy to create a
patch for one of the tests if that is what it takes to convey this.

Also, while it is normal to have a fair bit of duplication in tests,
just to make them easier to read, it does make them hard to maintain,
and the duplication here seems very large.

We have the same code repeated but with different code style or
indentation. Sorry, but this is not setting us up for the future, in
terms of maintaining this code. Just search for 'env set -e -nv -bs
-rt OsIndications =0x0000000000000004' for example.

The common code needs to go in functions in a separate Python file, as
we have done for other tests. The goal should be to use the same code
for the same functions. Sure there will be some duplication, but it is
too much.

These fixes should happen before we accept any more non=trivial
patches to this code.

BTW I do wonder whether the test would be better written mostly in C,
since from what I can see, it mostly just runs commands. You can still
have a Python wrapper - see for example how test_vbe works. It uses
'ut xxx -f' to execute the C part of a Python test. You can do setup
in Python, then run the C test. What do you think?

Regards,
Simon