Re: [OE-Core][PATCH v3 1/2] testimage: move TESTIMAGE_FAILED_QA_ARTIFACTS default to core-image

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2024-02-05 at 15:37 +0100, Alexis Lothoré via
lists.openembedded.org wrote:
> From: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> TESTIMAGE_FAILED_QA_ARTIFACTS currently sets a default list of files to be
> saved whenever some tests fail. Unfortunately, this default value is very
> easily discarded, because TESTIMAGE_FAILED_QA_ARTIFACTS is expected to be
> enriched by some core recipes (example: ptest images) which are often
> parsed before testimage.bbclass. Those core recipes could still manage to
> get the default value AND add some files by using override syntax, but then
> it makes it difficult for downstream recipes or bbappend files to tune this
> variable.
> 
> Allow to set this default value and make sure it is not overwritten by
> recipes wanting to tune it, by setting the default value in core-image,
> which is parsed early enough. While doing so, set it as a default value
> instead of a weak default value.
> 
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> ---
> Changes in v3:
> - fix commit subject length and content
> 
> Changes in v2:
> - set the default value in core-image instead of core-image-minimal
> ---
>  meta/classes-recipe/core-image.bbclass | 8 ++++++++
>  meta/classes-recipe/testimage.bbclass  | 9 ---------
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/meta/classes-recipe/core-image.bbclass b/meta/classes-recipe/core-image.bbclass
> index 40fc15cb04f2..8ca64a95a318 100644
> --- a/meta/classes-recipe/core-image.bbclass
> +++ b/meta/classes-recipe/core-image.bbclass
> @@ -83,4 +83,12 @@ CORE_IMAGE_EXTRA_INSTALL ?= ""
>  
>  IMAGE_INSTALL ?= "${CORE_IMAGE_BASE_INSTALL}"
>  
> +# When any test fails, TESTIMAGE_FAILED_QA_ARTIFACTS will be parsed and for
> +# each entry in it, if artifact pointed by path description exists on target,
> +# it will be retrieved onto host
> +TESTIMAGE_FAILED_QA_ARTIFACTS ?= "\
> +    ${localstatedir}/log \
> +    ${sysconfdir}/version \
> +    ${sysconfdir}/os-release"
> +
>  inherit image
> diff --git a/meta/classes-recipe/testimage.bbclass b/meta/classes-recipe/testimage.bbclass
> index f36d9418914f..cfda5b631ba8 100644
> --- a/meta/classes-recipe/testimage.bbclass
> +++ b/meta/classes-recipe/testimage.bbclass
> @@ -18,15 +18,6 @@ inherit image-artifact-names
>  
>  TESTIMAGE_AUTO ??= "0"
>  
> -# When any test fails, TESTIMAGE_FAILED_QA ARTIFACTS will be parsed and for
> -# each entry in it, if artifact pointed by path description exists on target,
> -# it will be retrieved onto host
> -
> -TESTIMAGE_FAILED_QA_ARTIFACTS ??= "\
> -    ${localstatedir}/log \
> -    ${sysconfdir}/version \
> -    ${sysconfdir}/os-release"
> -
>  # You can set (or append to) TEST_SUITES in local.conf to select the tests
>  # which you want to run for your target.
>  # The test names are the module names in meta/lib/oeqa/runtime/cases.

I can imagine why ??= breaks if you use +=.

Does this work if you change it from ??= to just = ?

I'd really prefer to keep this in testimage.bbclass. Recipes should be
able to use += if it is set with = and the addition is after the class
inherit.

Cheers,

Richard