[libcamera-ci] [RFC PATCH v1 2/3] Separate the building and running of unit tests
Barnabás Pőcze
barnabas.pocze at ideasonboard.com
Mon Dec 16 18:28:46 CET 2024
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`.
>
> 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?)
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?
3. Ignoring `*.o` makes the sizes a lot closer in my opinion (for one
particular build: uncompressed 163M vs. 143M, compressed 28M vs. 15M
[probably largely due to the different compression schemes]).
In any case, please see the new version of these changes.
Regards,
Barnabás Pőcze
>
>>>>>> + expire_in: 1 day
>>>>>> +
>>>>>> +# Run the unit tests in a virtual machine.
>>>>>> +test-unit:
>>>>>> + extends:
>>>>>> + - .fdo.distribution-image at debian
>>>>>> + - .libcamera-ci.debian:12
>>>>>> + - .libcamera-ci.scripts
>>>>>> + stage: test
>>>>>> + needs:
>>>>>> + - job: build-test:debug
>>>>>> + tags:
>>>>>> + - kvm
>>>>>> + script:
>>>>>> + - $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/
>>>>>>
>>>>>> # meson prior to 1.2.0 doesn't correctly escape non-printable characters
>>>>>> # when generating the testlog XML. This results in an unparseable file.
>
More information about the libcamera-devel
mailing list