[OE-Core][PATCH 0/2] testimage: enable artifacts retrieval for failed tests

[OE-Core][PATCH 0/2] testimage: enable artifacts retrieval for failed tests

[Alexis Lothoré]

Hello,
this small series is a very late follow-up to my initial artifacts
retrieval series (sorry for the delay).
The initial series can be found in [1]. The goal is to retrieve some
specific files from target whenever some tests fail, to ease debugging. The
feature relies on a new TESTIMAGE_FAILED_QA_ARTIFACTS variable listing some
files, and the testimage class using this variable after failed tests to
save corresponding files in tmp/log/oeqa/artifacts.

Most of the initial series has been merged, except the part enriching
TESTIMAGE_FAILED_QA_ARTIFACTS with ptest files path from target to save
files specific to ptests ([2]), so the feature is currently not usable to
debug ptests. There was some discussions around how to properly set default
value and then enrich this variable. After wrapping my head around all the
ways to affect this variable (and mostly, struggling with recipes parsing
order), I came up with this new version, which:
- sets default TESTIMAGE_FAILED_QA_ARTIFACTS in core-image-minimal (because
  basically, testimage.bbclass is parsed too late, while we need to tune
  this variable in some core layers)
- uses += instead of append syntax in core layers (e.g: in ptest images),
  so the variable can still be tuned with append/prepend/remove syntax in downstream layers

[1] https://lore.kernel.org/openembedded-core/20230609064802.11777-1-alexis.lothore@bootlin.com/
[2] https://lore.kernel.org/openembedded-core/20230609064802.11777-5-alexis.lothore@bootlin.com/

Alexis Lothoré (2):
  testimage: move TESTIMAGE_FAILED_QA_ARTIFACTS default value to
    core-image-minimal
  core-image-ptest: retrieve ptests directory when ptests fail

 meta/classes-recipe/testimage.bbclass          | 9 ---------
 meta/recipes-core/images/core-image-minimal.bb | 8 ++++++++
 meta/recipes-core/images/core-image-ptest.bb   | 2 ++
 3 files changed, 10 insertions(+), 9 deletions(-)

-- 
2.42.1

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

[Alexis Lothoré]

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-minimal.
While doing so, set it as a default value instead of a weak default value.

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 meta/classes-recipe/testimage.bbclass          | 9 ---------
 meta/recipes-core/images/core-image-minimal.bb | 8 ++++++++
 2 files changed, 8 insertions(+), 9 deletions(-)

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.
diff --git a/meta/recipes-core/images/core-image-minimal.bb b/meta/recipes-core/images/core-image-minimal.bb
index 84343adcd8e2..8f5fb0d2ae51 100644
--- a/meta/recipes-core/images/core-image-minimal.bb
+++ b/meta/recipes-core/images/core-image-minimal.bb
@@ -10,3 +10,11 @@ inherit core-image
 
 IMAGE_ROOTFS_SIZE ?= "8192"
 IMAGE_ROOTFS_EXTRA_SPACE:append = "${@bb.utils.contains("DISTRO_FEATURES", "systemd", " + 4096", "", d)}"
+
+# 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"
\ No newline at end of file
-- 
2.42.1

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

[Richard Purdie]

On Wed, 2024-01-24 at 15:29 +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-minimal.
> While doing so, set it as a default value instead of a weak default value.
> 
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> ---
>  meta/classes-recipe/testimage.bbclass          | 9 ---------
>  meta/recipes-core/images/core-image-minimal.bb | 8 ++++++++
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> 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.
> diff --git a/meta/recipes-core/images/core-image-minimal.bb b/meta/recipes-core/images/core-image-minimal.bb
> index 84343adcd8e2..8f5fb0d2ae51 100644
> --- a/meta/recipes-core/images/core-image-minimal.bb
> +++ b/meta/recipes-core/images/core-image-minimal.bb
> @@ -10,3 +10,11 @@ inherit core-image
>  
>  IMAGE_ROOTFS_SIZE ?= "8192"
>  IMAGE_ROOTFS_EXTRA_SPACE:append = "${@bb.utils.contains("DISTRO_FEATURES", "systemd", " + 4096", "", d)}"
> +
> +# 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"
> \ No newline at end of file

I'm a little puzzled by this. Doesn't this mean it won't work for any
image that isn't based upon minimal (which is most of them)?

Cheers,

Richard

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

[Alexis Lothoré]

On 1/24/24 17:20, Richard Purdie wrote:
> On Wed, 2024-01-24 at 15:29 +0100, Alexis Lothoré via
> lists.openembedded.org wrote:
>> From: Alexis Lothoré <alexis.lothore@bootlin.com>

[...]

>> +
>> +# 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"
>> \ No newline at end of file
> 
> I'm a little puzzled by this. Doesn't this mean it won't work for any
> image that isn't based upon minimal (which is most of them)?

Arg, I may have made wrong assumptions there. I focused on ptest images (ptest
fast and ptest all), observed that at some point they inherited from minimal,
and assumed it would be the case for any image used for runtime tests.

Checking autobuilder config.json, I see indeed that a lot of runtime tests are
for example done on core-image-sato, which does not depend on
core-image-minimal.So indeed this default value needs to move elsewhere, maybe
remain in testimage (but I will have to find a fix for issues mentioned in the
commit). I'll check how to properly fix this.

Thanks,

Alexis

> 
> Cheers,
> 
> Richard
> 

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[OE-Core][PATCH 2/2] core-image-ptest: retrieve ptests directory when ptests fail

[Alexis Lothoré]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

TESTIMAGE_FAILED_QA_ARTIFACTS is defined in core-image-minimal with a
minimal list of files to retrieve whenever a runtime test fails. By
appending the ptest directory only in core-image-ptest.bb, thanks to
multiconfig feature used in the recipe, only failing ptests will lead to
corresponding ptest artifacts retrieval, instead of all ptests artifacts
retrieval.

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
This patch was present in the original artifacts series ([1])

Changes since previous series:
- use += instead of append syntax

[1] https://lore.kernel.org/openembedded-core/20230609064802.11777-4-alexis.lothore@bootlin.com/
---
 meta/recipes-core/images/core-image-ptest.bb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/recipes-core/images/core-image-ptest.bb b/meta/recipes-core/images/core-image-ptest.bb
index b6f5c2fd6049..fb96c542c0a3 100644
--- a/meta/recipes-core/images/core-image-ptest.bb
+++ b/meta/recipes-core/images/core-image-ptest.bb
@@ -43,3 +43,5 @@ python () {
         raise bb.parse.SkipRecipe("No class extension set")
 }
 
+# Include ptest directory in artifacts to retrieve if there is a failed test
+TESTIMAGE_FAILED_QA_ARTIFACTS += "${libdir}/${MCNAME}/ptest"
-- 
2.42.1