[PATCHv2,1/2] classes/ptest-python-pytest: add a new class to consolidate pytest ptest functionality

A large number of python packages leverage the pytest unit test
framework for their ptest functionality.  Currently, many of the tests
have duplicate code for:
    1. Installing pytest files
    2. Declaring ptest dependencies
    3. Script for executing tests (run-ptes)

To simplify adding common pytest based ptests, added a new class
enabling base functionality.  Users can also override the location of
the pytest files in addition to using their own version of run-ptest

Signed-off-by: Derek Straka <derek@asterius.io>
---
 .../ptest-python-pytest.bbclass               | 42 +++++++++++++++++++
 meta/files/ptest-python-pytest/run-ptest      |  3 ++
 2 files changed, 45 insertions(+)
 create mode 100644 meta/classes-recipe/ptest-python-pytest.bbclass
 create mode 100755 meta/files/ptest-python-pytest/run-ptest

On Wed, 18 Dec 2024 at 00:12, Derek Straka via lists.openembedded.org
<derek=asterius.io@lists.openembedded.org> wrote:
> +addtask install_ptest_python_pytest after do_install_ptest_base before do_package
> +
> +python () {
> +    if not bb.data.inherits_class('native', d) and not bb.data.inherits_class('cross', d):
> +        d.setVarFlag('do_install_ptest_python_pytest', 'fakeroot', '1')
> +        d.setVarFlag('do_install_ptest_python_pytest', 'umask', '022')
> +
> +    # Remove all '*ptest_python_pytest' tasks when ptest is not enabled
> +    if not(d.getVar('PTEST_ENABLED') == "1"):
> +        for i in ['do_install_ptest_python_pytest']:
> +            bb.build.deltask(i, d)
> +}

I know other ptest extension classes do this, but I really do not
think adding another task and copy-pasting special handling in
anonymous python is necessary. Just define do_install_ptest().

Alex

Hi Derek,

On 12/18/24 12:12 AM, Derek Straka via lists.openembedded.org wrote:
> A large number of python packages leverage the pytest unit test
> framework for their ptest functionality.  Currently, many of the tests
> have duplicate code for:
>      1. Installing pytest files
>      2. Declaring ptest dependencies
>      3. Script for executing tests (run-ptes)
> 
> To simplify adding common pytest based ptests, added a new class
> enabling base functionality.  Users can also override the location of
> the pytest files in addition to using their own version of run-ptest
> 
> Signed-off-by: Derek Straka <derek@asterius.io>
> ---
>   .../ptest-python-pytest.bbclass               | 42 +++++++++++++++++++
>   meta/files/ptest-python-pytest/run-ptest      |  3 ++
>   2 files changed, 45 insertions(+)
>   create mode 100644 meta/classes-recipe/ptest-python-pytest.bbclass
>   create mode 100755 meta/files/ptest-python-pytest/run-ptest
> 
> diff --git a/meta/classes-recipe/ptest-python-pytest.bbclass b/meta/classes-recipe/ptest-python-pytest.bbclass
> new file mode 100644
> index 0000000000..89ff10c335
> --- /dev/null
> +++ b/meta/classes-recipe/ptest-python-pytest.bbclass
> @@ -0,0 +1,42 @@
> +#
> +# Copyright OpenEmbedded Contributors
> +#
> +# SPDX-License-Identifier: MIT
> +#
> +
> +inherit ptest
> +
> +FILESEXTRAPATHS:prepend := "${COREBASE}/meta/files:"

I think it'd make sense to use :append here as we don't want to override 
anything, just provide a fallback in case the 
ptest-python-pytest/run-ptest isn't find anywhere else. This would also 
allow to easily override this file by just adding the file next to the 
recipe, instead of having to add a FILESEXTRAPATHS:prepend := 
"${THISDIR}/${PN}:" in the recipe after the inherit for example.

> +
> +SRC_URI += "file://ptest-python-pytest/run-ptest"
> +

We use append/prepend everywhere but here, should we?

> +# Overridable configuration for the directory within the source tree
> +# containing the pytest files
> +PTEST_PYTEST_DIR ?= "/tests"
> +
> +do_install_ptest_python_pytest() {
> +	if [ ! -f ${D}${PTEST_PATH}/run-ptest ]; then
> +		install -m 0755 ${UNPACKDIR}/ptest-python-pytest/run-ptest ${D}${PTEST_PATH}
> +	fi

This means if we ever built the recipe and don't clean its WORKDIR, but 
update the file, it will never be updated. I don't think that's what we 
want. Can you explain what you're trying to prevent here by having a 
check for NOT installing the file?

> +    if [ -d "${S}/${PTEST_PYTEST_DIR}" ]; then

Shouldn't we use UNPACKDIR for file:// SRC_URI in master?

Cheers,
Quentin

V3 to be sent shortly based on inputs from you and Alex.

On Wed, Dec 18, 2024 at 7:32 AM Quentin Schulz <quentin.schulz@cherry.de>
wrote:

> Hi Derek,
>
Hi Quentin,

Thanks for your note.

>
> On 12/18/24 12:12 AM, Derek Straka via lists.openembedded.org wrote:
> > A large number of python packages leverage the pytest unit test
> > framework for their ptest functionality.  Currently, many of the tests
> > have duplicate code for:
> >      1. Installing pytest files
> >      2. Declaring ptest dependencies
> >      3. Script for executing tests (run-ptes)
> >
> > To simplify adding common pytest based ptests, added a new class
> > enabling base functionality.  Users can also override the location of
> > the pytest files in addition to using their own version of run-ptest
> >
> > Signed-off-by: Derek Straka <derek@asterius.io>
> > ---
> >   .../ptest-python-pytest.bbclass               | 42 +++++++++++++++++++
> >   meta/files/ptest-python-pytest/run-ptest      |  3 ++
> >   2 files changed, 45 insertions(+)
> >   create mode 100644 meta/classes-recipe/ptest-python-pytest.bbclass
> >   create mode 100755 meta/files/ptest-python-pytest/run-ptest
> >
> > diff --git a/meta/classes-recipe/ptest-python-pytest.bbclass
> b/meta/classes-recipe/ptest-python-pytest.bbclass
> > new file mode 100644
> > index 0000000000..89ff10c335
> > --- /dev/null
> > +++ b/meta/classes-recipe/ptest-python-pytest.bbclass
> > @@ -0,0 +1,42 @@
> > +#
> > +# Copyright OpenEmbedded Contributors
> > +#
> > +# SPDX-License-Identifier: MIT
> > +#
> > +
> > +inherit ptest
> > +
> > +FILESEXTRAPATHS:prepend := "${COREBASE}/meta/files:"
>
> I think it'd make sense to use :append here as we don't want to override
> anything, just provide a fallback in case the
> ptest-python-pytest/run-ptest isn't find anywhere else. This would also
> allow to easily override this file by just adding the file next to the
> recipe, instead of having to add a FILESEXTRAPATHS:prepend :=
> "${THISDIR}/${PN}:" in the recipe after the inherit for example.
>
> I was able to override in oe-core without adding any prepends in the
recipes.  My understanding is the EXTRA paths are searched as a last
resort, so recipes having their own run-ptest get theirs included first.
Overall, the class was adapted from ptest-perl and ptest-cargo.

> > +
> > +SRC_URI += "file://ptest-python-pytest/run-ptest"
> > +
>
> We use append/prepend everywhere but here, should we?
>
> The code was borrowed from ptest-perl.bbclass.  I'll update the python
class as suggested.

> > +# Overridable configuration for the directory within the source tree
> > +# containing the pytest files
> > +PTEST_PYTEST_DIR ?= "/tests"
> > +
> > +do_install_ptest_python_pytest() {
> > +     if [ ! -f ${D}${PTEST_PATH}/run-ptest ]; then
> > +             install -m 0755 ${UNPACKDIR}/ptest-python-pytest/run-ptest
> ${D}${PTEST_PATH}
> > +     fi
>
> This means if we ever built the recipe and don't clean its WORKDIR, but
> update the file, it will never be updated. I don't think that's what we
> want. Can you explain what you're trying to prevent here by having a
> check for NOT installing the file?
>
> > +    if [ -d "${S}/${PTEST_PYTEST_DIR}" ]; then
>
> Shouldn't we use UNPACKDIR for file:// SRC_URI in master?
>
To my knowledge, the pypi recipes aren't putting the sources into UNPACKDIR
for better or worse.  I looked at several packages, and they're using `S =
"${WORKDIR}/${PYPI_PACKAGE}-${PV}"` (See pypi.bbclass)

>
> Cheers,
> Quentin
>

Hi Derek,

On 12/18/24 7:04 PM, Derek Straka wrote:
> V3 to be sent shortly based on inputs from you and Alex.
> 
> On Wed, Dec 18, 2024 at 7:32 AM Quentin Schulz <quentin.schulz@cherry.de>
> wrote:
> 
>> Hi Derek,
>>
> Hi Quentin,
> 
> Thanks for your note.
> 
>>
>> On 12/18/24 12:12 AM, Derek Straka via lists.openembedded.org wrote:
>>> A large number of python packages leverage the pytest unit test
>>> framework for their ptest functionality.  Currently, many of the tests
>>> have duplicate code for:
>>>       1. Installing pytest files
>>>       2. Declaring ptest dependencies
>>>       3. Script for executing tests (run-ptes)
>>>
>>> To simplify adding common pytest based ptests, added a new class
>>> enabling base functionality.  Users can also override the location of
>>> the pytest files in addition to using their own version of run-ptest
>>>
>>> Signed-off-by: Derek Straka <derek@asterius.io>
>>> ---
>>>    .../ptest-python-pytest.bbclass               | 42 +++++++++++++++++++
>>>    meta/files/ptest-python-pytest/run-ptest      |  3 ++
>>>    2 files changed, 45 insertions(+)
>>>    create mode 100644 meta/classes-recipe/ptest-python-pytest.bbclass
>>>    create mode 100755 meta/files/ptest-python-pytest/run-ptest
>>>
>>> diff --git a/meta/classes-recipe/ptest-python-pytest.bbclass
>> b/meta/classes-recipe/ptest-python-pytest.bbclass
>>> new file mode 100644
>>> index 0000000000..89ff10c335
>>> --- /dev/null
>>> +++ b/meta/classes-recipe/ptest-python-pytest.bbclass
>>> @@ -0,0 +1,42 @@
>>> +#
>>> +# Copyright OpenEmbedded Contributors
>>> +#
>>> +# SPDX-License-Identifier: MIT
>>> +#
>>> +
>>> +inherit ptest
>>> +
>>> +FILESEXTRAPATHS:prepend := "${COREBASE}/meta/files:"
>>
>> I think it'd make sense to use :append here as we don't want to override
>> anything, just provide a fallback in case the
>> ptest-python-pytest/run-ptest isn't find anywhere else. This would also
>> allow to easily override this file by just adding the file next to the
>> recipe, instead of having to add a FILESEXTRAPATHS:prepend :=
>> "${THISDIR}/${PN}:" in the recipe after the inherit for example.
>>
> I was able to override in oe-core without adding any prepends in the
> recipes.  My understanding is the EXTRA paths are searched as a last
> resort, so recipes having their own run-ptest get theirs included first.
> Overall, the class was adapted from ptest-perl and ptest-cargo.
>

FILESEXTRAPATHS:prepend is what we use in bbappends to search in the 
directory next to the bbappend **first** and then from the original 
recipe second.

It's a bit convoluted, but FILESPATH stores a list of paths that is 
constructed from FILESEXTRAPATHS + <original-recipe-dir>/${BP} + 
<original-recipe-dir>/${BPN} + <original-recipe-dir>/files (c.f. 
meta/classes-global/base.bbclass line 59, which calls base_set_filespath 
from meta/classes-global/utils.bbclass). FILESPATH is then read from 
left to right and the first path to have the listed file "wins".

Can you please provide the code you used to validate overriding 
run-ptest from the recipe? If that is the case, then I would like to 
understand how that happens and we possibly have hit a corner case we 
should fix (or at least be aware of).

>>> +
>>> +SRC_URI += "file://ptest-python-pytest/run-ptest"
>>> +
>>
>> We use append/prepend everywhere but here, should we?
>>
>> The code was borrowed from ptest-perl.bbclass.  I'll update the python
> class as suggested.
> 
>>> +# Overridable configuration for the directory within the source tree
>>> +# containing the pytest files
>>> +PTEST_PYTEST_DIR ?= "/tests"
>>> +
>>> +do_install_ptest_python_pytest() {
>>> +     if [ ! -f ${D}${PTEST_PATH}/run-ptest ]; then
>>> +             install -m 0755 ${UNPACKDIR}/ptest-python-pytest/run-ptest
>> ${D}${PTEST_PATH}
>>> +     fi
>>
>> This means if we ever built the recipe and don't clean its WORKDIR, but
>> update the file, it will never be updated. I don't think that's what we
>> want. Can you explain what you're trying to prevent here by having a
>> check for NOT installing the file?
>>
>>> +    if [ -d "${S}/${PTEST_PYTEST_DIR}" ]; then
>>
>> Shouldn't we use UNPACKDIR for file:// SRC_URI in master?
>>
> To my knowledge, the pypi recipes aren't putting the sources into UNPACKDIR
> for better or worse.  I looked at several packages, and they're using `S =
> "${WORKDIR}/${PYPI_PACKAGE}-${PV}"` (See pypi.bbclass)
> 

I'm not familiar with UNPACKDIR and its use as I'm still on Scarthgap.

Cheers,
Quentin

Thank you for the detailed explanation.  Paths have evolved a bit over
time.  I intend the run-ptest to be a file of last resort.

I'd ask the broader community what's the preferred way of adding a default
file from a class if it does not exist in bbappend or bb files?  I'm
struggling to get the syntax correct.  Thanks!

On Thu, Dec 19, 2024 at 4:37 AM Quentin Schulz <quentin.schulz@cherry.de>
wrote:

> Hi Derek,
>
> On 12/18/24 7:04 PM, Derek Straka wrote:
> > V3 to be sent shortly based on inputs from you and Alex.
> >
> > On Wed, Dec 18, 2024 at 7:32 AM Quentin Schulz <quentin.schulz@cherry.de
> >
> > wrote:
> >
> >> Hi Derek,
> >>
> > Hi Quentin,
> >
> > Thanks for your note.
> >
> >>
> >> On 12/18/24 12:12 AM, Derek Straka via lists.openembedded.org wrote:
> >>> A large number of python packages leverage the pytest unit test
> >>> framework for their ptest functionality.  Currently, many of the tests
> >>> have duplicate code for:
> >>>       1. Installing pytest files
> >>>       2. Declaring ptest dependencies
> >>>       3. Script for executing tests (run-ptes)
> >>>
> >>> To simplify adding common pytest based ptests, added a new class
> >>> enabling base functionality.  Users can also override the location of
> >>> the pytest files in addition to using their own version of run-ptest
> >>>
> >>> Signed-off-by: Derek Straka <derek@asterius.io>
> >>> ---
> >>>    .../ptest-python-pytest.bbclass               | 42
> +++++++++++++++++++
> >>>    meta/files/ptest-python-pytest/run-ptest      |  3 ++
> >>>    2 files changed, 45 insertions(+)
> >>>    create mode 100644 meta/classes-recipe/ptest-python-pytest.bbclass
> >>>    create mode 100755 meta/files/ptest-python-pytest/run-ptest
> >>>
> >>> diff --git a/meta/classes-recipe/ptest-python-pytest.bbclass
> >> b/meta/classes-recipe/ptest-python-pytest.bbclass
> >>> new file mode 100644
> >>> index 0000000000..89ff10c335
> >>> --- /dev/null
> >>> +++ b/meta/classes-recipe/ptest-python-pytest.bbclass
> >>> @@ -0,0 +1,42 @@
> >>> +#
> >>> +# Copyright OpenEmbedded Contributors
> >>> +#
> >>> +# SPDX-License-Identifier: MIT
> >>> +#
> >>> +
> >>> +inherit ptest
> >>> +
> >>> +FILESEXTRAPATHS:prepend := "${COREBASE}/meta/files:"
> >>
> >> I think it'd make sense to use :append here as we don't want to override
> >> anything, just provide a fallback in case the
> >> ptest-python-pytest/run-ptest isn't find anywhere else. This would also
> >> allow to easily override this file by just adding the file next to the
> >> recipe, instead of having to add a FILESEXTRAPATHS:prepend :=
> >> "${THISDIR}/${PN}:" in the recipe after the inherit for example.
> >>
> > I was able to override in oe-core without adding any prepends in the
> > recipes.  My understanding is the EXTRA paths are searched as a last
> > resort, so recipes having their own run-ptest get theirs included first.
> > Overall, the class was adapted from ptest-perl and ptest-cargo.
> >
>
> FILESEXTRAPATHS:prepend is what we use in bbappends to search in the
> directory next to the bbappend **first** and then from the original
> recipe second.
>
> It's a bit convoluted, but FILESPATH stores a list of paths that is
> constructed from FILESEXTRAPATHS + <original-recipe-dir>/${BP} +
> <original-recipe-dir>/${BPN} + <original-recipe-dir>/files (c.f.
> meta/classes-global/base.bbclass line 59, which calls base_set_filespath
> from meta/classes-global/utils.bbclass). FILESPATH is then read from
> left to right and the first path to have the listed file "wins".
>
> Can you please provide the code you used to validate overriding
> run-ptest from the recipe? If that is the case, then I would like to
> understand how that happens and we possibly have hit a corner case we
> should fix (or at least be aware of).
>
> >>> +
> >>> +SRC_URI += "file://ptest-python-pytest/run-ptest"
> >>> +
> >>
> >> We use append/prepend everywhere but here, should we?
> >>
> >> The code was borrowed from ptest-perl.bbclass.  I'll update the python
> > class as suggested.
> >
> >>> +# Overridable configuration for the directory within the source tree
> >>> +# containing the pytest files
> >>> +PTEST_PYTEST_DIR ?= "/tests"
> >>> +
> >>> +do_install_ptest_python_pytest() {
> >>> +     if [ ! -f ${D}${PTEST_PATH}/run-ptest ]; then
> >>> +             install -m 0755
> ${UNPACKDIR}/ptest-python-pytest/run-ptest
> >> ${D}${PTEST_PATH}
> >>> +     fi
> >>
> >> This means if we ever built the recipe and don't clean its WORKDIR, but
> >> update the file, it will never be updated. I don't think that's what we
> >> want. Can you explain what you're trying to prevent here by having a
> >> check for NOT installing the file?
> >>
> >>> +    if [ -d "${S}/${PTEST_PYTEST_DIR}" ]; then
> >>
> >> Shouldn't we use UNPACKDIR for file:// SRC_URI in master?
> >>
> > To my knowledge, the pypi recipes aren't putting the sources into
> UNPACKDIR
> > for better or worse.  I looked at several packages, and they're using `S
> =
> > "${WORKDIR}/${PYPI_PACKAGE}-${PV}"` (See pypi.bbclass)
> >
>
> I'm not familiar with UNPACKDIR and its use as I'm still on Scarthgap.
>
> Cheers,
> Quentin
>