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

["Alexis Lothoré" <alexis.lothore@bootlin.com>]

On 2/5/24 20:22, Alexis Lothoré wrote:
> Hello Richard, thanks for the fast review
> 
> On 2/5/24 16:08, Richard Purdie wrote:
>> On Mon, 2024-02-05 at 15:37 +0100, Alexis Lothoré via
>> lists.openembedded.org wrote:
> 
> [...]
> 
>>> -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've indeed given a try to this before moving TESTIMAGE_FAILED_QA_ARTIFACTS to
> core-image, but when using "=" instead of "?=" or "??=", I observe that the part
> affecting TESTIMAGE_FAILED_QA_ARTIFACTS in core-image-ptest is not taken, so we
> are missing the ptests artifacts (but we still get the generic artifacts defined
> in testimage.bbclass). So to summarize:
> - "??=" or "?=" : we only get ptests artifacts, not the "generic" one
> (/etc/version, /var/log, etc)
> - "=": we only get the generic artifacts, not the ptests one.
> It is not clear for me why the second one fails, and
> TESTIMAGE_FAILED_QA_ARTIFACTS does not appear at all in 'bitbake -e" output,
> which makes it even more cryptic to me.
> 
>> 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.
> 
> Based on my understanding of bitbake manual, I theoretically agree with you,
> yet, as mentioned above this is not the behavior I observe, so I may be missing
> something important here. Maybe the parsing order is not the one I think I am
> observing, maybe its multiconfig, maybe something else.
> I can try to dig this further and clarify why it does not work as expected with "="

So I've did multiple attempts to make this work, and I see no way to define the
default value in testimage.bbclass with "=", and add content to it in
core-image-ptest.bbwith "+=". My understanding is that testimage is always
parsed after core-image-ptest.bb, so there is no nice way to set its default
value in there:
- "=" will always overwrite ptests artifacts set with "+="
- "?=" or "??=" will always be overwritten by ptests artifacts set with "+="
My configuration relies on IMAGE_CLASSES += 'testimage', which is also done in CI.

Still, I understand your concern about keeping default value in
testimage.bbclass. The only compromise I see here to both keep the default value
in testimage and be able to override it at user level is not to enrich it in
core-image-ptest but to redefine it:

diff --git a/meta/recipes-core/images/core-image-ptest.bb
b/meta/recipes-core/images/core-image-ptest.bb
index fb96c542c0a3..4a90181afd83 100644
--- a/meta/recipes-core/images/core-image-ptest.bb
+++ b/meta/recipes-core/images/core-image-ptest.bb
@@ -43,5 +43,8 @@ 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"
+TESTIMAGE_FAILED_QA_ARTIFACTS = "\
+    ${localstatedir}/log \
+    ${sysconfdir}/version \
+    ${sysconfdir}/os-release \
+    ${libdir}/${MCNAME}/ptest"

That's for example what is done with TEST_SUITES. In this setup, we can keep a
default value in testimage, and override it as needed with append/prepend/remove
syntax at user level.

I'll wait a bit in case there's any comment or concern about this approach, and
then send a new version.

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