[libcamera-ci] [RFC PATCH v1 2/3] Separate the building and running of unit tests
Barnabás Pőcze
barnabas.pocze at ideasonboard.com
Tue Dec 17 17:09:45 CET 2024
2024. 12. 16. 21:01 keltezéssel, Laurent Pinchart írta:
> On Mon, Dec 16, 2024 at 06:26:14PM +0000, Kieran Bingham wrote:
>> Quoting Barnabás Pőcze (2024-12-16 17:28:46)
>>> 2024. 12. 16. 12:04 keltezéssel, Laurent Pinchart írta:
>>>> On Mon, Dec 16, 2024 at 10:13:54AM +0100, Barnabás Pőcze wrote:
>>>>> 2024. 12. 15. 21:43 keltezéssel, Laurent Pinchart írta:
>>>>>> On Sun, Dec 15, 2024 at 09:43:20PM +0200, Laurent Pinchart wrote:
>>>>>>> On Sun, Dec 15, 2024 at 09:04:08PM +0200, Laurent Pinchart wrote:
>>>>>>>> Hi Barnabás,
>>>>>>>>
>>>>>>>> Thank you for the patch.
>>>>>>>>
>>>>>>>> On Thu, Dec 12, 2024 at 07:16:54PM +0100, Barnabás Pőcze wrote:
>>>>>>>>> The built artifacts will be reused in a later job, so split
>>>>>>>>> the "test-unit" into the "build-test" and "test-unit" jobs.
>>>>>>>>>
>>>>>>>>> The `libevent` development package cannot be installed in the container
>>>>>>>>
>>>>>>>> I've write `libevent-dev` here to avoid ambiguities.
>>>>>>>>
>>>>>>>>> directly because it is not multiarch compatible. It is, however, installed
>>>>>>>>> in the architecture specific build jobs, right before building. To ensure
>>>>>>>>> that the it is available for already built executables in different jobs,
>>>>>>>>
>>>>>>>> "that the it is" ?
>>>>>>>>
>>>>>>>>> install just the libraries in the container.
>>>>>>>>
>>>>>>>> And name here `libevent`.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
>>>>>>>>> ---
>>>>>>>>> .gitlab-ci/setup-container.sh | 3 +++
>>>>>>>>> gitlab-ci.yml | 42 +++++++++++++++++++++++------------
>>>>>>>>> 2 files changed, 31 insertions(+), 14 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh
>>>>>>>>> index d2909c7..0658368 100755
>>>>>>>>> --- a/.gitlab-ci/setup-container.sh
>>>>>>>>> +++ b/.gitlab-ci/setup-container.sh
>>>>>>>>> @@ -103,6 +103,9 @@ case $FDO_DISTRIBUTION_VERSION in
>>>>>>>>> 'bookworm')
>>>>>>>>> # libclang-rt-dev for the clang ASan runtime.
>>>>>>>>> PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-dev )
>>>>>>>>> + # For cam and lc-compliance
>>>>>>>>> + # libevent-dev cannot be used here, see build-libcamera-common.sh
>>>>>>>>> + PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libevent-2.1-7 libevent-pthreads-2.1-7 )
>>>>>>>>> ;;
>>>>>>>>> 'trixie')
>>>>>>>>> # gcc 13 to expand compilation testing coverage.
>>>>>>>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
>>>>>>>>> index 8bc8bc2..c7448b8 100644
>>>>>>>>> --- a/gitlab-ci.yml
>>>>>>>>> +++ b/gitlab-ci.yml
>>>>>>>>> @@ -64,7 +64,7 @@ include:
>>>>>>>>> .libcamera-ci.debian:12:
>>>>>>>>> variables:
>>>>>>>>> FDO_DISTRIBUTION_VERSION: 'bookworm'
>>>>>>>>> - FDO_DISTRIBUTION_TAG: '2024-12-12.1'
>>>>>>>>> + FDO_DISTRIBUTION_TAG: '2024-12-12.2'
>>>>>>>>>
>>>>>>>>> .libcamera-ci.debian:13:
>>>>>>>>> variables:
>>>>>>>>> @@ -363,28 +363,18 @@ test-soraka:
>>>>>>>>> script:
>>>>>>>>> - submit .gitlab-ci/lava/soraka-camera-test.yml
>>>>>>>>>
>>>>>>>>> -# Run the unit tests in a virtual machine. Enable only the options exercised by
>>>>>>>>> -# the unit tests.
>>>>>>>>> -test-unit:
>>>>>>>>> +# Enable only the options exercised by the unit tests.
>>>>>>>>> +build-test:debug:
>>>>>>>>
>>>>>>>> I'd call this build-package:amd64, as we have build-package:arm64 and
>>>>>>>> build-package:cros. I think it would also make sense to use the same
>>>>>>>> build options for the amd64 and arm64 packages (beside possibly the
>>>>>>>> selected pipeline handlers, although the 'auto' option may work for
>>>>>>>> both).
>>>>>>>>
>>>>>>>>> extends:
>>>>>>>>> - .fdo.distribution-image at debian
>>>>>>>>> - .libcamera-ci.debian:12
>>>>>>>>> - .libcamera-ci.scripts
>>>>>>>>> - stage: test
>>>>>>>>> + stage: build
>>>>>>>>> needs:
>>>>>>>>> - job: container-debian:12
>>>>>>>>> artifacts: false
>>>>>>>>> - tags:
>>>>>>>>> - - kvm
>>>>>>>>> script:
>>>>>>>>> - $CI_PROJECT_DIR/.gitlab-ci/build-libcamera.sh
>>>>>>>>> - - $CI_PROJECT_DIR/.gitlab-ci/test-libcamera-qemu.sh
>>>>>>>>> - artifacts:
>>>>>>>>> - name: libcamera-unit-tests-${CI_COMMIT_SHA}
>>>>>>>>> - when: always
>>>>>>>>> - expire_in: 1 week
>>>>>>>>> - paths:
>>>>>>>>> - - build/meson-logs/
>>>>>>>>> variables:
>>>>>>>>> BUILD_TYPE: debug
>>>>>>>>> MESON_OPTIONS: >-
>>>>>>>>> @@ -399,6 +389,30 @@ test-unit:
>>>>>>>>> -D qcam=disabled
>>>>>>>>> -D test=true
>>>>>>>>> -D v4l2=true
>>>>>>>>> + artifacts:
>>>>>>>>> + paths:
>>>>>>>>> + - build/
>>>>>>>>
>>>>>>>> The whole build directory can be very large. Can't we do the same as
>>>>>>>> build-package:arm64 and run package-libcamera.sh to only package what we
>>>>>>>> need ? We'll need probably need an unpackage script for the test-unit
>>>>>>>> job.
>>>>>>>
>>>>>>> But of course the unit test binaries don't get installed... Can we fix
>>>>>>> that and install them ? You can specify "install_tag : 'tests'" in
>>>>>>> meson.build so they won't be installed by default (an appropriate
>>>>>>> install_dir is also needed). This in turn requires bumping the minimum
>>>>>>> meson version from 0.63.0 to 0.64.0, which shouldn't be an issue.
>>>>>>
>>>>>> I've been told on IRC that the motivation for the "tests" install tag in
>>>>>> meson is https://gitlab.gnome.org/GNOME/gnome-desktop-testing. I don't
>>>>>> think we should switch to a separate runner for unit tests (the pain is
>>>>>> not worth the gain at this point in my opinion), but it could be useful
>>>>>> to tag lc-compliance with install_tag = 'tests'.
>>>>>>
>>>>>>> And now that I've said this, I realize we wouldn't be able to run "meson
>>>>>>> test" to run the tests :-/ I'm not sure there's an appropriate solution
>>>>>>> for this. If not, given the size of the build directory, and to avoid
>>>>>>> transferring a large amount of data between runners, we may need to keep
>>>>>>> building libcamera within the test-unit job :-(
>>>>>>>
>>>>>>> A separate build-package target for lc-compliance would still make
>>>>>>> sense.
>>>>>
>>>>> I think it would be unfortunate to give up the usage `meson test` as you
>>>>> mentioned.
>>>>
>>>> We could work on a replacement, but it would require a significant
>>>> amount of work and I think there are better things to do.
>>>>
>>>>> I have not noticed that these build artifacts would put any
>>>>> appreciable strain on the infrastructure. The compressed build directory
>>>>> comes out to around 167 MiB; I am not sure if I would consider that a
>>>>> large amount of data. It is definitely cheaper, in terms of time, than
>>>>> building libcamera twice. Clearing the object files could be another
>>>>> option. With `artifacts:exclude: build/**/*.o` we can seemingly
>>>>> remove more than half of the uncompressed size, and about 1/4 of
>>>>> the compressed size. Does this look acceptable?
>>>>
>>>> Possibly. We should probably ask the fdo sysadmins about what is
>>>> acceptable.
>>>>
>>>> I gave it a try locally though, and deleting all *.o files in the build
>>>> directory results in "meson test" rebuilding everything.
>>>
>>> As far as I can see that does not happen with the `--no-rebuild` option,
>>> which is already used in `test-libcamera-qemu.sh`.
>
> Ah, good point, I missed that.
>
>>>> For other uses of the artifacts (in particular deployment on real
>>>> devices), I would still prefer minimizing the bandwidth, creating a
>>>> package similarly to what build-package:arm64 does. How about keeping
>>>> test-unit as-is, at the cost of a recompilation, and creating a
>>>> build-package:amd64 that will be used by the lc-compliance test job ? We
>>>> can try to improve on top when/if needed.
>>>
>>> Couple observations:
>>>
>>> 1. The virtual pipeline handler configuration is not installed, so
>>> that needs to be addressed. (Was this omitted intentionally?)
>
> I don't know. We have to be careful here, we don't want to virtual
> pipeline handler to end up being enabled with a default valid
> configuration in distributions, otherwise people will all of a sudden
> see virtual cameras poluting their devices list.
I am not sure what should be done. Maybe a separate meson `install_tag`.
Or I suppose the configuration file could be created right before
running it?
>
>>> 2. I am not a fan of the extra `tar` and `ldconfig` calls that
>>> need to be sprinkled in. I think this would be much better
>>> if the package was not a mere tar archive but a proper deb/etc.
>>> package. I imagine that is a prerequisite of deploying on real
>>> hardware in any case, correct?
>>
>> Having the server build a 'real' deb would be a real benefit IMO, and
>> indeed could help with installation/set up on real targets for testing
>> in defined environments.
>>
>> I've wanted to tackle that for a while but never got time.
>
> Same, I think it's probably worth it, but it will require some effort
> and I didn't have enough time when I implemented build-package:arm64.
>
More information about the libcamera-devel
mailing list