[libcamera-ci] [RFC PATCH v1 2/3] Separate the building and running of unit tests

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Dec 16 12:04:42 CET 2024


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.

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.

> >>>> +    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.

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list