No test suite run at built time nor as autopkgtest

[Paul Mars]

Track the addition of a test suite to be run at built time and during autopkgtest.

[Paul Mars (upils)]

Updated patch. The test binary will now return 1 if at least one test failed.

I will run autopkgtest when the pkg is published, and I expect it will fail (and correctly report this failure) for s390x. We should then decide if this failure is blocking the MIR process or not.

The ppa: https://launchpad.net/~upils/+archive/ubuntu/test-ppa/+packages

  - libtraceevent/1:1.8.2-1ubuntu5
    + ✅ libtraceevent on noble for amd64 @ 27.02.24 14:45:01
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/amd64/libt/libtraceevent/20240227_144501_533ef@/log.gz
    + ✅ libtraceevent on noble for arm64 @ 27.02.24 14:44:50
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/arm64/libt/libtraceevent/20240227_144450_c5fe0@/log.gz
    + ✅ libtraceevent on noble for armhf @ 27.02.24 14:50:27
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/armhf/libt/libtraceevent/20240227_145027_856fe@/log.gz
    + ✅ libtraceevent on noble for ppc64el @ 27.02.24 14:50:53
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/ppc64el/libt/libtraceevent/20240227_145053_65299@/log.gz

[Paul Mars (upils)]

Current patch.

[Paul Mars (upils)]

Failure on s390x https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/s390x/libt/libtraceevent/20240228_090511_c0996@/log.gz

[Sudip Mukherjee (sudipmuk)]

few points.
1. libtraceevent will be using files from "/sys/kernel/debug/tracing/events" which might or might not be available depending on debugfs is mounted or not.

2. The build is usually done in a schroot and the schroot will not have its own kernel, so it will use /sys/kernel/ of the host and the tests during build time might fail depending on if it can access "/sys/kernel/debug/tracing/events" or not. In my local schroot, the builds can not access "/sys/kernel/debug/tracing/events".

3. In autopkgtest, the tests will work for Ubuntu as Ubuntu autopkgtests are run in a VM, but Debian autopkgtests are run in a lxc container which again will not have its own kernel. So, enabling tests for autopkgtest needs to stay as an Ubuntu delta.

[Sudip Mukherjee (sudipmuk)]

Ignore the above comment please, that was before seeing the diff. I thought you were running the samples as a test. The unit tests should be ok.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "libtraceevent_1.8.2-1ubuntu5.diff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Paul Mars (upils)]

Hey sudipmuk. Thanks for the insight anyway, because I thought of executing the samples at first, so I could have hit these issues.

By the way, If you have any idea why tests on s390x fail, I am interested!

So far I suspected the endianness in dyn_str_data (since s390x is big endian) but it looks like it was correctly handled. But I do not enough about C to know if https://salsa.debian.org/sudip/libtraceevent/-/blob/master/utest/traceevent-utest.c?ref_type=heads#L45 is the right method to detect endianess.

[Paul Mars (upils)]

sudipmuk do you happen to know if unit tests ever worked on s390x?

[Paul Mars (upils)]

I made some progress. It looks like part of the dataset used to run test is adapted to big endian (via #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) but this is not needed (and even problematic).

I am now able to run 9 out of 11 tests successfully and I am still trying to find a solution for the remaining two.

[Paul Mars (upils)]

Current patch. 3 tests still fail.

[Paul Mars (upils)]

Here is new patch. Thanks to mkukri for finding the root cause of the failing tests on s390x.

sudipmuk I have split the change in 3 different patch so this will be easier to upstream.

PPA: https://launchpad.net/~upils/+archive/ubuntu/test-ppa

Tests are passing

  - libtraceevent/1:1.8.2-1ubuntu9
    + ✅ libtraceevent on noble for amd64 @ 07.03.24 09:10:27
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/amd64/libt/libtraceevent/20240307_091027_1a1a3@/log.gz
    + ✅ libtraceevent on noble for arm64 @ 07.03.24 09:04:53
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/arm64/libt/libtraceevent/20240307_090453_37dbe@/log.gz
    + ✅ libtraceevent on noble for armhf @ 07.03.24 09:07:03
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/armhf/libt/libtraceevent/20240307_090703_fff6c@/log.gz
    + ✅ libtraceevent on noble for ppc64el @ 07.03.24 09:04:11
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/ppc64el/libt/libtraceevent/20240307_090411_7b1ec@/log.gz
    + ✅ libtraceevent on noble for s390x @ 07.03.24 09:07:19
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-upils-test-ppa/noble/s390x/libt/libtraceevent/20240307_090719_74df1@/log.gz

[Paul Mars (upils)]

Updated patch. Autopktests in progress, I will update this comment when they are done.

[Benjamin Drung (bdrung)]

1. You do not need to bump the package number as long as it wasn't uploaded to the main archive. So 1:1.8.2-1ubuntu1 should be the version number.

2. Have you forwarded 0002-fail-unitest-when-test-fail.patch and 0003-fix-tests-on-big-endian.patch to upstream? If yes, please add that to the patch headers.

3. A newline at the end of debian/rules is missing.

4. 0001-utest-autopkgtest.patch is not needed. You can override it with `make test LIBTRACEEVENT_STATIC="-ltraceevent"`

5. debian/tests/utest is quite small. You might consider putting `make test LIBTRACEEVENT_STATIC="-ltraceevent" && ./utest/trace-utest` directly into debian/tests/control (and set the test name there).

6. Have/will you forward your changes to Debian?

Unsubscribing ~ubuntu-sponsors. Please resubscribe ~ubuntu-sponsors and ping me once you addressed/answered these points.

[Paul Mars (upils)]

Thanks for the review bdrung.

> 2. Have you forwarded 0002-fail-unitest-when-test-fail.patch and 0003-fix-tests-on-big-endian.patch to upstream?

No, because I would rather be sure this is reviewed and validated before forwarding.

> 3. A newline at the end of debian/rules is missing.

Thanks, now fixed.

> 4. 0001-utest-autopkgtest.patch is not needed. You can override it with `make test LIBTRACEEVENT_STATIC="-ltraceevent"`

Well I fixed that, but this is not working and I do not understand why yet.

> 5. debian/tests/utest is quite small.

Fixed. Indeed this is simpler.

> 6. Have/will you forward your changes to Debian?

Is that the same question as 2? :)

[Paul Mars (upils)]

Correction of the previous patch using dpkg -L to get the lib path again, because this is working and the previous method was not. Maybe we can improve later.

[Benjamin Drung (bdrung)]

>> 2. Have you forwarded 0002-fail-unitest-when-test-fail.patch and 0003-fix-tests-on-big-endian.patch to upstream?
>
> No, because I would rather be sure this is reviewed and validated before forwarding.

IMO it is better to have upstream feedback on the change before uploading (in case they respond quickly) and to have the upstream merge request linked in the patch file.

In this specific case, these two patches look plausible to me and could be forwarded.

[Nick Rosbrook (enr0n)]

Hi Paul,

I think Benjamin led you astray with this comment:

> 4. 0001-utest-autopkgtest.patch is not needed. You can override it with `make test LIBTRACEEVENT_STATIC="-ltraceevent"`

This variable is *not* override-able without your patch -- it will just be assigned unconditionally in the Makefile. When I ran autopkgtest locally with your current patch, it fails:

```
autopkgtest [11:50:49]: test command1: LIBTRACEEVENT_STATIC=$(dpkg -L libtraceevent-dev | grep libtraceevent.so) make test && ./utest/trace-utest
autopkgtest [11:50:49]: test command1: [-----------------------
  DESCEND src libtraceevent.a
  COMPILE FPIC event-parse-api.o
  COMPILE FPIC event-parse.o
  COMPILE FPIC event-plugin.o
  COMPILE FPIC kbuffer-parse.o
  COMPILE FPIC parse-filter.o
  COMPILE FPIC parse-utils.o
  COMPILE FPIC tep_strerror.o
  COMPILE FPIC trace-seq.o
  BUILD STATIC LIB libtraceevent.a
ar: /tmp/autopkgtest.1gCwTh/build.6Vd/src/lib/libtraceevent.a: No such file or directory
make[1]: *** [Makefile:22: /tmp/autopkgtest.1gCwTh/build.6Vd/src/lib/libtraceevent.a] Error 1
make: *** [Makefile:212: /tmp/autopkgtest.1gCwTh/build.6Vd/src/lib/libtraceevent.a] Error 2
autopkgtest [11:50:51]: test command1: -----------------------]
autopkgtest [11:50:52]: test command1: - - - - - - - - - - results - - - - - - - - - -
command1 FAIL non-zero exit status 2
autopkgtest [11:50:52]: test command1: - - - - - - - - - - stderr - - - - - - - - - -
ar: /tmp/autopkgtest.1gCwTh/build.6Vd/src/lib/libtraceevent.a: No such file or directory
make[1]: *** [Makefile:22: /tmp/autopkgtest.1gCwTh/build.6Vd/src/lib/libtraceevent.a] Error 1
make: *** [Makefile:212: /tmp/autopkgtest.1gCwTh/build.6Vd/src/lib/libtraceevent.a] Error 2
```

Restoring the patch to make LIBTRACEEVENT_STATIC override-able fixes this.

One other thing that would be nice, but is not strictly necessary, would be to give the autopkgtest a user-friendly name, e.g. 'trace-utest'. You can do this by adding `Features: test-name=trace-utest` to the appropriate stanza in debian/tests/control. Otherwise, the test is called 'command1', which is fine, but not ideal.

I don't think we need to block on upstream, but as Benjamin suggested, please do forward the patches (after fixing the above) to upstream, and add a `Forwarded: <URL to upstream PR>` header to the appropriate patch files.

Thanks for your continued effort on this.

[Paul Mars (upils)]

Here is a patch with your suggested changes:

- a friendly name on the autopkgtest
- the patch to override the LIBTRACEEVENT_STATIC var added back
- a note about the hijack of this var
- patches 0002 and 0003 forwarded upstream
- Forwarded headers added

autopkgtest is currently running in my PPA.

[Paul Mars (upils)]

Updated patch with enr0n's fix to properly link against the installed lib and avoid building and linking to the LIBTRACEEVENT_STATIC.

[Nick Rosbrook (enr0n)]

Thanks, Paul! Looks good to me. I have sponsored this now.

[Launchpad Janitor (janitor)]

This bug was fixed in the package libtraceevent - 1:1.8.2-1ubuntu2

---------------
libtraceevent (1:1.8.2-1ubuntu2) noble; urgency=medium

  * No-change rebuild for CVE-2024-3094

 -- William Grant <email address hidden> Wed, 03 Apr 2024 22:46:23 +1100

[Benjamin Drung (bdrung)]

>> 4. 0001-utest-autopkgtest.patch is not needed. You can override it with `make test LIBTRACEEVENT_STATIC="-ltraceevent"`
>
> This variable is *not* override-able without your patch -- it will just be assigned unconditionally in the Makefile.

I tested the attached patch and it is working. The important part is to specify LIBTRACEEVENT_STATIC as parameter to the make call and not as environment variable.